Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
oliverkurth committed Feb 1, 2019
1 parent f2ff192 commit a1306fc
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 27 deletions.
9 changes: 5 additions & 4 deletions open-vm-tools/services/plugins/vmbackup/stateMachine.c
Expand Up @@ -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.";
}
}
Expand Down
165 changes: 142 additions & 23 deletions open-vm-tools/services/plugins/vmbackup/syncDriverOps.c
Expand Up @@ -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;


/*
*-----------------------------------------------------------------------------
*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -208,16 +245,17 @@ 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
{
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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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


/*
Expand Down Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions open-vm-tools/services/plugins/vmbackup/vmBackupInt.h
Expand Up @@ -156,6 +156,7 @@ typedef struct VmBackupSyncProvider {
VmBackupProviderCallback start;
#else
ToolsCorePoolCb start;
VmBackupProviderCallback undo;
#endif
VmBackupProviderCallback snapshotDone;
void (*release)(struct VmBackupSyncProvider *);
Expand Down

0 comments on commit a1306fc

Please sign in to comment.