3:24 p.m.
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(a)redhat.com>
---
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