File no_manifest_on_aborted_snapshot.patch of Package open-vm-tools

Overview Repositories Revisions Requests Users Attributes Meta

File no_manifest_on_aborted_snapshot.patch of Package open-vm-tools (Revision 4ccabd5d0986b927ca487e3064a718d5)

Currently displaying revision 4ccabd5d0986b927ca487e3064a718d5 , Show latest

commit a1306fcbb6de6eae5344d5d74747068ea89aa5fc
Author: Oliver Kurth <okurth@vmware.com>
Date:   Tue Jan 29 14:03:19 2019 -0800

Don't send a backup manifest when aborting a Linux quiesced snapshot.
    
    When taking a Linux quiesced snapshot, communication failures between
    VMX and VMTools may result in VMTools sending a genericManifest event
    message after the quiesced snapshot operation has been aborted.  If
    this happens, VMX will send an error back to VMTools, which in turn
    causes VMTools not to send genericManifest messages on subsequent
    quiesced snapshots even if the host supports such messages.
    
    One aspect of the implementation that gives rise to this behavior is
    the use of the sync provider's snapshotDone function to undo a
    quiescing operation.  Specifically, if VMTools aborts a quiesced
    snapshot when the file system is quiesced, the quiescing must be
    undone.  Currently, this is handled by calling the sync provider's
    snapshotDone function.  This is the same function that is called to
    complete the quiescing snapshot protocol when it is successful.  In
    some respects this makes sense, since in either case snapshotDone
    unquiesces the file system.  However, architecturally and conceptually,
    it seems useful to distinguish between the action to be taken in the
    successful case versus the aborting case.  It's also useful to do so
    in practice, because the successful case sends the genericManifest
    event to notify the host there is a backup manifest file, while the
    aborting case should not do that.
    
    To address the issue, add an "undo" function for the Linux sync
    provider.  The undo function is called instead of snapshotDone as
    part of aborting a quiesced snapshot in which the file system is
    quiesced at the time of the abort.

diff --git a/open-vm-tools/services/plugins/vmbackup/stateMachine.c b/open-vm-tools/services/plugins/vmbackup/stateMachine.c
index ebeaf42b..14d08a77 100644
--- a/open-vm-tools/services/plugins/vmbackup/stateMachine.c
+++ b/open-vm-tools/services/plugins/vmbackup/stateMachine.c
@@ -453,12 +453,13 @@ VmBackupDoAbort(void)
       g_static_mutex_unlock(&gBackupState->opLock);
 
 #ifdef __linux__
