Basic live migration was broken by the commit that added non-shared block support in two ways: 1) It added a virCheckFlags() to doNativeMigrate(). Besides the fact that typical usage of virCheckFlags() is in driver entry points, and doNativeMigrate() is not an entry point, it was missing important flags like VIR_MIGRATE_LIVE. Move the virCheckFlags to the top-level qemuDomainMigratePrepare2 and friends. Unfortunately this required adding a new variant of the virCheckFlags() macro that could handle unsigned long arguments, but it should be easily extendable from here. 2) It also added a memory leak in qemuMonitorTextMigrate() by not freeing the memory used by virBufferContentAndReset(). This is fixed by storing the pointer in a temporary variable and freeing it at the end. With this patch in place, normal live migration works again. Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx> --- src/esx/esx_driver.c | 18 +++++----- src/internal.h | 10 +++++- src/nwfilter/nwfilter_driver.c | 2 +- src/qemu/qemu_driver.c | 64 +++++++++++++++++++++++++++------------ src/qemu/qemu_monitor_text.c | 10 ++++-- src/storage/storage_driver.c | 2 +- src/vbox/vbox_tmpl.c | 24 +++++++------- src/xen/xend_internal.c | 6 ++-- 8 files changed, 85 insertions(+), 51 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 7c9c50e..7a3609b 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3310,7 +3310,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_TaskInfoState taskInfoState; virDomainSnapshotPtr snapshot = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); if (esxVI_EnsureSession(priv->host) < 0) { goto failure; @@ -3383,7 +3383,7 @@ esxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot, char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; char *xml = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); memset(&def, 0, sizeof (virDomainSnapshotDef)); @@ -3435,7 +3435,7 @@ esxDomainSnapshotNum(virDomainPtr domain, unsigned int flags) esxPrivate *priv = domain->conn->privateData; esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); if (esxVI_EnsureSession(priv->host) < 0) { goto failure; @@ -3469,7 +3469,7 @@ esxDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, esxPrivate *priv = domain->conn->privateData; esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); if (names == NULL || nameslen < 0) { ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", _("Invalid argument")); @@ -3514,7 +3514,7 @@ esxDomainSnapshotLookupByName(virDomainPtr domain, const char *name, esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; virDomainSnapshotPtr snapshot = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); if (esxVI_EnsureSession(priv->host) < 0) { goto cleanup; @@ -3545,7 +3545,7 @@ esxDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags) esxPrivate *priv = domain->conn->privateData; esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); if (esxVI_EnsureSession(priv->host) < 0) { goto failure; @@ -3581,7 +3581,7 @@ esxDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) esxPrivate *priv = domain->conn->privateData; esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); if (esxVI_EnsureSession(priv->host) < 0) { goto cleanup; @@ -3614,7 +3614,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); if (esxVI_EnsureSession(priv->host) < 0) { goto failure; @@ -3667,7 +3667,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; - virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); + virCheckFlagsUI(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); if (esxVI_EnsureSession(priv->host) < 0) { goto failure; diff --git a/src/internal.h b/src/internal.h index c3c5311..d39a35e 100644 --- a/src/internal.h +++ b/src/internal.h @@ -210,7 +210,7 @@ * Returns nothing. Exits the caller function if unsupported flags were * passed to it. */ -# define virCheckFlags(supported, retval) \ +# define virCheckFlags(supported, retval, format) \ do { \ if ((flags & ~(supported))) { \ virReportErrorHelper(NULL, \ @@ -219,10 +219,16 @@ __FILE__, \ __FUNCTION__, \ __LINE__, \ - _("%s: unsupported flags (0x%x)"), \ + _("%s: unsupported flags (0x" #format ")"),\ __FUNCTION__, flags & ~(supported)); \ return retval; \ } \ } while (0) +# define virCheckFlagsUI(supported, retval) \ + virCheckFlags(supported, retval, %x) + +# define virCheckFlagsUL(supported, retval) \ + virCheckFlags(supported, retval, %lx) + #endif /* __VIR_INTERNAL_H__ */ diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 3ded2be..b356e95 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -389,7 +389,7 @@ nwfilterDumpXML(virNWFilterPtr obj, virNWFilterPoolObjPtr pool; char *ret = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); nwfilterDriverLock(driver); pool = virNWFilterPoolObjFindByUUID(&driver->pools, obj->uuid); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 51ee320..6e93755 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5252,7 +5252,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) int ret = -1; int compressed; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -5313,7 +5313,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) int ret = -1; char *name = NULL; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -5347,7 +5347,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) int ret = -1; char *name = NULL; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -8077,9 +8077,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virCgroupPtr cgroup = NULL; int ret = -1; - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | - VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + virCheckFlagsUI(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | + VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -9457,7 +9457,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, struct stat sb; int i; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -10149,6 +10149,15 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, int ret = -1; int internalret; + virCheckFlagsUL(VIR_MIGRATE_LIVE | + VIR_MIGRATE_PEER2PEER | + VIR_MIGRATE_TUNNELLED | + VIR_MIGRATE_PERSIST_DEST | + VIR_MIGRATE_UNDEFINE_SOURCE | + VIR_MIGRATE_PAUSED | + VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC, -1); + *uri_out = NULL; qemuDriverLock(driver); @@ -10330,9 +10339,6 @@ static int doNativeMigrate(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; unsigned int background_flags = 0; - virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC, - -1); - /* Issue the migrate command. */ if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) { /* HACK: source host generates bogus URIs, so fix them up */ @@ -10753,6 +10759,15 @@ qemudDomainMigratePerform (virDomainPtr dom, int resume = 0; qemuDomainObjPrivatePtr priv; + virCheckFlagsUL(VIR_MIGRATE_LIVE | + VIR_MIGRATE_PEER2PEER | + VIR_MIGRATE_TUNNELLED | + VIR_MIGRATE_PERSIST_DEST | + VIR_MIGRATE_UNDEFINE_SOURCE | + VIR_MIGRATE_PAUSED | + VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC, -1); + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { @@ -10856,6 +10871,15 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn, virErrorPtr orig_err; int newVM = 1; + virCheckFlagsUL(VIR_MIGRATE_LIVE | + VIR_MIGRATE_PEER2PEER | + VIR_MIGRATE_TUNNELLED | + VIR_MIGRATE_PERSIST_DEST | + VIR_MIGRATE_UNDEFINE_SOURCE | + VIR_MIGRATE_PAUSED | + VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC, NULL); + /* Migration failed. Save the current error so nothing squashes it */ orig_err = virSaveLastError(); @@ -11224,7 +11248,7 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, qemuDomainObjPrivatePtr priv; int ret = -1; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -11394,7 +11418,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL, NULL }; int i; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); qemuDriverLock(driver); virUUIDFormat(domain->uuid, uuidstr); @@ -11514,7 +11538,7 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, virDomainObjPtr vm = NULL; int n = -1; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); @@ -11542,7 +11566,7 @@ static int qemuDomainSnapshotNum(virDomainPtr domain, virDomainObjPtr vm = NULL; int n = -1; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); @@ -11572,7 +11596,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain, virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); @@ -11607,7 +11631,7 @@ static int qemuDomainHasCurrentSnapshot(virDomainPtr domain, virDomainObjPtr vm; int ret = -1; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); @@ -11635,7 +11659,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, virDomainObjPtr vm; virDomainSnapshotPtr snapshot = NULL; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); @@ -11671,7 +11695,7 @@ static char *qemuDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot, virDomainSnapshotObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); qemuDriverLock(driver); virUUIDFormat(snapshot->domain->uuid, uuidstr); @@ -11711,7 +11735,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuDomainObjPrivatePtr priv; int rc; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); qemuDriverLock(driver); virUUIDFormat(snapshot->domain->uuid, uuidstr); @@ -11949,7 +11973,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, char uuidstr[VIR_UUID_STRING_BUFLEN]; struct snap_remove rem; - virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); + virCheckFlagsUI(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); qemuDriverLock(driver); virUUIDFormat(snapshot->domain->uuid, uuidstr); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 3df078a..66988aa 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1134,6 +1134,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, int ret = -1; char *safedest = qemuMonitorEscapeArg(dest); virBuffer extra = VIR_BUFFER_INITIALIZER; + char *extrastr = NULL; if (!safedest) { virReportOOMError(); @@ -1149,10 +1150,12 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, if (virBufferError(&extra)) { virBufferFreeAndReset(&extra); virReportOOMError(); - free(safedest); - return -1; + goto cleanup; } - if (virAsprintf(&cmd, "migrate %s\"%s\"", virBufferContentAndReset(&extra), safedest) < 0) { + + extrastr = virBufferContentAndReset(&extra); + if (virAsprintf(&cmd, "migrate %s\"%s\"", extrastr ? extrastr : "", + safedest) < 0) { virReportOOMError(); goto cleanup; } @@ -1181,6 +1184,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, ret = 0; cleanup: + VIR_FREE(extrastr); VIR_FREE(safedest); VIR_FREE(info); VIR_FREE(cmd); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index b148e39..27aaf86 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1681,7 +1681,7 @@ storageVolumeWipe(virStorageVolPtr obj, virStorageVolDefPtr vol = NULL; int ret = -1; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); storageDriverLock(driver); pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 6a9a2bf..ce4d659 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -4877,9 +4877,9 @@ static int vboxDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, static int vboxDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | - VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + virCheckFlagsUI(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | + VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { vboxError(VIR_ERR_OPERATION_INVALID, "%s", @@ -5189,7 +5189,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, PRInt32 result; #endif - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1))) goto cleanup; @@ -5304,7 +5304,7 @@ vboxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot, PRBool online = PR_FALSE; char uuidstr[VIR_UUID_STRING_BUFLEN]; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(domiid) < 0) { @@ -5415,7 +5415,7 @@ vboxDomainSnapshotNum(virDomainPtr dom, nsresult rc; PRUint32 snapshotCount; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(iid) < 0) { @@ -5462,7 +5462,7 @@ vboxDomainSnapshotListNames(virDomainPtr dom, int count = 0; int i; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(iid) < 0) { @@ -5532,7 +5532,7 @@ vboxDomainSnapshotLookupByName(virDomainPtr dom, ISnapshot *snapshot = NULL; nsresult rc; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(iid) < 0) { @@ -5571,7 +5571,7 @@ vboxDomainHasCurrentSnapshot(virDomainPtr dom, ISnapshot *snapshot = NULL; nsresult rc; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(iid) < 0) { @@ -5618,7 +5618,7 @@ vboxDomainSnapshotCurrent(virDomainPtr dom, char *name = NULL; nsresult rc; - virCheckFlags(0, NULL); + virCheckFlagsUI(0, NULL); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(iid) < 0) { @@ -5793,7 +5793,7 @@ vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, PRUint32 state; nsresult rc; - virCheckFlags(0, -1); + virCheckFlagsUI(0, -1); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(domiid) < 0) { @@ -5961,7 +5961,7 @@ vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, PRUint32 state; nsresult rc; - virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); + virCheckFlagsUI(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); #if VBOX_API_VERSION == 2002 if (VIR_ALLOC(domiid) < 0) { diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index a203a8d..f1267cd 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -4205,9 +4205,9 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | - VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + virCheckFlagsUI(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | + VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); -- 1.6.6.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list