[libvirt] [PATCH RFC] helper of copy-storage-* features
help to create disk images copy-storage-* required, try to do non-shared migration without bothering to create disk images at target by hand. consider this situation: 1. non-shared migration virsh migrate --copy-storage-all ... 2. migration fails 3. create disk images required qemu-img create ... 4 migration run smoothly so, try do remove step 2, 3, 4 this kind of use had been discussed before, http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html maybe there're some flaws: - It did not handle more about complete situations suggested by Daniel P. Berrange, https://www.redhat.com/archives/libvir-list/2012-October/msg00407.html but may try to take care of them later. so, now only normal disk image files be handled. - for creation of disk images, size was setting as 0xffffffff boldly, hope it can consolidate qemu, haven't constructed a comfortable idea to solve it. Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 87 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..5c3ad9f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -49,6 +49,7 @@ #include "storage_file.h" #include "viruri.h" #include "hooks.h" +#include "dirname.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS, QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT, + QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE, QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -77,12 +79,13 @@ enum qemuMigrationCookieFlags { VIR_ENUM_DECL(qemuMigrationCookieFlag); VIR_ENUM_IMPL(qemuMigrationCookieFlag, QEMU_MIGRATION_COOKIE_FLAG_LAST, - "graphics", "lockstate", "persistent"); + "graphics", "lockstate", "persistent", "copystorage"); enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE), QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT), + QEMU_MIGRATION_COOKIE_COPYSTORAGE = (1 << QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE), }; typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -439,6 +442,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver, virBufferAdjustIndent(buf, -2); } + if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + virBufferAsprintf(buf, " <copystorage/>\n"); + virBufferAddLit(buf, "</qemu-migration>\n"); return 0; } @@ -662,6 +668,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, VIR_FREE(nodes); } + if ((flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE)) { + if (virXPathBoolean("count(./copystorage) > 0", ctxt)) + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE; + } + return 0; error: @@ -721,6 +732,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, qemuMigrationCookieAddPersistent(mig, dom) < 0) return -1; + if (flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE; + if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) return -1; @@ -1168,6 +1182,14 @@ char *qemuMigrationBegin(struct qemud_driver *driver, QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0) goto cleanup; + if (flags & (VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC)) { + if (qemuMigrationBakeCookie(mig, driver, vm, + cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_COPYSTORAGE) < 0) + goto cleanup; + } + if (xmlin) { if (!(def = virDomainDefParseString(driver->caps, xmlin, QEMU_EXPECTED_VIRT_TYPES, @@ -1215,6 +1237,54 @@ qemuMigrationPrepareCleanup(struct qemud_driver *driver, qemuDomainObjDiscardAsyncJob(driver, vm); } +static int qemuMigrationHandleDiskFiles(virDomainDefPtr def, int pin) +{ + char *tmp_dir = NULL, *outbuf = NULL; + char *size = _("0xffffffff"); + virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0; + for (i = 0; i < def->ndisks; i++) { + if (STRNEQ(def->disks[i]->driverName, "qemu")) + continue; + if (def->disks[i]->src == NULL) + continue; + if (def->disks[i]->driverType == NULL) + goto cleanup; + if (virFileExists(def->disks[i]->src) && pin) + continue; + if (!pin && !virFileExists(def->disks[i]->src)) + continue; + if ((tmp_dir = mdir_name(def->disks[i]->src)) == NULL) + continue; + if (!virFileExists(tmp_dir)) + if (virFileMakePath(tmp_dir) < 0) + continue; + if (pin) { + cmd = virCommandNewArgList("qemu-img", "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, + size, NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + } else + cmd = virCommandNewArgList("rm", "-f", def->disks[i]->src, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + goto cleanup; + virReportSystemError(errno, "%s", + _("unable create disk images by qemu-img")); + } + } + + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(tmp_dir); + return ret; +} + static int qemuMigrationPrepareAny(struct qemud_driver *driver, virConnectPtr dconn, @@ -1308,6 +1378,15 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* virDomainAssignDef already set the error */ goto cleanup; } + + if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, + QEMU_MIGRATION_COOKIE_COPYSTORAGE))) + goto cleanup; + + if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + if (qemuMigrationHandleDiskFiles(def, 1) < 0) + goto cleanup; + def = NULL; priv = vm->privateData; priv->origname = origname; @@ -2929,7 +3008,7 @@ qemuMigrationFinish(struct qemud_driver *driver, int newVM = 1; qemuMigrationCookiePtr mig = NULL; virErrorPtr orig_err = NULL; - int cookie_flags = 0; + int cookie_flags = 0, migration_status = 0; qemuDomainObjPrivatePtr priv = vm->privateData; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " @@ -3088,7 +3167,11 @@ qemuMigrationFinish(struct qemud_driver *driver, if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0) VIR_WARN("Unable to encode migration cookie"); + migration_status = 1; + endjob: + if (!migration_status) + qemuMigrationHandleDiskFiles(vm->def, 0); if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!vm->persistent && !virDomainObjIsActive(vm)) { -- 1.7.2.5
On Sun, Oct 21, 2012 at 7:53 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
help to create disk images copy-storage-* required, try to do non-shared migration without bothering to create disk images at target by hand.
consider this situation: 1. non-shared migration virsh migrate --copy-storage-all ... 2. migration fails 3. create disk images required qemu-img create ... 4 migration run smoothly so, try do remove step 2, 3, 4
this kind of use had been discussed before, http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
maybe there're some flaws: - It did not handle more about complete situations suggested by Daniel P. Berrange, https://www.redhat.com/archives/libvir-list/2012-October/msg00407.html but may try to take care of them later. so, now only normal disk image files be handled.
The problem is it will silently blow up for people using LVM or other backend types which is definitely not ok. You should do preflight checks to ensure that you can handle all the disks before attempting the migration.
- for creation of disk images, size was setting as 0xffffffff boldly, hope it can consolidate qemu, haven't constructed a comfortable idea to solve it.
What if the virtual disk coming over is larger? Will this go boom? Or are you expecting that qemu will resize? If you're expecting that qemu will resize it then you should likely only create the disk 1 block big or so. If not you need to send the sizes as part of the cookie.
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 87 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..5c3ad9f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -49,6 +49,7 @@ #include "storage_file.h" #include "viruri.h" #include "hooks.h" +#include "dirname.h"
#define VIR_FROM_THIS VIR_FROM_QEMU @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS, QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT, + QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -77,12 +79,13 @@ enum qemuMigrationCookieFlags { VIR_ENUM_DECL(qemuMigrationCookieFlag); VIR_ENUM_IMPL(qemuMigrationCookieFlag, QEMU_MIGRATION_COOKIE_FLAG_LAST, - "graphics", "lockstate", "persistent"); + "graphics", "lockstate", "persistent", "copystorage");
enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE), QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT), + QEMU_MIGRATION_COOKIE_COPYSTORAGE = (1 << QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE), };
typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -439,6 +442,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver, virBufferAdjustIndent(buf, -2); }
+ if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + virBufferAsprintf(buf, " <copystorage/>\n"); + virBufferAddLit(buf, "</qemu-migration>\n"); return 0; } @@ -662,6 +668,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, VIR_FREE(nodes); }
+ if ((flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE)) { + if (virXPathBoolean("count(./copystorage) > 0", ctxt)) + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE; + } + return 0;
error: @@ -721,6 +732,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, qemuMigrationCookieAddPersistent(mig, dom) < 0) return -1;
+ if (flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE; + if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) return -1;
@@ -1168,6 +1182,14 @@ char *qemuMigrationBegin(struct qemud_driver *driver, QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0) goto cleanup;
+ if (flags & (VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC)) { + if (qemuMigrationBakeCookie(mig, driver, vm, + cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_COPYSTORAGE) < 0) + goto cleanup; + } + if (xmlin) { if (!(def = virDomainDefParseString(driver->caps, xmlin, QEMU_EXPECTED_VIRT_TYPES, @@ -1215,6 +1237,54 @@ qemuMigrationPrepareCleanup(struct qemud_driver *driver, qemuDomainObjDiscardAsyncJob(driver, vm); }
+static int qemuMigrationHandleDiskFiles(virDomainDefPtr def, int pin)
Please provide some kind of documentation as to what the function is expected to do and potentially return. Also describe the arguments, specifically 'pin'. It seems poorly named. It appears to mean you want the disk file created or deleted.
+{ + char *tmp_dir = NULL, *outbuf = NULL; + char *size = _("0xffffffff"); + virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0;
Redundant with the for loop.
+ for (i = 0; i < def->ndisks; i++) {
Some code comments above these checks would definitely make this clearer and wouldn't cause someone to have to grok all the checks to know what's about to happen.
+ if (STRNEQ(def->disks[i]->driverName, "qemu")) + continue; + if (def->disks[i]->src == NULL) + continue; + if (def->disks[i]->driverType == NULL) + goto cleanup;
If you got a NULL driver don't you want to try the next disk instead of exiting?
+ if (virFileExists(def->disks[i]->src) && pin) + continue; + if (!pin && !virFileExists(def->disks[i]->src)) + continue;
At least align the above arguments to make it clearer. Or refactor it to make it even clearer.
+ if ((tmp_dir = mdir_name(def->disks[i]->src)) == NULL) + continue;
Based on Eric's comments I thought you were going to make a wrapper function for this and handle the NULL case. mdir_name() also allocates memory and you're calling it inside of a loop but only freeing it outside of the loop so this is a memory leak.
+ if (!virFileExists(tmp_dir)) + if (virFileMakePath(tmp_dir) < 0) + continue; + if (pin) { + cmd = virCommandNewArgList("qemu-img", "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, + size, NULL); + virCommandSetOutputBuffer(cmd, &outbuf);
You aren't doing anything with outbuf and you aren't freeing it either.
+ } else + cmd = virCommandNewArgList("rm", "-f", def->disks[i]->src, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + goto cleanup; + virReportSystemError(errno, "%s", + _("unable create disk images by qemu-img")); + }
The error message reported doesn't match what happened when the rm -f fails. Also 'cmd' is being leaked here.
+ } + + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(tmp_dir); + return ret; +} + static int qemuMigrationPrepareAny(struct qemud_driver *driver, virConnectPtr dconn, @@ -1308,6 +1378,15 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* virDomainAssignDef already set the error */ goto cleanup; } + + if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, + QEMU_MIGRATION_COOKIE_COPYSTORAGE))) + goto cleanup; + + if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + if (qemuMigrationHandleDiskFiles(def, 1) < 0) + goto cleanup; + def = NULL; priv = vm->privateData; priv->origname = origname; @@ -2929,7 +3008,7 @@ qemuMigrationFinish(struct qemud_driver *driver, int newVM = 1; qemuMigrationCookiePtr mig = NULL; virErrorPtr orig_err = NULL; - int cookie_flags = 0; + int cookie_flags = 0, migration_status = 0; qemuDomainObjPrivatePtr priv = vm->privateData;
VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " @@ -3088,7 +3167,11 @@ qemuMigrationFinish(struct qemud_driver *driver, if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0) VIR_WARN("Unable to encode migration cookie");
+ migration_status = 1; + endjob: + if (!migration_status) + qemuMigrationHandleDiskFiles(vm->def, 0); if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!vm->persistent && !virDomainObjIsActive(vm)) { -- 1.7.2.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Doug Goldstein
在 2012-10-21日的 21:09 -0500，Doug Goldstein写道：
On Sun, Oct 21, 2012 at 7:53 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
help to create disk images copy-storage-* required, try to do non-shared migration without bothering to create disk images at target by hand.
consider this situation: 1. non-shared migration virsh migrate --copy-storage-all ... 2. migration fails 3. create disk images required qemu-img create ... 4 migration run smoothly so, try do remove step 2, 3, 4
this kind of use had been discussed before, http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
maybe there're some flaws: - It did not handle more about complete situations suggested by Daniel P. Berrange, https://www.redhat.com/archives/libvir-list/2012-October/msg00407.html but may try to take care of them later. so, now only normal disk image files be handled.
The problem is it will silently blow up for people using LVM or other backend types which is definitely not ok. You should do preflight checks to ensure that you can handle all the disks before attempting the migration.
- for creation of disk images, size was setting as 0xffffffff boldly, hope it can consolidate qemu, haven't constructed a comfortable idea to solve it.
What if the virtual disk coming over is larger? Will this go boom? Or are you expecting that qemu will resize? If you're expecting that qemu will resize it then you should likely only create the disk 1 block big or so. If not you need to send the sizes as part of the cookie.
I don't know exactly what action qemu will take if pre-alloc is smaller, sorry about it. my pre-allocation will only take small real disk space, and hope it's greater than real disk space migration required. pretty roughly way! or, any good idea about it? but, I really don't want to take disk size from source to target.
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 87 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..5c3ad9f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -49,6 +49,7 @@ #include "storage_file.h" #include "viruri.h" #include "hooks.h" +#include "dirname.h"
#define VIR_FROM_THIS VIR_FROM_QEMU @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS, QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT, + QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -77,12 +79,13 @@ enum qemuMigrationCookieFlags { VIR_ENUM_DECL(qemuMigrationCookieFlag); VIR_ENUM_IMPL(qemuMigrationCookieFlag, QEMU_MIGRATION_COOKIE_FLAG_LAST, - "graphics", "lockstate", "persistent"); + "graphics", "lockstate", "persistent", "copystorage");
enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE), QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT), + QEMU_MIGRATION_COOKIE_COPYSTORAGE = (1 << QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE), };
typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -439,6 +442,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver, virBufferAdjustIndent(buf, -2); }
+ if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + virBufferAsprintf(buf, " <copystorage/>\n"); + virBufferAddLit(buf, "</qemu-migration>\n"); return 0; } @@ -662,6 +668,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, VIR_FREE(nodes); }
+ if ((flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE)) { + if (virXPathBoolean("count(./copystorage) > 0", ctxt)) + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE; + } + return 0;
error: @@ -721,6 +732,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, qemuMigrationCookieAddPersistent(mig, dom) < 0) return -1;
+ if (flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE; + if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig))) return -1;
@@ -1168,6 +1182,14 @@ char *qemuMigrationBegin(struct qemud_driver *driver, QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0) goto cleanup;
+ if (flags & (VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC)) { + if (qemuMigrationBakeCookie(mig, driver, vm, + cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_COPYSTORAGE) < 0) + goto cleanup; + } + if (xmlin) { if (!(def = virDomainDefParseString(driver->caps, xmlin, QEMU_EXPECTED_VIRT_TYPES, @@ -1215,6 +1237,54 @@ qemuMigrationPrepareCleanup(struct qemud_driver *driver, qemuDomainObjDiscardAsyncJob(driver, vm); }
+static int qemuMigrationHandleDiskFiles(virDomainDefPtr def, int pin)
Please provide some kind of documentation as to what the function is expected to do and potentially return. Also describe the arguments, specifically 'pin'. It seems poorly named. It appears to mean you want the disk file created or deleted.
+{ + char *tmp_dir = NULL, *outbuf = NULL; + char *size = _("0xffffffff"); + virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0;
Redundant with the for loop.
+ for (i = 0; i < def->ndisks; i++) {
Some code comments above these checks would definitely make this clearer and wouldn't cause someone to have to grok all the checks to know what's about to happen.
+ if (STRNEQ(def->disks[i]->driverName, "qemu")) + continue; + if (def->disks[i]->src == NULL) + continue; + if (def->disks[i]->driverType == NULL) + goto cleanup;
If you got a NULL driver don't you want to try the next disk instead of exiting?
+ if (virFileExists(def->disks[i]->src) && pin) + continue; + if (!pin && !virFileExists(def->disks[i]->src)) + continue;
At least align the above arguments to make it clearer. Or refactor it to make it even clearer.
+ if ((tmp_dir = mdir_name(def->disks[i]->src)) == NULL) + continue;
Based on Eric's comments I thought you were going to make a wrapper function for this and handle the NULL case.
mdir_name() also allocates memory and you're calling it inside of a loop but only freeing it outside of the loop so this is a memory leak.
yes, thanks!
+ if (!virFileExists(tmp_dir)) + if (virFileMakePath(tmp_dir) < 0) + continue; + if (pin) { + cmd = virCommandNewArgList("qemu-img", "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, + size, NULL); + virCommandSetOutputBuffer(cmd, &outbuf);
You aren't doing anything with outbuf and you aren't freeing it either.
+ } else + cmd = virCommandNewArgList("rm", "-f", def->disks[i]->src, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + goto cleanup; + virReportSystemError(errno, "%s", + _("unable create disk images by qemu-img")); + }
The error message reported doesn't match what happened when the rm -f fails. Also 'cmd' is being leaked here.
+ } + + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(tmp_dir); + return ret; +} + static int qemuMigrationPrepareAny(struct qemud_driver *driver, virConnectPtr dconn, @@ -1308,6 +1378,15 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* virDomainAssignDef already set the error */ goto cleanup; } + + if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, + QEMU_MIGRATION_COOKIE_COPYSTORAGE))) + goto cleanup; + + if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) + if (qemuMigrationHandleDiskFiles(def, 1) < 0) + goto cleanup; + def = NULL; priv = vm->privateData; priv->origname = origname; @@ -2929,7 +3008,7 @@ qemuMigrationFinish(struct qemud_driver *driver, int newVM = 1; qemuMigrationCookiePtr mig = NULL; virErrorPtr orig_err = NULL; - int cookie_flags = 0; + int cookie_flags = 0, migration_status = 0; qemuDomainObjPrivatePtr priv = vm->privateData;
VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " @@ -3088,7 +3167,11 @@ qemuMigrationFinish(struct qemud_driver *driver, if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0) VIR_WARN("Unable to encode migration cookie");
+ migration_status = 1; + endjob: + if (!migration_status) + qemuMigrationHandleDiskFiles(vm->def, 0); if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!vm->persistent && !virDomainObjIsActive(vm)) { -- 1.7.2.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- liguang lig.fnst@cn.fujitsu.com FNST linux kernel team
On 10/21/2012 08:09 PM, Doug Goldstein wrote:
On Sun, Oct 21, 2012 at 7:53 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
help to create disk images copy-storage-* required, try to do non-shared migration without bothering to create disk images at target by hand.
+ } else + cmd = virCommandNewArgList("rm", "-f", def->disks[i]->src, NULL); + + if (virCommandRun(cmd, NULL) < 0) { + goto cleanup; + virReportSystemError(errno, "%s", + _("unable create disk images by qemu-img")); + }
The error message reported doesn't match what happened when the rm -f fails. Also 'cmd' is being leaked here.
Not to mention virCommandRun() is overkill. Use unlink() here instead, rather than forking out to an external program. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Doug Goldstein -
Eric Blake -
li guang -
liguang