Hgfs Fuse Client: fix for backdoor not available

The client dropped errors for failing to open the backdoor returning FALSE,
when the HGFS server was not enabled which migrated to a protocol error.
This could mislead users when mounting the FUSE file system when the
feature was disabled into thinking there was a different issue when
actually enabling the feature was required before mounting. It was not
a protocol mismatch between the client and server.

The main entry point now logs a message for the user if the transport init
fails and so the mount does not occur.
i
The main issue to prevent the mount being created when the feature is disabled
is addressed by the transport init function now trying to open the
transport and generate an initial connection. This fails when the HGFS
server is disabled because the feature is disabled. Hence, there will not
be a mount made and an error is reported to users.

If a mount is created when the feature is enabled and then later disabled
via the UI settings, the mount will still exist as there is no umount made
at that point. This will be addressed in later fixes. However, it should be
noted that the leftover mount which is now not accessible reports that the
"Transport endpoint is not connected" instead of the previous error of
"protocol error".

diff --git a/open-vm-tools/vmhgfs-fuse/bdhandler.c b/open-vm-tools/vmhgfs-fuse/bdhandler.c
index 62c489fd725b6fd756ec793d891551269ae6a5de..968e52af86500a69d51f770a1da04d8565f24ac3 100644 (file)
--- a/open-vm-tools/vmhgfs-fuse/bdhandler.c
+++ b/open-vm-tools/vmhgfs-fuse/bdhandler.c
@@ -45,7 +45,7 @@ static HgfsTransportChannel bdChannel;
  *      Open the backdoor in an idempotent way.
  *
  * Results:
- *      TRUE on success, FALSE on failure.
+ *      Existing or updated channel status, HGFS_CHANNEL_CONNECTED on success.
  *
  * Side effects:
  *      None
@@ -53,36 +53,35 @@ static HgfsTransportChannel bdChannel;
  *-----------------------------------------------------------------------------
  */
 
-static Bool
+static HgfsChannelStatus
 HgfsBdChannelOpen(HgfsTransportChannel *channel) // IN: Channel
 {
-   Bool ret;
-
    pthread_mutex_lock(&channel->connLock);
    switch (channel->status) {
    case HGFS_CHANNEL_UNINITIALIZED:
-      ret = FALSE;
+      LOG(8, ("Backdoor uninitialized.\n"));
       break;
    case HGFS_CHANNEL_CONNECTED:
-      ret = TRUE;
+      LOG(8, ("Backdoor already connected.\n"));
       break;
    case HGFS_CHANNEL_NOTCONNECTED:
       if (HgfsBd_OpenBackdoor((RpcOut **)&channel->priv)) {
-         LOG(8, ("Backdoor opened.\n"));
-         bdChannel.status = HGFS_CHANNEL_CONNECTED;
-         ret = TRUE;
+         LOG(8, ("Backdoor opened and connected.\n"));
+         channel->status = HGFS_CHANNEL_CONNECTED;
          ASSERT(channel->priv != NULL);
       } else {
-         ret = FALSE;
+         LOG(8, ("ERROR: Backdoor cannot connect.\n"));
       }
       break;
    default:
       ASSERT(0); /* Not reached. */
-      ret = FALSE;
+      LOG(2, ("ERROR: Backdoor status %d is unknown resetting.\n",
+              channel->status));
+      channel->status = HGFS_CHANNEL_UNINITIALIZED;
    }
 
    pthread_mutex_unlock(&channel->connLock);
-   return ret;
+   return channel->status;
 }
 
 

