[libvirt] [PATCH v3 1/2] 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 usage 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. v2: 1. handle disk encrytion case 2. check kvm-img & qemu-img 3. set disk image size to 0xfffffffK bytes blindly v3: 1.use qemuImgBinary to create disk image 2.set max size for different disk image format respectively qcow and qcow2: 1PiB qed:64TiB raw:1TiB from qemu's setting, qcow and qcow2's max size is 2EiB, qed's max size is 64TiB raw's max size is 1TiB Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 122 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 120 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..80abb51 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,89 @@ qemuMigrationPrepareCleanup(struct qemud_driver *driver, qemuDomainObjDiscardAsyncJob(driver, vm); } +/* + if gen_del is 1, find out disk images migration required, + so try to generate them at target, + if gen_del is 0, delete disk images generated before. +*/ +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver, + virDomainDefPtr def, int gen_del) +{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary; + virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0; + + if (img_tool == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + goto error; + } + + 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) + continue; + if (virFileExists(def->disks[i]->src) && gen_del) + continue; + if (!gen_del && !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 (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff); + else if (STREQ(def->disks[i]->driverType, "qed")) + virCommandAddArgFormat(cmd, "%llu", 0x400000000000); + else if (STREQ(def->disks[i]->driverType, "raw")) + virCommandAddArgFormat(cmd, "%llu", 0xffffffffff); + else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only suport qcow,qcow2,qed,raw format")); + goto cleanup; + } + if (def->disks[i]->encryption) + virCommandAddArgList(cmd, "-o", "encryption=on", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { + virReportSystemError(errno, "%s", outbuf); + goto cleanup; + } + } else { + if (unlink(def->disks[i]->src) < 0) { + virReportError(errno, "%s", _("fail to unlink disk image file")); + goto cleanup; + } + } + virCommandFree(cmd); + VIR_FREE(tmp_dir); + VIR_FREE(outbuf); + } + + ret = 0; + +cleanup: + if (ret < 0) { + virCommandFree(cmd); + VIR_FREE(tmp_dir); + VIR_FREE(outbuf); + } +error: + return ret; +} + static int qemuMigrationPrepareAny(struct qemud_driver *driver, virConnectPtr dconn, @@ -1308,6 +1413,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(driver, def, 1) < 0) + goto cleanup; + def = NULL; priv = vm->privateData; priv->origname = origname; @@ -2929,7 +3043,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 +3202,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(driver, vm->def, 0); if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!vm->persistent && !virDomainObjIsActive(vm)) { -- 1.7.1
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 17ae3b9..ac16772 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver, int i; bool skipped = false; - qemuimgarg[0] = qemuFindQemuImgBinary(driver); + qemuimgarg[0] = driver->qemuImgBinary; if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 97ad23e..882e95a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -624,6 +624,9 @@ qemudStartup(int privileged) { if (!qemu_driver->domainEventState) goto error; + /*find kvm-img or qemu-img*/ + qemuFindQemuImgBinary(qemu_driver); + /* read the host sysinfo */ if (privileged) qemu_driver->hostsysinfo = virSysinfoRead(); -- 1.7.1
ping ... 在 2012-11-01四的 08:40 +0800，liguang写道：
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 17ae3b9..ac16772 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver, int i; bool skipped = false;
- qemuimgarg[0] = qemuFindQemuImgBinary(driver); + qemuimgarg[0] = driver->qemuImgBinary; if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 97ad23e..882e95a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -624,6 +624,9 @@ qemudStartup(int privileged) { if (!qemu_driver->domainEventState) goto error;
+ /*find kvm-img or qemu-img*/ + qemuFindQemuImgBinary(qemu_driver); + /* read the host sysinfo */ if (privileged) qemu_driver->hostsysinfo = virSysinfoRead();
-- liguang lig.fnst@cn.fujitsu.com FNST linux kernel team
Hi, Eric any comment on this patch? 在 2012-11-06二的 16:09 +0800，li guang写道：
ping ...
在 2012-11-01四的 08:40 +0800，liguang写道：
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 17ae3b9..ac16772 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver, int i; bool skipped = false;
- qemuimgarg[0] = qemuFindQemuImgBinary(driver); + qemuimgarg[0] = driver->qemuImgBinary; if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 97ad23e..882e95a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -624,6 +624,9 @@ qemudStartup(int privileged) { if (!qemu_driver->domainEventState) goto error;
+ /*find kvm-img or qemu-img*/ + qemuFindQemuImgBinary(qemu_driver); + /* read the host sysinfo */ if (privileged) qemu_driver->hostsysinfo = virSysinfoRead();
-- li guang lig.fnst@cn.fujitsu.com linux kernel team at FNST, china
On 10/31/2012 06:40 PM, liguang wrote:
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 17ae3b9..ac16772 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver, int i; bool skipped = false;
- qemuimgarg[0] = qemuFindQemuImgBinary(driver); + qemuimgarg[0] = driver->qemuImgBinary;
Personally, I like going through accessor methods rather than direct field access, as it leaves us more freedom for changing the implementation in the future. I'm not sure this patch buys us anything, other than it would fail to start the libvirtd qemu driver at startup rather than on the first command that needed to use qemu-img. Since failing early on a bad setup is generally useful, I guess I could live with this patch, except that you need to rebase it on top of Peter's recent patches that added another caller of qemuFindQemuImgBinary. For that matter, if you are going to gaurantee that driver->qemuImgBinary is always set, it might be better to remove the qemuFindQemuImgBinary() function altogether, and instead inline its body...
+++ b/src/qemu/qemu_driver.c @@ -624,6 +624,9 @@ qemudStartup(int privileged) { if (!qemu_driver->domainEventState) goto error;
+ /*find kvm-img or qemu-img*/ + qemuFindQemuImgBinary(qemu_driver); +
into this one remaining call site. Also, I would rebase the series to put this patch first, not last, since your patch 1/2 was using driver->qemuImgBinary instead of qemuFindQemuImgBinary(). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
ping ... 在 2012-11-01四的 08:40 +0800，liguang写道：
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 usage 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.
v2: 1. handle disk encrytion case 2. check kvm-img & qemu-img 3. set disk image size to 0xfffffffK bytes blindly
v3: 1.use qemuImgBinary to create disk image 2.set max size for different disk image format respectively qcow and qcow2: 1PiB qed:64TiB raw:1TiB from qemu's setting, qcow and qcow2's max size is 2EiB, qed's max size is 64TiB raw's max size is 1TiB
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 122 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..80abb51 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,89 @@ qemuMigrationPrepareCleanup(struct qemud_driver *driver, qemuDomainObjDiscardAsyncJob(driver, vm); }
+/* + if gen_del is 1, find out disk images migration required, + so try to generate them at target, + if gen_del is 0, delete disk images generated before. +*/ +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver, + virDomainDefPtr def, int gen_del) +{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary; + virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0; + + if (img_tool == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + goto error; + } + + 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) + continue; + if (virFileExists(def->disks[i]->src) && gen_del) + continue; + if (!gen_del && !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 (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff); + else if (STREQ(def->disks[i]->driverType, "qed")) + virCommandAddArgFormat(cmd, "%llu", 0x400000000000); + else if (STREQ(def->disks[i]->driverType, "raw")) + virCommandAddArgFormat(cmd, "%llu", 0xffffffffff); + else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only suport qcow,qcow2,qed,raw format")); + goto cleanup; + } + if (def->disks[i]->encryption) + virCommandAddArgList(cmd, "-o", "encryption=on", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { + virReportSystemError(errno, "%s", outbuf); + goto cleanup; + } + } else { + if (unlink(def->disks[i]->src) < 0) { + virReportError(errno, "%s", _("fail to unlink disk image file")); + goto cleanup; + } + } + virCommandFree(cmd); + VIR_FREE(tmp_dir); + VIR_FREE(outbuf); + } + + ret = 0; + +cleanup: + if (ret < 0) { + virCommandFree(cmd); + VIR_FREE(tmp_dir); + VIR_FREE(outbuf); + } +error: + return ret; +} + static int qemuMigrationPrepareAny(struct qemud_driver *driver, virConnectPtr dconn, @@ -1308,6 +1413,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(driver, def, 1) < 0) + goto cleanup; + def = NULL; priv = vm->privateData; priv->origname = origname; @@ -2929,7 +3043,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 +3202,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(driver, vm->def, 0); if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!vm->persistent && !virDomainObjIsActive(vm)) {
-- liguang lig.fnst@cn.fujitsu.com FNST linux kernel team
Hi, Eric any comment on this patch? 在 2012-11-06二的 16:09 +0800，li guang写道：
ping ... 在 2012-11-01四的 08:40 +0800，liguang写道：
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 usage 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.
v2: 1. handle disk encrytion case 2. check kvm-img & qemu-img 3. set disk image size to 0xfffffffK bytes blindly
v3: 1.use qemuImgBinary to create disk image 2.set max size for different disk image format respectively qcow and qcow2: 1PiB qed:64TiB raw:1TiB from qemu's setting, qcow and qcow2's max size is 2EiB, qed's max size is 64TiB raw's max size is 1TiB
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 122 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..80abb51 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,89 @@ qemuMigrationPrepareCleanup(struct qemud_driver *driver, qemuDomainObjDiscardAsyncJob(driver, vm); }
+/* + if gen_del is 1, find out disk images migration required, + so try to generate them at target, + if gen_del is 0, delete disk images generated before. +*/ +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver, + virDomainDefPtr def, int gen_del) +{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary; + virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0; + + if (img_tool == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + goto error; + } + + 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) + continue; + if (virFileExists(def->disks[i]->src) && gen_del) + continue; + if (!gen_del && !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 (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff); + else if (STREQ(def->disks[i]->driverType, "qed")) + virCommandAddArgFormat(cmd, "%llu", 0x400000000000); + else if (STREQ(def->disks[i]->driverType, "raw")) + virCommandAddArgFormat(cmd, "%llu", 0xffffffffff); + else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only suport qcow,qcow2,qed,raw format")); + goto cleanup; + } + if (def->disks[i]->encryption) + virCommandAddArgList(cmd, "-o", "encryption=on", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { + virReportSystemError(errno, "%s", outbuf); + goto cleanup; + } + } else { + if (unlink(def->disks[i]->src) < 0) { + virReportError(errno, "%s", _("fail to unlink disk image file")); + goto cleanup; + } + } + virCommandFree(cmd); + VIR_FREE(tmp_dir); + VIR_FREE(outbuf); + } + + ret = 0; + +cleanup: + if (ret < 0) { + virCommandFree(cmd); + VIR_FREE(tmp_dir); + VIR_FREE(outbuf); + } +error: + return ret; +} + static int qemuMigrationPrepareAny(struct qemud_driver *driver, virConnectPtr dconn, @@ -1308,6 +1413,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(driver, def, 1) < 0) + goto cleanup; + def = NULL; priv = vm->privateData; priv->origname = origname; @@ -2929,7 +3043,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 +3202,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(driver, vm->def, 0); if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!vm->persistent && !virDomainObjIsActive(vm)) {
-- li guang lig.fnst@cn.fujitsu.com linux kernel team at FNST, china
On 10/31/2012 06:40 PM, liguang 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 usage had been discussed before, http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
Typically, you would put your signed off line here, followed by ---, so that the rest of this commentary...
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,
Setting to MAX_INT sounds wrong. Either you create the destination image empty and then let qemu automatically enlarge it as it populates incoming data (preferred), or you need to pass size information in the cookie (harder).
hope it can consolidate qemu, haven't constructed a comfortable idea to solve it.
v2: 1. handle disk encrytion case 2. check kvm-img & qemu-img 3. set disk image size to 0xfffffffK bytes blindly
v3: 1.use qemuImgBinary to create disk image 2.set max size for different disk image format respectively qcow and qcow2: 1PiB qed:64TiB raw:1TiB from qemu's setting, qcow and qcow2's max size is 2EiB, qed's max size is 64TiB raw's max size is 1TiB
...remains useful for reviewing on list but is not codified in git when the patch is finally approved. That is, when reading 'git log', we don't care how many versions it took us to get to the right solution, only that the right solution is in git. 'git am' automatically strips comments after the first ---, making it a useful divider between the commit description and the notes for reviewers.
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 122 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..80abb51 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,
I'm not yet convinced whether you need to do anything with the cookie. Rather, it should be sufficient to add a new public VIR_MIGRATE_* flag that says whether libvirt should be attempting to create destination files when given VIR_MIGRATE_NON_SHARED_{DISK,INC}. Then, just like the offline migration case, all you have to do is make sure the flag is properly passed through the entire migration chain to all the points that care about it. But if you DO need the cookie, the I think you need more than just a single flag - you need to pass a list of all file information (such as size) that will be needed to create the same types of files on the destination.
+/* + if gen_del is 1, find out disk images migration required, + so try to generate them at target, + if gen_del is 0, delete disk images generated before. +*/ +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver, + virDomainDefPtr def, int gen_del)
It sounds like you are using gen_del as a bool - if so, type it as bool, not int. And its name is confusing; I might go with 'bool generate', where true means generate, and false means delete.
+{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary;
Don't grab this field directly. Instead, use qemuFindQemuImgBinary(driver).
+ virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0; + + if (img_tool == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + goto error; + } + + for (i = 0; i < def->ndisks; i++) { + if (STRNEQ(def->disks[i]->driverName, "qemu")) + continue;
You need to rebase your patches on top of the latest libvirt.git. The driverName field no longer exists; it is now an enum value named 'format'.
+ if (def->disks[i]->src == NULL) + continue; + if (def->disks[i]->driverType == NULL) + continue; + if (virFileExists(def->disks[i]->src) && gen_del) + continue; + if (!gen_del && !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 (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff);
Ouch. You should not be passing in random sizes to qemu - if you need to know the source size in order to create a file on the destination side, then we HAVE to modify the migration cookie in order to pass sizing information when the new flag is present.
+ } else { + if (unlink(def->disks[i]->src) < 0) { + virReportError(errno, "%s", _("fail to unlink disk image file")); + goto cleanup;
Are you sure that this only ever unlink()s files that you just created, or does it have the possibility of unlinking a file that already existed prior to the migration attempt? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
在 2012-11-08四的 22:10 -0700，Eric Blake写道：
On 10/31/2012 06:40 PM, liguang 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 usage had been discussed before, http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
Typically, you would put your signed off line here, followed by ---, so that the rest of this commentary...
OK, got it.
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,
Setting to MAX_INT sounds wrong. Either you create the destination image empty and then let qemu automatically enlarge it as it populates incoming data (preferred), or you need to pass size information in the cookie (harder).
can qemu automatically enlarge? I tried, but failed.
hope it can consolidate qemu, haven't constructed a comfortable idea to solve it.
v2: 1. handle disk encrytion case 2. check kvm-img & qemu-img 3. set disk image size to 0xfffffffK bytes blindly
v3: 1.use qemuImgBinary to create disk image 2.set max size for different disk image format respectively qcow and qcow2: 1PiB qed:64TiB raw:1TiB from qemu's setting, qcow and qcow2's max size is 2EiB, qed's max size is 64TiB raw's max size is 1TiB
...remains useful for reviewing on list but is not codified in git when the patch is finally approved. That is, when reading 'git log', we don't care how many versions it took us to get to the right solution, only that the right solution is in git. 'git am' automatically strips comments after the first ---, making it a useful divider between the commit description and the notes for reviewers.
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 122 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index db69a0a..80abb51 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,
I'm not yet convinced whether you need to do anything with the cookie. Rather, it should be sufficient to add a new public VIR_MIGRATE_* flag that says whether libvirt should be attempting to create destination files when given VIR_MIGRATE_NON_SHARED_{DISK,INC}. Then, just like the offline migration case, all you have to do is make sure the flag is properly passed through the entire migration chain to all the points that care about it. But if you DO need the cookie, the I think you need more than just a single flag - you need to pass a list of all file information (such as size) that will be needed to create the same types of files on the destination.
as we talk before.
+/* + if gen_del is 1, find out disk images migration required, + so try to generate them at target, + if gen_del is 0, delete disk images generated before. +*/ +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver, + virDomainDefPtr def, int gen_del)
It sounds like you are using gen_del as a bool - if so, type it as bool, not int. And its name is confusing; I might go with 'bool generate', where true means generate, and false means delete.
right, will fix.
+{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary;
Don't grab this field directly. Instead, use qemuFindQemuImgBinary(driver).
driver->qemuImgBinary is useful, I've initiated it. why don't we use it?
+ virCommandPtr cmd = NULL; + int i, ret = -1; + + if (!def->ndisks) + return 0; + + if (img_tool == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + goto error; + } + + for (i = 0; i < def->ndisks; i++) { + if (STRNEQ(def->disks[i]->driverName, "qemu")) + continue;
You need to rebase your patches on top of the latest libvirt.git. The driverName field no longer exists; it is now an enum value named 'format'.
OK.
+ if (def->disks[i]->src == NULL) + continue; + if (def->disks[i]->driverType == NULL) + continue; + if (virFileExists(def->disks[i]->src) && gen_del) + continue; + if (!gen_del && !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 (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff);
Ouch. You should not be passing in random sizes to qemu - if you need to know the source size in order to create a file on the destination side, then we HAVE to modify the migration cookie in order to pass sizing information when the new flag is present.
passing by cookie seems a little rough, these sizes are not random, they're the max size of their format. does setting max size hurt? the disk images are sparse files, and after qemu migration finished, size will be right.
+ } else { + if (unlink(def->disks[i]->src) < 0) { + virReportError(errno, "%s", _("fail to unlink disk image file")); + goto cleanup;
Are you sure that this only ever unlink()s files that you just created, or does it have the possibility of unlinking a file that already existed prior to the migration attempt?
-- li guang lig.fnst@cn.fujitsu.com linux kernel team at FNST, china
On 11/08/2012 10:26 PM, li guang wrote:
+{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary;
Don't grab this field directly. Instead, use qemuFindQemuImgBinary(driver).
driver->qemuImgBinary is useful, I've initiated it. why don't we use it?
In the order your series was written, driver->qemuImgBinary may be uninitialized. If you swap the order and put patch 2/2 first, then yes, you could use this field directly instead of calling qemuFindQemuImgBinary. You should always try to order your patches so that no patch is broken until later patches in the series have been applied.
+ if (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff);
Ouch. You should not be passing in random sizes to qemu - if you need to know the source size in order to create a file on the destination side, then we HAVE to modify the migration cookie in order to pass sizing information when the new flag is present.
passing by cookie seems a little rough, these sizes are not random, they're the max size of their format. does setting max size hurt?
Yes, because it is wrong.
the disk images are sparse files,
So? A user that calls virsh vol-info on the file on the destination side should not get infinite size, but the same size as they would get on the source. Just because it doesn't (yet) occupy the entire disk doesn't mean that we should size it so large that the file can grow without effective bounds and cause ENOSPC.
and after qemu migration finished, size will be right.
If qemu changes the size after migration, then why can't it change from 0 to the proper size, rather than from max to the proper size? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
在 2012-11-09五的 07:27 -0700，Eric Blake写道：
On 11/08/2012 10:26 PM, li guang wrote:
+{ + char *tmp_dir = NULL, *outbuf = NULL; + char *img_tool = driver->qemuImgBinary;
Don't grab this field directly. Instead, use qemuFindQemuImgBinary(driver).
driver->qemuImgBinary is useful, I've initiated it. why don't we use it?
In the order your series was written, driver->qemuImgBinary may be uninitialized. If you swap the order and put patch 2/2 first, then yes, you could use this field directly instead of calling qemuFindQemuImgBinary. You should always try to order your patches so that no patch is broken until later patches in the series have been applied.
+ if (gen_del) { + cmd = virCommandNewArgList(img_tool, "create", "-f", + def->disks[i]->driverType, def->disks[i]->src, NULL); + if (STREQ(def->disks[i]->driverType, "qcow2") || + STREQ(def->disks[i]->driverType, "qcow")) + virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff);
Ouch. You should not be passing in random sizes to qemu - if you need to know the source size in order to create a file on the destination side, then we HAVE to modify the migration cookie in order to pass sizing information when the new flag is present.
passing by cookie seems a little rough, these sizes are not random, they're the max size of their format. does setting max size hurt?
Yes, because it is wrong.
the disk images are sparse files,
So?
real size of disk image will not take much disk space.
A user that calls virsh vol-info on the file on the destination side should not get infinite size, but the same size as they would get on the source. Just because it doesn't (yet) occupy the entire disk doesn't mean that we should size it so large that the file can grow without effective bounds and cause ENOSPC.
um, vol-info is a problem, though it seems odd for me. can we add size field to _virDomainDiskDef? if be allowed, life will be easier.
and after qemu migration finished, size will be right.
If qemu changes the size after migration, then why can't it change from 0 to the proper size, rather than from max to the proper size?
I mean the disk size not virtual size.
-- li guang lig.fnst@cn.fujitsu.com linux kernel team at FNST, china
participants (3)
-
Eric Blake -
li guang -
liguang