-      /* Thaw the guest if already quiesced */
+      /* If quiescing has been completed, then undo it.  */
       if (gBackupState->machineState == VMBACKUP_MSTATE_SYNC_FREEZE) {
-         g_debug("Guest already quiesced, thawing for abort\n");
-         if (!gBackupState->provider->snapshotDone(gBackupState,
+         g_debug("Aborting with file system already quiesced, undo quiescing "
+                 "operation.\n");
+         if (!gBackupState->provider->undo(gBackupState,
                                       gBackupState->provider->clientData)) {
-            g_debug("Thaw during abort failed\n");
+            g_debug("Quiescing undo failed.\n");
             eventMsg = "Quiesce could not be aborted.";
          }
       }
diff --git a/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c b/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c
index 9f584443..1dd98e32 100644
--- a/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c
+++ b/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c
@@ -35,16 +35,53 @@
 #include <process.h>
 #endif
 
+/*
+ * Define an enumeration type VmBackupOpType and a corresponding array
+ * VmBackupOpName whose entries provide the printable names of the
+ * enumeration ids in VmBackupOpType.
+ *
+ * VmBackupOpType and VmBackupOpName are each defined as an invocation
+ * of a macro VMBACKUP_OPLIST.  VMBACKUP_OPLIST specifies a list of
+ * enumeration ids using a macro VMBACKUP_OP that must be defined before
+ * invoking VMBACKUP_OPLIST.  VMBACKUP_OP takes a single argument, which
+ * should be an enumeration id, and is defined to generate from the id
+ * either the id itself or a string to be used as its printable name.  The
+ * result is that an invocation of VMBACKUP_OPLIST generates either the
+ * list of enumeration ids or the list of their printable names.
+ */
+#define VMBACKUP_OPLIST         \
+    VMBACKUP_OP(OP_FREEZE),     \
+    VMBACKUP_OP(OP_THAW),       \
+    VMBACKUP_OP(OP_UNDO),
+
+#define VMBACKUP_OPID(id)       id
+#define VMBACKUP_OPNAME(id)     #id
+
+#undef VMBACKUP_OP
+#define VMBACKUP_OP(id)       VMBACKUP_OPID(id)
+
+typedef enum {
+   VMBACKUP_OPLIST
+} VmBackupOpType;
+
+#undef VMBACKUP_OP
+#define VMBACKUP_OP(id)       VMBACKUP_OPNAME(id)
+
+static const char *VmBackupOpName[] = {
+   VMBACKUP_OPLIST
+};
+
+#undef VMBACKUP_OP
+
 typedef struct VmBackupDriverOp {
    VmBackupOp callbacks;
    const char *volumes;
-   Bool freeze;
+   VmBackupOpType opType;
    Bool canceled;
    SyncDriverHandle *syncHandle;
    SyncManifest *manifest;
 } VmBackupDriverOp;
 
-
 /*
  *-----------------------------------------------------------------------------
  *
@@ -97,7 +134,7 @@ VmBackupDriverOpQuery(VmBackupOp *_op) // IN
    VmBackupDriverOp *op = (VmBackupDriverOp *) _op;
    VmBackupOpStatus ret;
 
-   if (op->freeze) {
+   if (op->opType == OP_FREEZE) {
       SyncDriverStatus st = SyncDriver_QueryStatus(*op->syncHandle, 0);
 
       g_debug("SyncDriver status: %d\n", st);
@@ -208,7 +245,7 @@ VmBackupDriverOpCancel(VmBackupOp *_op)   // IN
 
 static VmBackupDriverOp *
 VmBackupNewDriverOp(VmBackupState *state,       // IN
-                    Bool freeze,                // IN
+                    VmBackupOpType opType,      // IN
                     SyncDriverHandle *handle,   // IN
                     const char *volumes,        // IN
                     Bool useNullDriverPrefs)    // IN
@@ -216,8 +253,9 @@ VmBackupNewDriverOp(VmBackupState *state,       // IN
    Bool success;
    VmBackupDriverOp *op = NULL;
 
-   g_return_val_if_fail((handle == NULL || *handle == SYNCDRIVER_INVALID_HANDLE) ||
-                        !freeze,
+   g_return_val_if_fail((handle == NULL ||
+                         *handle == SYNCDRIVER_INVALID_HANDLE) ||
+                        opType != OP_FREEZE,
                         NULL);
 
    op = Util_SafeMalloc(sizeof *op);
@@ -226,24 +264,32 @@ VmBackupNewDriverOp(VmBackupState *state,       // IN
    op->callbacks.queryFn = VmBackupDriverOpQuery;
    op->callbacks.cancelFn = VmBackupDriverOpCancel;
    op->callbacks.releaseFn = VmBackupDriverOpRelease;
-   op->freeze = freeze;
+   op->opType = opType;
    op->volumes = volumes;
 
    op->syncHandle = g_new0(SyncDriverHandle, 1);
    *op->syncHandle = (handle != NULL) ? *handle : SYNCDRIVER_INVALID_HANDLE;
 
-   if (freeze) {
-      success = SyncDriver_Freeze(op->volumes,
-                                  useNullDriverPrefs ?
-                                  state->enableNullDriver : FALSE,
-                                  op->syncHandle,
-                                  state->excludedFileSystems);
-   } else {
-      op->manifest = SyncNewManifest(state, *op->syncHandle);
-      success = VmBackupDriverThaw(op->syncHandle);
+   switch (opType) {
+      case OP_FREEZE:
+         success = SyncDriver_Freeze(op->volumes,
+                                     useNullDriverPrefs ?
+                                        state->enableNullDriver : FALSE,
+                                     op->syncHandle,
+                                     state->excludedFileSystems);
+         break;
+      case OP_THAW:
+         op->manifest = SyncNewManifest(state, *op->syncHandle);
+         success = VmBackupDriverThaw(op->syncHandle);
+         break;
+      default:
+         ASSERT(opType == OP_UNDO);
+         success = VmBackupDriverThaw(op->syncHandle);
+         break;
    }
    if (!success) {
-      g_warning("Error %s filesystems.", freeze ? "freezing" : "thawing");
+      g_warning("Error trying to perform %s on filesystems.",
+                VmBackupOpName[opType]);
       g_free(op->syncHandle);
       SyncManifestRelease(op->manifest);
       free(op);
@@ -329,7 +375,7 @@ VmBackupSyncDriverStart(VmBackupState *state,
    VmBackupDriverOp *op;
 
    g_debug("*** %s\n", __FUNCTION__);
-   op = VmBackupNewDriverOp(state, TRUE, NULL, state->volumes, TRUE);
+   op = VmBackupNewDriverOp(state, OP_FREEZE, NULL, state->volumes, TRUE);
 
    if (op != NULL) {
       state->clientData = op->syncHandle;
@@ -366,7 +412,7 @@ VmBackupSyncDriverOnlyStart(VmBackupState *state,
    VmBackupDriverOp *op;
 
    g_debug("*** %s\n", __FUNCTION__);
-   op = VmBackupNewDriverOp(state, TRUE, NULL, state->volumes, FALSE);
+   op = VmBackupNewDriverOp(state, OP_FREEZE, NULL, state->volumes, FALSE);
 
    if (op != NULL) {
       state->clientData = op->syncHandle;
@@ -404,7 +450,7 @@ VmBackupSyncDriverStart(ToolsAppCtx *ctx,
    VmBackupState *state = (VmBackupState*) clientData;
 
    g_debug("*** %s\n", __FUNCTION__);
-   op = VmBackupNewDriverOp(state, TRUE, NULL, state->volumes, TRUE);
+   op = VmBackupNewDriverOp(state, OP_FREEZE, NULL, state->volumes, TRUE);
 
    if (op != NULL) {
       state->clientData = op->syncHandle;
@@ -442,7 +488,7 @@ VmBackupSyncDriverOnlyStart(ToolsAppCtx *ctx,
    VmBackupState *state = (VmBackupState*) clientData;
 
    g_debug("*** %s\n", __FUNCTION__);
-   op = VmBackupNewDriverOp(state, TRUE, NULL, state->volumes, FALSE);
+   op = VmBackupNewDriverOp(state, OP_FREEZE, NULL, state->volumes, FALSE);
 
    if (op != NULL) {
       state->clientData = op->syncHandle;
@@ -480,7 +526,7 @@ VmBackupSyncDriverSnapshotDone(VmBackupState *state,
 
    g_debug("*** %s\n", __FUNCTION__);
 
-   op = VmBackupNewDriverOp(state, FALSE, state->clientData, NULL, TRUE);
+   op = VmBackupNewDriverOp(state, OP_THAW, state->clientData, NULL, TRUE);
    g_free(state->clientData);
    state->clientData = NULL;
 
@@ -513,12 +559,78 @@ VmBackupSyncDriverOnlySnapshotDone(VmBackupState *state,
 
    g_debug("*** %s\n", __FUNCTION__);
 
-   op = VmBackupNewDriverOp(state, FALSE, state->clientData, NULL, FALSE);
+   op = VmBackupNewDriverOp(state, OP_THAW, state->clientData, NULL, FALSE);
+   g_free(state->clientData);
+   state->clientData = NULL;
+
+   return VmBackup_SetCurrentOp(state, (VmBackupOp *) op, NULL, __FUNCTION__);
+}
+
+
+#if defined(__linux__)
+/*
+ *-----------------------------------------------------------------------------
+ *
+ *  VmBackupSyncDriverUndo --
+ *
+ *    Undo a completed quiescing operation.
+ *
+ * Result
+ *    TRUE, unless an error occurs.
+ *
+ * Side effects:
+ *    None.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+static Bool
+VmBackupSyncDriverUndo(VmBackupState *state,
+                       void *clientData)
+{
+   VmBackupDriverOp *op;
+
+   g_debug("*** %s\n", __FUNCTION__);
+
+   op = VmBackupNewDriverOp(state, OP_UNDO, state->clientData, NULL, TRUE);
+   g_free(state->clientData);
+   state->clientData = NULL;
+
+   return VmBackup_SetCurrentOp(state, (VmBackupOp *) op, NULL, __FUNCTION__);
+}
+
+
+/*
+ *-----------------------------------------------------------------------------
+ *
+ *  VmBackupSyncDriverOnlyUndo --
+ *
+ *    Undo a completed quiescing operation.
+ *
+ * Result
+ *    TRUE, unless an error occurs.
+ *
+ * Side effects:
+ *    None.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+static Bool
+VmBackupSyncDriverOnlyUndo(VmBackupState *state,
+                           void *clientData)
+{
+   VmBackupDriverOp *op;
+
+   g_debug("*** %s\n", __FUNCTION__);
+
+   op = VmBackupNewDriverOp(state, OP_UNDO, state->clientData, NULL, FALSE);
    g_free(state->clientData);
    state->clientData = NULL;
 
    return VmBackup_SetCurrentOp(state, (VmBackupOp *) op, NULL, __FUNCTION__);
 }
+#endif
 
 
 /*
@@ -579,10 +691,17 @@ VmBackup_NewSyncDriverProviderInternal(Bool useNullDriverPrefs)
    if (useNullDriverPrefs) {
       provider->start = VmBackupSyncDriverStart;
       provider->snapshotDone = VmBackupSyncDriverSnapshotDone;
+#if defined(__linux__)
+      provider->undo = VmBackupSyncDriverUndo;
+#endif
    } else {
       provider->start = VmBackupSyncDriverOnlyStart;
       provider->snapshotDone = VmBackupSyncDriverOnlySnapshotDone;
+#if defined(__linux__)
+      provider->undo = VmBackupSyncDriverOnlyUndo;
+#endif
    }
+
    provider->release = VmBackupSyncDriverRelease;
    provider->clientData = NULL;
 
diff --git a/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h b/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h
index 7b819ac1..ad3f2d7c 100644
--- a/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h
+++ b/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h
@@ -156,6 +156,7 @@ typedef struct VmBackupSyncProvider {
    VmBackupProviderCallback start;
 #else
    ToolsCorePoolCb start;
+   VmBackupProviderCallback undo;
 #endif
    VmBackupProviderCallback snapshotDone;
    void (*release)(struct VmBackupSyncProvider *);

Places

File no_manifest_on_aborted_snapshot.patch of Package open-vm-tools (Revision 4ccabd5d0986b927ca487e3064a718d5)

Places