diff --git a/open-vm-tools/vmhgfs-fuse/main.c b/open-vm-tools/vmhgfs-fuse/main.c
index 241ce68a71c37bb00d21a9b6de98f127821addf7..26698b690545ca29f8fc94c819cef38607e632d7 100644 (file)
--- a/open-vm-tools/vmhgfs-fuse/main.c
+++ b/open-vm-tools/vmhgfs-fuse/main.c
@@ -1371,7 +1371,7 @@ main(int argc,       //IN: Argument count
    HgfsResetOps();
    res = HgfsTransportInit();
    if (res != 0) {
-      LOG(4, ("Main: Error in HgfsTransportInit %d\n", res));
+      fprintf(stderr, "Error %d cannot open connection!\n", res);
       return res;
    }
    HgfsInitCache();

diff --git a/open-vm-tools/vmhgfs-fuse/transport.c b/open-vm-tools/vmhgfs-fuse/transport.c
index 4878d3689da1ab15a4c3a011c47a4ac51a9dfe36..9f3f4325bc3e1ec453623a975811fadfd859cdd3 100644 (file)
--- a/open-vm-tools/vmhgfs-fuse/transport.c
+++ b/open-vm-tools/vmhgfs-fuse/transport.c
@@ -40,9 +40,11 @@
 
 static HgfsTransportChannel *gHgfsActiveChannel;     /* Current active channel. */
 static pthread_mutex_t gHgfsActiveChannelLock;       /* Current active channel lock. */
+static Bool gHgfsActiveChannelLockInited;
 
 static struct list_head gHgfsPendingRequests;        /* Pending requests queue. */
 static pthread_mutex_t gHgfsPendingRequestsLock;     /* Pending requests queue lock. */
+static Bool gHgfsPendingRequestsLockInited;
 
 
 #define HgfsRequestId(req) ((HgfsRequest *)req)->id
@@ -61,7 +63,7 @@ static void HgfsTransportChannelClose(HgfsTransportChannel **channel);
  *     Open a new workable channel.
  *
  * Results:
- *     TRUE on success and the new channel, otherwise FALSE and NULL.
+ *     0 on success and the new channel, otherwise -ENOTCONN and NULL.
  *
  * Side effects:
  *     None
@@ -69,17 +71,18 @@ static void HgfsTransportChannelClose(HgfsTransportChannel **channel);
  *----------------------------------------------------------------------
  */
 
-static Bool
+static int
 HgfsTransportChannelOpen(HgfsTransportChannel **channel) // IN: active channel
 {
-   Bool result = FALSE;
+   int result = 0;
 
    *channel = HgfsBdChannelInit();
    if (NULL != *channel) {
-      if ((*channel)->ops.open(*channel)) {
-         result = TRUE;
-      } else {
+      HgfsChannelStatus status = (*channel)->ops.open(*channel);
+      if (status != HGFS_CHANNEL_CONNECTED) {
          HgfsTransportChannelClose(channel);
+         result = -ENOTCONN;
+         *channel = NULL;
       }
    }
 
@@ -138,9 +141,14 @@ static Bool
 HgfsTransportChannelReset(HgfsTransportChannel **channel) // IN: active channel
 {
    Bool ret = FALSE;
+   int openResult;
+
    HgfsTransportChannelClose(channel);
-   ret = HgfsTransportChannelOpen(channel);
-   LOG(8, ("Result: %s.\n", ret ? "TRUE" : "FALSE"));
+   openResult = HgfsTransportChannelOpen(channel);
+   if (openResult == 0) {
+      ret = TRUE;
+   }
+   LOG(8, ("Result: %d: %s.\n",openResult, ret ? "TRUE" : "FALSE"));
    return ret;
 }
 
@@ -321,10 +329,11 @@ HgfsTransportSendRequest(HgfsReq *req)   // IN: Request to send
    pthread_mutex_lock(&gHgfsActiveChannelLock);
 
    /* Try opening the channel. */
-   if (NULL == gHgfsActiveChannel &&
-       !HgfsTransportChannelOpen(&gHgfsActiveChannel)) {
-      pthread_mutex_unlock(&gHgfsActiveChannelLock);
-      return -EPROTO;
+   if (NULL == gHgfsActiveChannel) {
+      ret = HgfsTransportChannelOpen(&gHgfsActiveChannel);
+      if (ret != 0) {
+         goto exit;
+      }
    }
 
    ASSERT(gHgfsActiveChannel->ops.send);
@@ -335,12 +344,12 @@ HgfsTransportSendRequest(HgfsReq *req)   // IN: Request to send
    if (ret < 0) {
       LOG(4, ("Send failed, status = %d. Try reopening the channel ...\n",
               ret));
-      if (gHgfsActiveChannel->ops.open(gHgfsActiveChannel) &&
-          HgfsTransportChannelReset(&gHgfsActiveChannel)) {
+      if (HgfsTransportChannelReset(&gHgfsActiveChannel)) {
          ret = gHgfsActiveChannel->ops.send(gHgfsActiveChannel, req);
       }
    }
 
+exit:
    ASSERT(req->state == HGFS_REQ_STATE_COMPLETED ||
           req->state == HGFS_REQ_STATE_SUBMITTED ||
           req->state == HGFS_REQ_STATE_UNSENT);
@@ -366,7 +375,7 @@ HgfsTransportSendRequest(HgfsReq *req)   // IN: Request to send
  *     connected socket.
  *
  * Results:
- *     Zero on success and negative one on failure.
+ *     Zero on success and negative error on failure.
  *
  * Side effects:
  *     None
@@ -378,17 +387,33 @@ int
 HgfsTransportInit(void)
 {
    int res;
+
+   gHgfsActiveChannel = NULL;
+   gHgfsPendingRequestsLockInited = FALSE;
+   gHgfsActiveChannelLockInited = FALSE;
    INIT_LIST_HEAD(&gHgfsPendingRequests);
+
    res = pthread_mutex_init(&gHgfsPendingRequestsLock, NULL);
-   if( res != 0) {
-      return -1;
+   if (res != 0) {
+      res = -res;
+      goto exit;
    }
+   gHgfsPendingRequestsLockInited = TRUE;
+
    res = pthread_mutex_init(&gHgfsActiveChannelLock, NULL);
-   if( res != 0) {
-      return -1;
+   if (res != 0) {
+      res = -res;
+      goto exit;
    }
-   gHgfsActiveChannel = NULL;
-   return 0;
+   gHgfsActiveChannelLockInited = TRUE;
+
+   res = HgfsTransportChannelOpen(&gHgfsActiveChannel);
+
+exit:
+   if (res != 0) {
+      HgfsTransportExit();
+   }
+   return res;
 }
 
 
@@ -412,10 +437,21 @@ void
 HgfsTransportExit(void)
 {
    LOG(8, ("Entered.\n"));
-   pthread_mutex_lock(&gHgfsActiveChannelLock);
-   HgfsTransportChannelClose(&gHgfsActiveChannel);
-   pthread_mutex_unlock(&gHgfsActiveChannelLock);
+
+   if (gHgfsActiveChannelLockInited) {
+      pthread_mutex_lock(&gHgfsActiveChannelLock);
+      HgfsTransportChannelClose(&gHgfsActiveChannel);
+      pthread_mutex_unlock(&gHgfsActiveChannelLock);
+
+      pthread_mutex_destroy(&gHgfsActiveChannelLock);
+      gHgfsActiveChannelLockInited = FALSE;
+   }
 
    ASSERT(list_empty(&gHgfsPendingRequests));
+
+   if (gHgfsPendingRequestsLockInited) {
+      pthread_mutex_destroy(&gHgfsPendingRequestsLock);
+      gHgfsPendingRequestsLockInited = FALSE;
+   }
    LOG(8, ("Exited.\n"));
 }

diff --git a/open-vm-tools/vmhgfs-fuse/transport.h b/open-vm-tools/vmhgfs-fuse/transport.h
index f623fe18c21e8df84614a035e0bb64843e36f6b8..e8d92e05be611612ea3647cd458c1e9aef0c32e3 100644 (file)
--- a/open-vm-tools/vmhgfs-fuse/transport.h
+++ b/open-vm-tools/vmhgfs-fuse/transport.h
@@ -26,24 +26,24 @@
 #include "request.h"
 #include <pthread.h>
 
+typedef enum {
+   HGFS_CHANNEL_UNINITIALIZED,
+   HGFS_CHANNEL_NOTCONNECTED,
+   HGFS_CHANNEL_CONNECTED,
+} HgfsChannelStatus;
+
 /*
  * There are the operations a channel should implement.
  */
 struct HgfsTransportChannel;
 typedef struct HgfsTransportChannelOps {
-   Bool (*open)(struct HgfsTransportChannel *);
+   HgfsChannelStatus (*open)(struct HgfsTransportChannel *);
    void (*close)(struct HgfsTransportChannel *);
    int (*send)(struct HgfsTransportChannel *, HgfsReq *);
    int (*recv)(struct HgfsTransportChannel *, char **, size_t *);
    void (*exit)(struct HgfsTransportChannel *);
 } HgfsTransportChannelOps;
 
-typedef enum {
-   HGFS_CHANNEL_UNINITIALIZED,
-   HGFS_CHANNEL_NOTCONNECTED,
-   HGFS_CHANNEL_CONNECTED,
-} HgfsChannelStatus;
-
 typedef struct HgfsTransportChannel {
    const char *name;               /* Channel name. */
    HgfsTransportChannelOps ops;    /* Channel ops. */