[libvirt] [PATCH 2/2] help to create disk images of non-shared migration
Doug Goldstein
cardoe at gentoo.org
Wed Nov 14 08:18:25 UTC 2012
On Tue, Nov 13, 2012 at 9:03 PM, liguang <lig.fnst at cn.fujitsu.com> wrote:
> 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
>
> this patch depends on my support offline migration patch:
> https://www.redhat.com/archives/libvir-list/2012-November/msg00512.html
>
> Signed-off-by: liguang <lig.fnst at cn.fujitsu.com>
> ---
> src/qemu/qemu_migration.c | 283 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 282 insertions(+), 1 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2479114..6d4a4d7 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -50,6 +50,7 @@
> #include "storage_file.h"
> #include "viruri.h"
> #include "hooks.h"
> +#include "dirname.h"
>
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
> @@ -72,6 +73,7 @@ enum qemuMigrationCookieFlags {
> QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
> QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
> QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
> + QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
>
> QEMU_MIGRATION_COOKIE_FLAG_LAST
> };
> @@ -79,13 +81,14 @@ enum qemuMigrationCookieFlags {
> VIR_ENUM_DECL(qemuMigrationCookieFlag);
> VIR_ENUM_IMPL(qemuMigrationCookieFlag,
> QEMU_MIGRATION_COOKIE_FLAG_LAST,
> - "graphics", "lockstate", "persistent", "network");
> + "graphics", "lockstate", "persistent", "network", "storage");
>
> 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_NETWORK = (1 << QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
> + QEMU_MIGRATION_COOKIE_COPYSTORAGE = (1 << QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE),
> };
>
> typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
> @@ -119,6 +122,19 @@ struct _qemuMigrationCookieNetwork {
> qemuMigrationCookieNetDataPtr net;
> };
>
> +typedef struct _qemuMigrationCookieStorageData qemuMigrationCookieStorageData;
> +typedef qemuMigrationCookieStorageData *qemuMigrationCookieStorageDataPtr;
> +struct _qemuMigrationCookieStorageData {
> + char *dsize;
> +};
> +
> +typedef struct _qemuMigrationCookieStorage qemuMigrationCookieStorage;
> +typedef qemuMigrationCookieStorage *qemuMigrationCookieStoragePtr;
> +struct _qemuMigrationCookieStorage {
> + int ndisks;
> + qemuMigrationCookieStorageDataPtr disk;
> +};
> +
> typedef struct _qemuMigrationCookie qemuMigrationCookie;
> typedef qemuMigrationCookie *qemuMigrationCookiePtr;
> struct _qemuMigrationCookie {
> @@ -147,6 +163,9 @@ struct _qemuMigrationCookie {
>
> /* If (flags & QEMU_MIGRATION_COOKIE_NETWORK) */
> qemuMigrationCookieNetworkPtr network;
> +
> + /* If (flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) */
> + qemuMigrationCookieStoragePtr storage;
> };
>
> static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> @@ -175,6 +194,21 @@ qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network)
> VIR_FREE(network);
> }
>
> +static void
> +qemuMigrationCookieStorageFree(qemuMigrationCookieStoragePtr storage)
> +{
> + int i;
> +
> + if (!storage)
> + return;
> +
> + if (storage->disk) {
> + for (i = 0; i < storage->ndisks; i++)
> + VIR_FREE(storage->disk[i].dsize);
> + }
> + VIR_FREE(storage->disk);
> + VIR_FREE(storage);
> +}
>
> static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> {
> @@ -187,6 +221,9 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> if (mig->flags & QEMU_MIGRATION_COOKIE_NETWORK)
> qemuMigrationCookieNetworkFree(mig->network);
>
> + if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE)
> + qemuMigrationCookieStorageFree(mig->storage);
> +
> VIR_FREE(mig->localHostname);
> VIR_FREE(mig->remoteHostname);
> VIR_FREE(mig->name);
> @@ -356,6 +393,64 @@ error:
> return NULL;
> }
>
> +static qemuMigrationCookieStoragePtr
> +qemuMigrationCookieStorageAlloc(struct qemud_driver *driver,
> + virDomainDefPtr def)
> +{
> + int i, exitstatus;
> + char *info = NULL, *start, *end, *tmp = NULL;
> + virCommandPtr cmd = NULL;
> + qemuMigrationCookieStoragePtr mig;
> +
> + if (VIR_ALLOC(mig) < 0)
> + goto no_memory;
> +
> + if (VIR_ALLOC_N(mig->disk, def->ndisks) < 0)
> + goto no_memory;
> + if (!driver->qemuImgBinary) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("unable to find kvm-img or qemu-img"));
Use qemuFindQemuImgBinary() instead.
> + goto error;
> + }
> + mig->ndisks = def->ndisks;
> +
> + for (i = 0; i < mig->ndisks; i++) {
> + cmd = virCommandNewArgList(driver->qemuImgBinary, "info",
> + def->disks[i]->src, NULL);
> + virCommandAddEnvString(cmd, "LC_ALL=C");
> + virCommandSetOutputBuffer(cmd, &info);
> + virCommandClearCaps(cmd);
> + if (virCommandRun(cmd, &exitstatus) < 0)
> + goto error;
> + if ((start = strstr(info, "virtual size: ")) == NULL ||
> + ((end = strstr(start, "\n")) == NULL)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unable to parse qemu-img output '%s'"),
> + info);
> + goto error;
> + }
> + if ((tmp = strstr(start, " (")) && tmp < end)
> + end = tmp;
> + start += strlen("virtual size: ");
> + if (start >= end)
> + goto error;
> + if (!(mig->disk[i].dsize = strndup(start, end-start)))
> + goto error;
> + VIR_FREE(info);
> + virCommandFree(cmd);
> + }
> +
> + return mig;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + virCommandFree(cmd);
> + VIR_FREE(info);
> + qemuMigrationCookieStorageFree(mig);
> + return NULL;
> +}
Honestly this entire function should be dropped. Doing a callout to
qemu-img isn't good due to the fact that we might have other file
types in play and this would effectively circumvent the disk format
probing prevention. Among other reasons.. Really I think you should
take a look at qemuDomainGetBlockInfo(), you're looking for the
capacity value. It'd probably be worthwhile to refactor some of that
code into a helper function that can be used by both
qemuDomainGetBlockInfo() and qemuMigrationCookieStorageAlloc(). It
might also be possible for you to just call qemuDomainGetBlockInfo()
directly if you have all the input available in the callee, which is
possible.
> +
> static qemuMigrationCookiePtr
> qemuMigrationCookieNew(virDomainObjPtr dom)
> {
> @@ -491,6 +586,25 @@ qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig,
> return 0;
> }
>
> +static int qemuMigrationCookieAddStorage(qemuMigrationCookiePtr mig,
> + struct qemud_driver *driver,
> + virDomainObjPtr dom)
> +{
> + if (mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("migration storage data already present"));
> + return -1;
> + }
> +
> + if (dom->def->ndisks > 0) {
> + mig->storage = qemuMigrationCookieStorageAlloc(driver, dom->def);
> + if (!mig->storage)
> + return -1;
> + mig->flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE;
> + }
> +
> + return 0;
> +}
>
> static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
> qemuMigrationCookieGraphicsPtr grap)
> @@ -540,6 +654,19 @@ qemuMigrationCookieNetworkXMLFormat(virBufferPtr buf,
> virBufferAddLit(buf, " </network>\n");
> }
>
> +static void
> +qemuMigrationCookieStorageXMLFormat(virBufferPtr buf,
> + qemuMigrationCookieStoragePtr dsz)
> +{
> + int i = 0;
> +
> + for (i = 0; i < dsz->ndisks; i++) {
> + char *dsize = dsz->disk[i].dsize;
> + virBufferAsprintf(buf, " <copystorage>\n");
> + virBufferAsprintf(buf, " <disksize>%s</disksize>\n", dsize);
> + virBufferAddLit(buf, " </copystorage>\n");
> + }
Is it safe to assume this list will be ordered the same way on the src
and dest machines with regard to target file/partition? If not we need
to carry some value along to label each disk.
> +}
>
> static int
> qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> @@ -594,6 +721,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network)
> qemuMigrationCookieNetworkXMLFormat(buf, mig->network);
>
> + if ((mig->flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) && mig->storage)
> + qemuMigrationCookieStorageXMLFormat(buf, mig->storage);
> +
> virBufferAddLit(buf, "</qemu-migration>\n");
> return 0;
> }
> @@ -722,6 +852,44 @@ error:
> goto cleanup;
> }
>
> +static qemuMigrationCookieStoragePtr
> +qemuMigrationCookieStorageXMLParse(xmlXPathContextPtr ctxt)
> +{
> + qemuMigrationCookieStoragePtr dsz;
> + int i, n;
> + xmlNodePtr *storage = NULL;
> +
> + if (VIR_ALLOC(dsz) < 0)
> + goto no_memory;
> +
> + if ((n = virXPathNodeSet("./copystorage", ctxt, &storage)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing storage information"));
> + goto error;
> + }
> +
> + dsz->ndisks = n;
> + if (VIR_ALLOC_N(dsz->disk, dsz->ndisks) < 0)
> + goto no_memory;
> +
> + for (i = 0; i < dsz->ndisks; i++) {
> + ctxt->node = storage[i];
> + if (!(dsz->disk[i].dsize = virXPathString("string(./disksize[1])", ctxt)))
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing tlsPort attribute in migration data"));
Wrong error message here.
> + }
> +
> +cleanup:
> + VIR_FREE(storage);
> + return dsz;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + qemuMigrationCookieStorageFree(dsz);
> + dsz = NULL;
> + goto cleanup;
> +}
>
> static int
> qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> @@ -874,6 +1042,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt))))
> goto error;
>
> + if ((flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE) &&
> + virXPathBoolean("count(./copystorage) > 0", ctxt) &&
> + (!(mig->storage = qemuMigrationCookieStorageXMLParse(ctxt))))
> + goto error;
> +
> return 0;
>
> error:
> @@ -938,6 +1111,11 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
> return -1;
> }
>
> + if (flags & QEMU_MIGRATION_COOKIE_COPYSTORAGE &&
> + qemuMigrationCookieAddStorage(mig, driver, dom) < 0) {
> + return -1;
> + }
> +
> if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
> return -1;
>
> @@ -1443,6 +1621,11 @@ char *qemuMigrationBegin(struct qemud_driver *driver,
> QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0)
> goto cleanup;
>
> + if (qemuMigrationBakeCookie(mig, driver, vm,
> + cookieout, cookieoutlen,
> + QEMU_MIGRATION_COOKIE_COPYSTORAGE) < 0)
> + goto cleanup;
> +
> if (flags & VIR_MIGRATE_OFFLINE) {
> if (flags & (VIR_MIGRATE_NON_SHARED_DISK|
> VIR_MIGRATE_NON_SHARED_INC)) {
> @@ -1484,6 +1667,89 @@ cleanup:
> }
>
>
> +/*
> + if gen is true, find out disk images migration required,
> + so try to generate them at target,
> + if gen is false, delete disk images generated before.
> +*/
> +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver,
> + virDomainDefPtr def, bool gen,
> + qemuMigrationCookiePtr mig)
> +{
> + char *tmp_dir = NULL, *outbuf = NULL;
> + char *img_tool = driver->qemuImgBinary;
Should be using qemuFindQemuImgBinary().
> + const char *disk_format[] = {"none", "raw", "none", "none", "none",
> + "cow", "none", "none", "qcow", "qcow2",
> + "qed", "vmdk", "vpc","none",
> + };
Drop this whole list and....
> + 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 (virFileExists(def->disks[i]->src) && gen)
> + continue;
> + if (!gen && !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 (STREQ(disk_format[def->disks[i]->format], "none"))
> + continue;
> +
> + if (gen) {
> + char *dsize = mig->storage->disk[i].dsize;
> + if (def->disks[i]->format < VIR_STORAGE_FILE_RAW)
> + goto error;
> + cmd = virCommandNewArgList(img_tool, "create", "-f",
> + disk_format[def->disks[i]->format],
...use virStorageFileFormatTypeToString().
> + def->disks[i]->src, NULL);
> + virCommandAddArgFormat(cmd, "%s", dsize);
> + 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;
> +}
> +
> +
> /* Prepare is the first step, and it runs on the destination host.
> */
>
> @@ -1599,6 +1865,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 (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_DISK))
> + if (qemuMigrationHandleDiskFiles(driver, def, true, mig) < 0)
> + goto endjob;
> +
> def = NULL;
> priv = vm->privateData;
> priv->origname = origname;
> @@ -3239,6 +3514,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
> virErrorPtr orig_err = NULL;
> int cookie_flags = 0;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> + bool migration_status = false;
>
> VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
> "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
> @@ -3404,7 +3680,12 @@ qemuMigrationFinish(struct qemud_driver *driver,
> if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 0)
> VIR_WARN("Unable to encode migration cookie");
>
> + migration_status = true;
> +
> endjob:
> + if (!migration_status && flags &
> + (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_DISK))
> + qemuMigrationHandleDiskFiles(driver, vm->def, false, NULL);
> if (qemuMigrationJobFinish(driver, vm) == 0) {
> vm = NULL;
> } else if (!vm->persistent && !virDomainObjIsActive(vm)) {
> --
> 1.7.1
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Just a quick look through tonight but given some comments I think this
warrants another revision. Its possible we might want to not do some
of the creation manually and ask the storage backend to be doing that
for us as well.
--
Doug Goldstein
More information about the libvir-list
mailing list