From 062860c7b1c7611d70a777a96da72792f4ab8d8c2e7d2671ae9ff0c561aa0e02 Mon Sep 17 00:00:00 2001 From: Kirk Allan Date: Fri, 8 Feb 2019 22:36:39 +0000 Subject: [PATCH] Accepting request 672900 from home:kallan:branches:Virtualization:VMware - Link VGAuthService to libxmlsec1 rather than libxml-security-c in SLE products where available. (bsc#1122435) - Add patches to correct and/or improve handling of certain quiesced snapshot failures (bsc#1124397). + no_manifest_on_aborted_snapshot.patch Don't send a backup manifest when aborting a Linux quiesced snapshot. + vmtoolsd_bailout_on_rpc_errors.patch Bail out vmtoolsd early when there are RPC errors. + send_vmbackup_event_generic_manifest.patch Always send VMBACKUP_EVENT_GENERIC_MANIFEST during quiesced snapshots. + include_log_h_for_g_info.patch Include vmware/tools/log.h to define g_info. - Update vmtoolsd.service to support cloud-init customization by default by adding "DefaultDependencies=no" and "Before=cloud-init-local.service" to the [Unit] section of vmtoolsd.service (bsc#1121964) . - Copyright year updated in spec file. OBS-URL: https://build.opensuse.org/request/show/672900 OBS-URL: https://build.opensuse.org/package/show/Virtualization:VMware/open-vm-tools?expand=0&rev=366 --- include_log_h_for_g_info.patch | 25 ++ no_manifest_on_aborted_snapshot.patch | 344 +++++++++++++++++++++ open-vm-tools.changes | 25 ++ open-vm-tools.spec | 45 ++- send_vmbackup_event_generic_manifest.patch | 291 +++++++++++++++++ vmtoolsd.service | 2 + vmtoolsd_bailout_on_rpc_errors.patch | 128 ++++++++ 7 files changed, 845 insertions(+), 15 deletions(-) create mode 100644 include_log_h_for_g_info.patch create mode 100644 no_manifest_on_aborted_snapshot.patch create mode 100644 send_vmbackup_event_generic_manifest.patch create mode 100644 vmtoolsd_bailout_on_rpc_errors.patch diff --git a/include_log_h_for_g_info.patch b/include_log_h_for_g_info.patch new file mode 100644 index 0000000..c58d725 --- /dev/null +++ b/include_log_h_for_g_info.patch @@ -0,0 +1,25 @@ +commit eab7b622e798bfb01283ef1336b26eea93baf977 +Author: Oliver Kurth +Date: Tue Jan 29 17:24:44 2019 -0800 + + Include vmware/tools/log.h to define g_info. + + A recent change added a call to g_info from syncManifest.c. This + in turn is causing open-vm-tools builds to fail because g_info is + not available on SLES 12sp1. + + To fix the problem, include vmware/tools/log.h in syncManifest.c. + log.h defines g_info as a macro. + +diff --git a/open-vm-tools/services/plugins/vmbackup/syncManifest.c b/open-vm-tools/services/plugins/vmbackup/syncManifest.c +index 4586c4c3..ddc99590 100644 +--- a/open-vm-tools/services/plugins/vmbackup/syncManifest.c ++++ b/open-vm-tools/services/plugins/vmbackup/syncManifest.c +@@ -27,6 +27,7 @@ + #include "syncDriver.h" + #include "syncManifest.h" + #include "vm_tools_version.h" ++#include "vmware/tools/log.h" + + #include + #include diff --git a/no_manifest_on_aborted_snapshot.patch b/no_manifest_on_aborted_snapshot.patch new file mode 100644 index 0000000..d69058f --- /dev/null +++ b/no_manifest_on_aborted_snapshot.patch @@ -0,0 +1,344 @@ +commit a1306fcbb6de6eae5344d5d74747068ea89aa5fc +Author: Oliver Kurth +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 + #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 *); diff --git a/open-vm-tools.changes b/open-vm-tools.changes index 2a39a92..a8c8ea2 100644 --- a/open-vm-tools.changes +++ b/open-vm-tools.changes @@ -1,3 +1,28 @@ +------------------------------------------------------------------- +Fri Feb 8 15:30:25 UTC 2019 - kallan@suse.com + +- Link VGAuthService to libxmlsec1 rather than libxml-security-c in SLE + products where available. (bsc#1122435) + +- Add patches to correct and/or improve handling of certain quiesced snapshot + failures (bsc#1124397). + + no_manifest_on_aborted_snapshot.patch + Don't send a backup manifest when aborting a Linux quiesced snapshot. + + vmtoolsd_bailout_on_rpc_errors.patch + Bail out vmtoolsd early when there are RPC errors. + + send_vmbackup_event_generic_manifest.patch + Always send VMBACKUP_EVENT_GENERIC_MANIFEST during quiesced snapshots. + + include_log_h_for_g_info.patch + Include vmware/tools/log.h to define g_info. + +------------------------------------------------------------------- +Tue Jan 29 21:43:09 UTC 2019 - kallan@suse.com + +- Update vmtoolsd.service to support cloud-init customization by default + by adding "DefaultDependencies=no" and "Before=cloud-init-local.service" + to the [Unit] section of vmtoolsd.service (bsc#1121964) . +- Copyright year updated in spec file. + ------------------------------------------------------------------- Fri Nov 9 15:40:16 UTC 2018 - kallan@suse.com diff --git a/open-vm-tools.spec b/open-vm-tools.spec index 7a3c65d..d422c05 100644 --- a/open-vm-tools.spec +++ b/open-vm-tools.spec @@ -1,7 +1,7 @@ # # spec file for package open-vm-tools # -# Copyright (c) 2018 SUSE LINUX GmbH, Nuernberg, Germany. +# Copyright (c) 2019 SUSE LINUX GmbH, Nuernberg, Germany. # Copyright (c) 2010 Dominique Leuenberger, Amsterdam, Netherlands. # # All modifications and additions to the file contributed by third parties @@ -114,15 +114,33 @@ BuildRequires: glibc >= 2.12 BuildRequires: xorg-x11-devel %endif %if %{with vgauth} -# vgauth requires xml2, xerces-c, and xml-security-c +# vgauth requires xml2, xerces-c, and xml-security-c/xmlsec1 BuildRequires: libxml2-devel -%if 0%{?suse_version } > 1500 || ( 0%{?is_opensuse} && 0%{?sle_version} >= 0120100 ) +BuildRequires: pkgconfig(xerces-c) + # Use xmlsec1 instead of xml-security-c where available. +# If using xmlsec1, also need to use libxmlsec1-openssl1 +%if 0%{?suse_version } > 1500 BuildRequires: pkgconfig(xmlsec1) +Requires: libxmlsec1-openssl1 +%else +%if ( 0%{?is_opensuse} && 0%{?sle_version} >= 0120100 ) +BuildRequires: pkgconfig(xmlsec1) +Requires: libxmlsec1-openssl1 +%else +%if 0%{?sle_version} == 120400 && !0%{?is_opensuse} +BuildRequires: pkgconfig(xmlsec1) +Requires: libxmlsec1-openssl1 %else BuildRequires: xml-security-c-devel +%define arg_xmlsecurity --enable-xmlsecurity %endif -BuildRequires: pkgconfig(xerces-c) +%endif +%endif +%else +# not using vgauth +%define arg_xmlsecurity --without-xmlsecurity +%define arg_xerces --without-xerces %endif # vmhgfs is always built so fuse is no longer optional BuildRequires: fuse-devel @@ -136,9 +154,6 @@ Requires: libvmtools0 = %{version}-%{release} Requires: net-tools Requires: tar Requires: which -%if 0%{?suse_version } > 1500 || ( 0%{?is_opensuse} && 0%{?sle_version} >= 0120100 ) -Requires: libxmlsec1-openssl1 -%endif # open-vm-tools >= 10.0.0 do not require open-vm-tools-deploypkg # provided by VMware. That functionality is now available as part # of open-vm-tools package itself. @@ -146,6 +161,10 @@ Obsoletes: open-vm-tools-deploypkg <= 10.0.5 Supplements: modalias(pci:v000015ADd*sv*sd*bc*sc*i*) ExclusiveArch: %ix86 x86_64 #Upstream patches +Patch0: no_manifest_on_aborted_snapshot.patch +Patch1: vmtoolsd_bailout_on_rpc_errors.patch +Patch2: send_vmbackup_event_generic_manifest.patch +Patch3: include_log_h_for_g_info.patch %systemd_requires @@ -212,6 +231,10 @@ if you intend to create own plugins for vmtoolsd. # fix for an rpmlint warning regarding wrong line feeds sed -i -e "s/\r//" README #Upstream patches +%patch0 -p2 +%patch1 -p2 +%patch2 -p2 +%patch3 -p2 %build %if %{with_X} @@ -219,14 +242,6 @@ sed -i -e "s/\r//" README %else %define arg_x --without-x %endif -%if ! %{with vgauth} - %define arg_xmlsecurity --without-xmlsecurity - %define arg_xerces --without-xerces -%else -%if !0%{?is_opensuse} - %define arg_xmlsecurity --enable-xmlsecurity -%endif -%endif # disable warning unused-but-set-variable which will raise error because of -Werror # disable warning deprecated-declarations which will raise error because of -Werror diff --git a/send_vmbackup_event_generic_manifest.patch b/send_vmbackup_event_generic_manifest.patch new file mode 100644 index 0000000..196975d --- /dev/null +++ b/send_vmbackup_event_generic_manifest.patch @@ -0,0 +1,291 @@ +commit c31710b3942f48b1c11ebde36f34e7e159d1cbf0 +Author: Oliver Kurth +Date: Tue Jan 29 17:24:44 2019 -0800 + + Always send VMBACKUP_EVENT_GENERIC_MANIFEST during quiesced snapshots. + + vSphere 6.7 added a host-side interface that allows VMTools to send + a "generic" backup manifest during a quiesced snapshot on Linux guests. + VMTools 10.2.0 or later tries to notify the host of the backup manifest + file through a vmbackup event message VMBACKUP_EVENT_GENERIC_MANIFEST. + If the host is unable to field the message, then VMTools logs the + failure and then continues with the quiesced snapshot in the older + fashion, without the backup manifest. + + An earlier change attempted to reduce the amount of logging done when + running on older hosts that don't support VMBACKUP_EVENT_GENERIC_MANIFEST + by detecting when sending VMBACKUP_EVENT_GENERIC_MANIFEST fails and + not sending the message again for subsequent quiesced snapshots. + However, subsequent stress testing has uncovered problems with this + approach when running on newer hosts; specifically, errors may sometimes + be encountered on newer hosts when sending VMBACKUP_EVENT_GENERIC_MANIFEST. + Therefore this change backs out that earlier change. + + Note that the need to solve the problem that that earlier change was + intended to solve has been reduced because support for + VMBACKUP_EVENT_GENERIC_MANIFEST has been backported to vSphere 6.5 + P03, which is available, and vSphere 6.0 P08, which is scheduled for + release later this year. ESXi 5.5 is out of general support. + + This change also addresses an issue that surfaced when testing on a + host without support for VMBACKUP_EVENT_GENERIC_MANIFEST. + If VMTools fails to send VMBACKUP_EVENT_GENERIC_MANIFEST, the quiesced + snapshot operation will be aborted rather than continuing as it should. + To address this, create a new function, VmBackup_SendEventNoAbort, + which does not abort the quiesced snapshot on failure, and call that + function rather than VmBackup_SendEvent when sending + VMBACKUP_EVENT_GENERIC_MANIFEST. + +diff --git a/open-vm-tools/services/plugins/vmbackup/stateMachine.c b/open-vm-tools/services/plugins/vmbackup/stateMachine.c +index 28118172..d38187d1 100644 +--- a/open-vm-tools/services/plugins/vmbackup/stateMachine.c ++++ b/open-vm-tools/services/plugins/vmbackup/stateMachine.c +@@ -201,17 +201,19 @@ VmBackupPrivSendMsg(gchar *msg, + * Sends a command to the VMX asking it to update VMDB about a new backup event. + * This will restart the keep-alive timer. + * ++ * As the name implies, does not abort the quiesce operation on failure. ++ * + * @param[in] event The event to set. + * @param[in] code Error code. +- * @param[in] dest Error description. ++ * @param[in] desc Error description. + * + * @return TRUE on success. + */ + + Bool +-VmBackup_SendEvent(const char *event, +- const uint32 code, +- const char *desc) ++VmBackup_SendEventNoAbort(const char *event, ++ const uint32 code, ++ const char *desc) + { + Bool success; + char *result = NULL; +@@ -280,11 +282,6 @@ VmBackup_SendEvent(const char *event, + } else { + g_warning("Failed to send vmbackup event: %s, result: %s.\n", + msg, result); +- if (gBackupState->rpcState != VMBACKUP_RPC_STATE_IGNORE) { +- g_debug("Changing rpcState from %d to %d\n", +- gBackupState->rpcState, VMBACKUP_RPC_STATE_ERROR); +- gBackupState->rpcState = VMBACKUP_RPC_STATE_ERROR; +- } + } + vm_free(result); + g_free(msg); +@@ -293,6 +290,36 @@ VmBackup_SendEvent(const char *event, + } + + ++/** ++ * Sends a command to the VMX asking it to update VMDB about a new backup event. ++ * This will restart the keep-alive timer. ++ * ++ * Aborts the quiesce operation on RPC failure. ++ * ++ * @param[in] event The event to set. ++ * @param[in] code Error code. ++ * @param[in] desc Error description. ++ * ++ * @return TRUE on success. ++ */ ++ ++Bool ++VmBackup_SendEvent(const char *event, ++ const uint32 code, ++ const char *desc) ++{ ++ Bool success = VmBackup_SendEventNoAbort(event, code, desc); ++ ++ if (!success && gBackupState->rpcState != VMBACKUP_RPC_STATE_IGNORE) { ++ g_debug("Changing rpcState from %d to %d\n", ++ gBackupState->rpcState, VMBACKUP_RPC_STATE_ERROR); ++ gBackupState->rpcState = VMBACKUP_RPC_STATE_ERROR; ++ } ++ ++ return success; ++} ++ ++ + /** + * Cleans up the backup state object and sends a "done" event to the VMX. + */ +@@ -1361,7 +1388,7 @@ VmBackupDumpState(gpointer src, + + + /** +- * Reset callback. ++ * Reset callback. Currently does nothing. + * + * @param[in] src The source object. Unused. + * @param[in] ctx Unused. +@@ -1373,7 +1400,7 @@ VmBackupReset(gpointer src, + ToolsAppCtx *ctx, + gpointer data) + { +- VmBackup_SyncDriverReset(); ++ + } + + +diff --git a/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c b/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c +index 1dd98e32..d659f1b4 100644 +--- a/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c ++++ b/open-vm-tools/services/plugins/vmbackup/syncDriverOps.c +@@ -761,26 +761,3 @@ VmBackup_NewSyncDriverOnlyProvider(void) + } + + #endif +- +- +-/* +- *----------------------------------------------------------------------------- +- * +- * VmBackup_SyncDriverReset -- +- * +- * Reset function +- * +- * Results: +- * None. +- * +- * Side effects: +- * Whatever are the side effects of what it calls. +- * +- *----------------------------------------------------------------------------- +- */ +- +-void +-VmBackup_SyncDriverReset(void) +-{ +- SyncManifestReset(); +-} +diff --git a/open-vm-tools/services/plugins/vmbackup/syncManifest.c b/open-vm-tools/services/plugins/vmbackup/syncManifest.c +index 224f7e8a..4586c4c3 100644 +--- a/open-vm-tools/services/plugins/vmbackup/syncManifest.c ++++ b/open-vm-tools/services/plugins/vmbackup/syncManifest.c +@@ -1,5 +1,5 @@ + /********************************************************* +- * Copyright (C) 2017-2018 VMware, Inc. All rights reserved. ++ * Copyright (C) 2017-2019 VMware, Inc. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published +@@ -49,12 +49,6 @@ static const char syncManifestFmt[] = { + */ + static const char syncManifestSwitch[] = "enableXmlManifest"; + +-/* +- * If TRUE, indicates that VMTools should try to generate the backup +- * manifest and send it to VMX; if FALSE, it won't try to do so. +- */ +-static Bool gSyncManifestTrySend = TRUE; +- + + /* + *----------------------------------------------------------------------------- +@@ -95,12 +89,6 @@ SyncNewManifest(VmBackupState *state, // IN + return NULL; + } + +- if (!gSyncManifestTrySend) { +- g_debug("No backup manifest generated since previous" +- " attempt to send one to host failed.\n"); +- return NULL; +- } +- + manifest = g_new0(SyncManifest, 1); + manifest->path = g_strdup_printf("%s/%s", state->configDir, + syncManifestName); +@@ -173,37 +161,14 @@ SyncManifestSend(SyncManifest *manifest) // IN + return FALSE; + } + +- if (!VmBackup_SendEvent(VMBACKUP_EVENT_GENERIC_MANIFEST, +- VMBACKUP_SUCCESS, manifest->path)) { +- g_warning("Host doesn't appear to support backup manifests " +- "for Linux guests.\n"); +- gSyncManifestTrySend = FALSE; ++ if (!VmBackup_SendEventNoAbort(VMBACKUP_EVENT_GENERIC_MANIFEST, ++ VMBACKUP_SUCCESS, manifest->path)) { ++ /* VmBackup_SendEventNoAbort logs the error */ ++ g_info("Non-fatal error occurred while sending %s, continuing " ++ "with the operation", VMBACKUP_EVENT_GENERIC_MANIFEST); + return FALSE; + } + + g_debug("Backup manifest was sent successfully.\n"); + return TRUE; + } +- +- +-/* +- *----------------------------------------------------------------------------- +- * +- * SyncManifestReset -- +- * +- * Reset SyncManifest global state +- * +- * Results: +- * None +- * +- * Side effects: +- * None +- * +- *----------------------------------------------------------------------------- +- */ +- +-void +-SyncManifestReset(void) +-{ +- gSyncManifestTrySend = TRUE; +-} +diff --git a/open-vm-tools/services/plugins/vmbackup/syncManifest.h b/open-vm-tools/services/plugins/vmbackup/syncManifest.h +index 9e4e9eb3..fd226adb 100644 +--- a/open-vm-tools/services/plugins/vmbackup/syncManifest.h ++++ b/open-vm-tools/services/plugins/vmbackup/syncManifest.h +@@ -1,5 +1,5 @@ + /********************************************************* +- * Copyright (C) 2017-2018 VMware, Inc. All rights reserved. ++ * Copyright (C) 2017-2019 VMware, Inc. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published +@@ -45,9 +45,6 @@ SyncManifestSend(SyncManifest *manifest); + void + SyncManifestRelease(SyncManifest *manifest); + +-void +-SyncManifestReset(void); +- + #else /* !defined(__linux__) */ + + typedef void SyncManifest; +@@ -55,7 +52,6 @@ typedef void SyncManifest; + #define SyncNewManifest(s, h) (NULL) + #define SyncManifestSend(m) (TRUE) + #define SyncManifestRelease(m) ASSERT(m == NULL) +-#define SyncManifestReset() + + #endif /* defined(__linux__) */ + +diff --git a/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h b/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h +index 4258ee0a..97fef214 100644 +--- a/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h ++++ b/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h +@@ -303,8 +303,11 @@ VmBackup_SendEvent(const char *event, + const uint32 code, + const char *desc); + +-void +-VmBackup_SyncDriverReset(void); ++ ++Bool ++VmBackup_SendEventNoAbort(const char *event, ++ const uint32 code, ++ const char *desc); + + #endif /* _VMBACKUPINT_H_*/ + diff --git a/vmtoolsd.service b/vmtoolsd.service index 2fdcd16..7cf2811 100644 --- a/vmtoolsd.service +++ b/vmtoolsd.service @@ -4,6 +4,8 @@ Documentation=http://github.com/vmware/open-vm-tools ConditionVirtualization=vmware Requires=vgauthd.service After=vgauthd.service +DefaultDependencies=no +Before=cloud-init-local.service [Service] Type=forking diff --git a/vmtoolsd_bailout_on_rpc_errors.patch b/vmtoolsd_bailout_on_rpc_errors.patch new file mode 100644 index 0000000..baf230a --- /dev/null +++ b/vmtoolsd_bailout_on_rpc_errors.patch @@ -0,0 +1,128 @@ +commit 0c9174716ba828899418ba07efc3aab0bff004cc +Author: Oliver Kurth +Date: Tue Jan 29 14:03:19 2019 -0800 + + Bail out vmtoolsd early when there are RPC errors. + + VMX state machine could give up quiescing operation for various + reasons when vmtoolsd is busy performing necessary state transitions. + Once VMX gives up quiescing operation, there is no point in + vmtoolsd continuing with it. vmtoolsd should also give up the + operation asap. vmtoolsd can detect VMX state machine change + when it gets errors sending VMBACKUP_PROTOCOL_EVENT_SET RPC. + + RPC errors are only used as a trigger to abort the operation. + We ignore the RPC errors that might occur after aborting the + operation. + +diff --git a/open-vm-tools/services/plugins/vmbackup/stateMachine.c b/open-vm-tools/services/plugins/vmbackup/stateMachine.c +index 14d08a77..28118172 100644 +--- a/open-vm-tools/services/plugins/vmbackup/stateMachine.c ++++ b/open-vm-tools/services/plugins/vmbackup/stateMachine.c +@@ -224,6 +224,7 @@ VmBackup_SendEvent(const char *event, + if (gBackupState->keepAlive != NULL) { + g_source_destroy(gBackupState->keepAlive); + g_source_unref(gBackupState->keepAlive); ++ gBackupState->keepAlive = NULL; + } + + msg = g_strdup_printf(VMBACKUP_PROTOCOL_EVENT_SET" %s %u %s", +@@ -267,19 +268,27 @@ VmBackup_SendEvent(const char *event, + &result, &resultLen); + #endif + +- if (!success) { ++ if (success) { ++ ASSERT(gBackupState->keepAlive == NULL); ++ gBackupState->keepAlive = ++ g_timeout_source_new(VMBACKUP_KEEP_ALIVE_PERIOD / 2); ++ VMTOOLSAPP_ATTACH_SOURCE(gBackupState->ctx, ++ gBackupState->keepAlive, ++ VmBackupKeepAliveCallback, ++ NULL, ++ NULL); ++ } else { + g_warning("Failed to send vmbackup event: %s, result: %s.\n", + msg, result); ++ if (gBackupState->rpcState != VMBACKUP_RPC_STATE_IGNORE) { ++ g_debug("Changing rpcState from %d to %d\n", ++ gBackupState->rpcState, VMBACKUP_RPC_STATE_ERROR); ++ gBackupState->rpcState = VMBACKUP_RPC_STATE_ERROR; ++ } + } + vm_free(result); + g_free(msg); + +- gBackupState->keepAlive = g_timeout_source_new(VMBACKUP_KEEP_ALIVE_PERIOD / 2); +- VMTOOLSAPP_ATTACH_SOURCE(gBackupState->ctx, +- gBackupState->keepAlive, +- VmBackupKeepAliveCallback, +- NULL, +- NULL); + return success; + } + +@@ -440,6 +449,12 @@ VmBackupDoAbort(void) + { + g_debug("*** %s\n", __FUNCTION__); + ASSERT(gBackupState != NULL); ++ ++ /* ++ * Once we abort the operation, we don't care about RPC state. ++ */ ++ gBackupState->rpcState = VMBACKUP_RPC_STATE_IGNORE; ++ + if (gBackupState->machineState != VMBACKUP_MSTATE_SCRIPT_ERROR && + gBackupState->machineState != VMBACKUP_MSTATE_SYNC_ERROR) { + const char *eventMsg = "Quiesce aborted."; +@@ -623,6 +638,17 @@ VmBackupAsyncCallback(void *clientData) + if (opPending) { + goto exit; + } ++ ++ /* ++ * VMX state might have changed when we were processing ++ * currentOp. This is usually detected by failures in ++ * sending backup event to the host. ++ */ ++ if (gBackupState->rpcState == VMBACKUP_RPC_STATE_ERROR) { ++ g_warning("Aborting backup operation due to RPC errors."); ++ VmBackupDoAbort(); ++ goto exit; ++ } + } + + switch (gBackupState->machineState) { +@@ -958,6 +984,7 @@ VmBackupStartCommon(RpcInData *data, + gBackupState->enableNullDriver = VMBACKUP_CONFIG_GET_BOOL(ctx->config, + "enableNullDriver", + TRUE); ++ gBackupState->rpcState = VMBACKUP_RPC_STATE_NORMAL; + + g_debug("Using quiesceApps = %d, quiesceFS = %d, allowHWProvider = %d," + " execScripts = %d, scriptArg = %s, timeout = %u," +diff --git a/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h b/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h +index ad3f2d7c..4258ee0a 100644 +--- a/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h ++++ b/open-vm-tools/services/plugins/vmbackup/vmBackupInt.h +@@ -72,6 +72,12 @@ typedef enum { + VMBACKUP_MSTATE_SYNC_ERROR + } VmBackupMState; + ++typedef enum { ++ VMBACKUP_RPC_STATE_NORMAL, ++ VMBACKUP_RPC_STATE_ERROR, ++ VMBACKUP_RPC_STATE_IGNORE ++} VmBackupRpcState; ++ + /** + * This is a "base struct" for asynchronous operations monitored by the + * state machine. Each implementation should provide these three functions +@@ -138,6 +144,7 @@ typedef struct VmBackupState { + Bool vssBootableSystemState; + Bool vssPartialFileSupport; + Bool vssUseDefault; ++ VmBackupRpcState rpcState; + } VmBackupState; + + typedef Bool (*VmBackupCallback)(VmBackupState *);