Signed-off-by: liguang <lig.fnst(a)cn.fujitsu.com&gt; --- 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();

Signed-off-by: liguang <lig.fnst(a)cn.fujitsu.com&gt; --- 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;

+++ 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); +

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(a)cn.fujitsu.com&gt; > --- > 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)) {

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(a)cn.fujitsu.com&gt; > --- > 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?