[PATCH 00/13] qemu: zero-detection for non-shared-storage migration
See 12/13 for rationale. The first patch fixes allocation setting when pre-creating qcow2 images for migration. Next few patches fix/improve typed parameter list handling. Next few refactor handling of 'migrate_disks' parameter and the final two implement the new feature. Peter Krempa (13): qemu: migration: Pre-create QCOW2 images for non-shared storage with 0 allocation virTypedParamsFilter: Adjust return type and docs virTypedParamsGetStringList: Refactor and adjust docs virTypedParamsFilter: Introduce option to filter also by type virTypedParamsGetStringList: Ensure that returned array is NULL if there are no matching fields virTypedParamsGetStringList: Ensure that returned string list is NULL-terminated qemuMigrationSrcBeginPhaseBlockDirtyBitmaps: Use qemuMigrationAnyCopyDisk() qemu: migration: Don't log 'nmigrate_disks' qemu: migration: Avoid use of 'nmigration_disks' qemu: migration: Extract validation of disk target list qemu: migration: Remove 'nmigration_disks' variable from all places qemu: Introduce and wire in 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' virsh: Add support for VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES migration parameter docs/manpages/virsh.rst | 8 +- include/libvirt/libvirt-domain.h | 13 ++ src/qemu/qemu_driver.c | 55 +++--- src/qemu/qemu_migration.c | 299 +++++++++++++++++-------------- src/qemu/qemu_migration.h | 7 +- src/util/virtypedparam.c | 72 ++++---- src/util/virtypedparam.h | 3 +- tests/virtypedparamtest.c | 14 +- tools/virsh-domain.c | 26 +++ 9 files changed, 287 insertions(+), 210 deletions(-) -- 2.46.0
Specify that the <allocation> parameter for the newly-created qcow2 image is 0 so that only metadata gets preallocated. Otherwise the storage driver code instructs qemu to use 'fallocate' preallocation mode and considers the image fully allocated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7f905f8584..b686e42e58 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -361,6 +361,8 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, virBufferAddLit(&buf, "<volume>\n"); virBufferAdjustIndent(&buf, 2); virBufferEscapeString(&buf, "<name>%s</name>\n", volName); + if (disk->src->format == VIR_STORAGE_FILE_QCOW2) + virBufferAddLit(&buf, "<allocation>0</allocation>\n"); virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n", capacity); virBufferAddLit(&buf, "<target>\n"); virBufferAdjustIndent(&buf, 2); -- 2.46.0
The 'virTypedParamsFilter' function can't fail and thus it never returns negative values. Change the return type to 'size_t' and adjust callers to not check the return value for being negative. Adjust the docs to hilight this and also the fact that the filtered typed param list returned via @ret is not a deep copy and thus callers must not use the common function to free it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 19 ++++++++++--------- src/util/virtypedparam.h | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 1be249d855..634385aa97 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -394,18 +394,22 @@ virTypedParamsCopy(virTypedParameterPtr *dst, * @ret: pointer to the returned array * * Filters @params retaining only the parameters named @name in the - * resulting array @ret. Caller should free the @ret array but not - * the items since they are pointing to the @params elements. + * resulting array @ret. * - * Returns amount of elements in @ret on success, -1 on error. + * Important Caller should free the @ret array but not the items since they are + * pointing to the @params elements. I.e. callers must not use + * 'virTypedParamsFree' or equivalent on pointer returned via @ret. + * + * Returns amount of elements in @ret. */ -int +size_t virTypedParamsFilter(virTypedParameterPtr params, int nparams, const char *name, virTypedParameterPtr **ret) { - size_t i, n = 0; + size_t i; + size_t n = 0; *ret = g_new0(virTypedParameterPtr, nparams); @@ -443,7 +447,7 @@ virTypedParamsGetStringList(virTypedParameterPtr params, const char ***values) { size_t i, n; - int nfiltered; + size_t nfiltered; virTypedParameterPtr *filtered = NULL; virCheckNonNullArgGoto(values, error); @@ -451,9 +455,6 @@ virTypedParamsGetStringList(virTypedParameterPtr params, nfiltered = virTypedParamsFilter(params, nparams, name, &filtered); - if (nfiltered < 0) - goto error; - if (nfiltered) *values = g_new0(const char *, nfiltered); diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 7454ef3ce0..afd923aacb 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -81,7 +81,7 @@ virTypedParamsGetStringList(virTypedParameterPtr params, int nparams, const char *name, const char ***values); -int +size_t virTypedParamsFilter(virTypedParameterPtr params, int nparams, const char *name, -- 2.46.0
Use automatic freeing, declare one variable per line and return early when possible. As this is an internal helper there's no need to check that the caller passed non-NULL @values. Modify the documentation to be accurate and warn callers to not free the strings just the array. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 634385aa97..86fbaf5e9d 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -432,13 +432,13 @@ virTypedParamsFilter(virTypedParameterPtr params, * @values: array of returned values * * Finds all parameters with desired @name within @params and - * store their values into @values. The @values array is self - * allocated and its length is stored into @picked. When no - * longer needed, caller should free the returned array, but not - * the items since they are taken from @params array. + * store their values into @values. * - * Returns amount of strings in @values array on success, - * -1 otherwise. + * Important: The strings in the returned string list @values are borrowed from + * @params and thus caller must free only the pointer returned as @values, but + * not the contents. + * + * Returns amount of strings in @values array on success. */ int virTypedParamsGetStringList(virTypedParameterPtr params, @@ -446,31 +446,26 @@ virTypedParamsGetStringList(virTypedParameterPtr params, const char *name, const char ***values) { - size_t i, n; + size_t i; + size_t n = 0; size_t nfiltered; - virTypedParameterPtr *filtered = NULL; + g_autofree virTypedParameterPtr *filtered = NULL; - virCheckNonNullArgGoto(values, error); *values = NULL; nfiltered = virTypedParamsFilter(params, nparams, name, &filtered); - if (nfiltered) - *values = g_new0(const char *, nfiltered); + if (nfiltered == 0) + return 0; + + *values = g_new0(const char *, nfiltered); - for (n = 0, i = 0; i < nfiltered; i++) { + for (i = 0; i < nfiltered; i++) { if (filtered[i]->type == VIR_TYPED_PARAM_STRING) (*values)[n++] = filtered[i]->value.s; } - VIR_FREE(filtered); return n; - - error: - if (values) - VIR_FREE(*values); - VIR_FREE(filtered); - return -1; } -- 2.46.0
The only caller of this function is doing some additional filtering so it's useful if the filtering function was able to do so internally. Introduce a 'type' parameter which will optionally filter the results by type and extend the testsuite to cover this scenario. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 19 +++++++++++++------ src/util/virtypedparam.h | 1 + tests/virtypedparamtest.c | 14 +++++++++++--- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 86fbaf5e9d..a080d7ba0f 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -391,10 +391,12 @@ virTypedParamsCopy(virTypedParameterPtr *dst, * @params: array of typed parameters * @nparams: number of parameters in the @params array * @name: name of the parameter to find + * @type: type of fields to filter (ignored if 0 is passed) * @ret: pointer to the returned array * * Filters @params retaining only the parameters named @name in the - * resulting array @ret. + * resulting array @ret. If @type is non-zero it also filters out parameters + * whose type doesn't match @type. * * Important Caller should free the @ret array but not the items since they are * pointing to the @params elements. I.e. callers must not use @@ -406,6 +408,7 @@ size_t virTypedParamsFilter(virTypedParameterPtr params, int nparams, const char *name, + int type, virTypedParameterPtr **ret) { size_t i; @@ -414,10 +417,14 @@ virTypedParamsFilter(virTypedParameterPtr params, *ret = g_new0(virTypedParameterPtr, nparams); for (i = 0; i < nparams; i++) { - if (STREQ(params[i].field, name)) { - (*ret)[n] = ¶ms[i]; - n++; - } + if (STRNEQ(params[i].field, name)) + continue; + + if (type != 0 && + params[i].type != type) + continue; + + (*ret)[n++] = ¶ms[i]; } return n; @@ -453,7 +460,7 @@ virTypedParamsGetStringList(virTypedParameterPtr params, *values = NULL; - nfiltered = virTypedParamsFilter(params, nparams, name, &filtered); + nfiltered = virTypedParamsFilter(params, nparams, name, 0, &filtered); if (nfiltered == 0) return 0; diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index afd923aacb..774744244a 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -85,6 +85,7 @@ size_t virTypedParamsFilter(virTypedParameterPtr params, int nparams, const char *name, + int type, virTypedParameterPtr **ret) G_GNUC_WARN_UNUSED_RESULT; diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c index 5ced453be5..1a8b49383f 100644 --- a/tests/virtypedparamtest.c +++ b/tests/virtypedparamtest.c @@ -91,13 +91,14 @@ testTypedParamsFilter(const void *opaque G_GNUC_UNUSED) { .field = "bar", .type = VIR_TYPED_PARAM_UINT }, { .field = "foo", .type = VIR_TYPED_PARAM_INT }, { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, - { .field = "foo", .type = VIR_TYPED_PARAM_INT } + { .field = "foo", .type = VIR_TYPED_PARAM_INT }, + { .field = "foobar", .type = VIR_TYPED_PARAM_INT }, }; virTypedParameterPtr *filtered = NULL; nfiltered = virTypedParamsFilter(params, G_N_ELEMENTS(params), - "foo", &filtered); + "foo", 0, &filtered); if (nfiltered != 3) goto cleanup; @@ -108,7 +109,7 @@ testTypedParamsFilter(const void *opaque G_GNUC_UNUSED) VIR_FREE(filtered); nfiltered = virTypedParamsFilter(params, G_N_ELEMENTS(params), - "bar", &filtered); + "bar", VIR_TYPED_PARAM_UINT, &filtered); if (nfiltered != 2) goto cleanup; @@ -117,6 +118,13 @@ testTypedParamsFilter(const void *opaque G_GNUC_UNUSED) if (filtered[i] != ¶ms[i * 2]) goto cleanup; } + VIR_FREE(filtered); + + nfiltered = virTypedParamsFilter(params, G_N_ELEMENTS(params), + "foobar", VIR_TYPED_PARAM_STRING, &filtered); + + if (nfiltered != 1) + goto cleanup; rv = 0; cleanup: -- 2.46.0
'virTypedParamsGetStringList' fills the returned array only with string parameters with matching name. The filtering code though leaves the possibility that all items are filtered out but the return array is still (over)allocated. Since 'virTypedParamsFilter()' now also allows filtering by type we can move the filtering there ensuring that we always allocate the right number of elements and more importantly the returned array will be NULL if none elements are present. Rework the code and adjust docs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index a080d7ba0f..564cb81acc 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -439,7 +439,8 @@ virTypedParamsFilter(virTypedParameterPtr params, * @values: array of returned values * * Finds all parameters with desired @name within @params and - * store their values into @values. + * store their values into @values. If none of the @params are strings named + * @name the returned @values will be NULL. * * Important: The strings in the returned string list @values are borrowed from * @params and thus caller must free only the pointer returned as @values, but @@ -454,13 +455,12 @@ virTypedParamsGetStringList(virTypedParameterPtr params, const char ***values) { size_t i; - size_t n = 0; size_t nfiltered; g_autofree virTypedParameterPtr *filtered = NULL; *values = NULL; - nfiltered = virTypedParamsFilter(params, nparams, name, 0, &filtered); + nfiltered = virTypedParamsFilter(params, nparams, name, VIR_TYPED_PARAM_STRING, &filtered); if (nfiltered == 0) return 0; @@ -468,11 +468,10 @@ virTypedParamsGetStringList(virTypedParameterPtr params, *values = g_new0(const char *, nfiltered); for (i = 0; i < nfiltered; i++) { - if (filtered[i]->type == VIR_TYPED_PARAM_STRING) - (*values)[n++] = filtered[i]->value.s; + (*values)[i] = filtered[i]->value.s; } - return n; + return nfiltered; } -- 2.46.0
This can simplify callers who don't really need to know the number of elements to check that a particular element is present. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 564cb81acc..f25530a735 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -439,8 +439,8 @@ virTypedParamsFilter(virTypedParameterPtr params, * @values: array of returned values * * Finds all parameters with desired @name within @params and - * store their values into @values. If none of the @params are strings named - * @name the returned @values will be NULL. + * store their values into a NULL-terminated string list @values. If none of + * the @params are strings named @name the returned @values will be NULL. * * Important: The strings in the returned string list @values are borrowed from * @params and thus caller must free only the pointer returned as @values, but @@ -465,7 +465,7 @@ virTypedParamsGetStringList(virTypedParameterPtr params, if (nfiltered == 0) return 0; - *values = g_new0(const char *, nfiltered); + *values = g_new0(const char *, nfiltered + 1); for (i = 0; i < nfiltered; i++) { (*values)[i] = filtered[i]->value.s; -- 2.46.0
The function open-coded the checking whether a disk is being migrated with non-shared storage and did so badly (not taking into account if user doesn't explicitly provide list of disks to migrate). Use the existing helper instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b686e42e58..a2c5be3e82 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2466,19 +2466,8 @@ qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(qemuMigrationCookie *mig, if (!nodedata) continue; - if (migrate_disks) { - bool migrating = false; - - for (j = 0; j < nmigrate_disks; j++) { - if (STREQ(migrate_disks[j], diskdef->dst)) { - migrating = true; - break; - } - } - - if (!migrating) - continue; - } + if (!qemuMigrationAnyCopyDisk(diskdef, nmigrate_disks, migrate_disks)) + continue; for (j = 0; j < nodedata->nbitmaps; j++) { qemuMigrationBlockDirtyBitmapsDiskBitmap *bitmap; -- 2.46.0
The actual number of disks to migrate is not important. The presence of disks to migrate can be inferred from presence of the 'migrate_disks' pointer which is logged. Since 'nmigrate_disks' will eventually be removed remove the logging right now. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a2c5be3e82..506699333b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2599,9 +2599,9 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s," " cookieout=%p, cookieoutlen=%p," - " nmigrate_disks=%zu, migrate_disks=%p, flags=0x%x", + " migrate_disks=%p, flags=0x%x", driver, vm, NULLSTR(xmlin), NULLSTR(dname), - cookieout, cookieoutlen, nmigrate_disks, + cookieout, cookieoutlen, migrate_disks, flags); /* Only set the phase if we are inside VIR_ASYNC_JOB_MIGRATION_OUT. @@ -3735,12 +3735,12 @@ qemuMigrationDstPrepareDirect(virQEMUDriver *driver, VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " "def=%p, origname=%s, listenAddress=%s, " - "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, " + "migrate_disks=%p, nbdPort=%d, " "nbdURI=%s, flags=0x%x", driver, dconn, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, NULLSTR(uri_in), uri_out, *def, origname, NULLSTR(listenAddress), - nmigrate_disks, migrate_disks, nbdPort, NULLSTR(nbdURI), + migrate_disks, nbdPort, NULLSTR(nbdURI), flags); *uri_out = NULL; @@ -4775,11 +4775,11 @@ qemuMigrationSrcRun(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%x, resource=%lu, " "spec=%p (dest=%d, fwd=%d), dconn=%p, graphicsuri=%s, " - "nmigrate_disks=%zu, migrate_disks=%p", + "migrate_disks=%p", driver, vm, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource, spec, spec->destType, spec->fwdType, dconn, - NULLSTR(graphicsuri), nmigrate_disks, migrate_disks); + NULLSTR(graphicsuri), migrate_disks); if (storageMigration) storageMigration = qemuMigrationHasAnyStorageMigrationDisks(vm->def, @@ -5199,10 +5199,10 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%x, resource=%lu, " - "graphicsuri=%s, nmigrate_disks=%zu migrate_disks=%p", + "graphicsuri=%s, migrate_disks=%p", driver, vm, uri, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource, - NULLSTR(graphicsuri), nmigrate_disks, migrate_disks); + NULLSTR(graphicsuri), migrate_disks); if (!(uribits = qemuMigrationAnyParseURI(uri, NULL))) return -1; @@ -5303,10 +5303,10 @@ qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, st=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%x, resource=%lu, " - "graphicsuri=%s, nmigrate_disks=%zu, migrate_disks=%p", + "graphicsuri=%s, migrate_disks=%p", driver, vm, st, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource, - NULLSTR(graphicsuri), nmigrate_disks, migrate_disks); + NULLSTR(graphicsuri), migrate_disks); spec.fwdType = MIGRATION_FWD_STREAM; spec.fwd.stream = st; @@ -5571,11 +5571,11 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, VIR_DEBUG("driver=%p, sconn=%p, dconn=%p, dconnuri=%s, vm=%p, xmlin=%s, " "dname=%s, uri=%s, graphicsuri=%s, listenAddress=%s, " - "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, nbdURI=%s, " + "migrate_disks=%p, nbdPort=%d, nbdURI=%s, " "bandwidth=%llu, useParams=%d, flags=0x%x", driver, sconn, dconn, NULLSTR(dconnuri), vm, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri), NULLSTR(graphicsuri), - NULLSTR(listenAddress), nmigrate_disks, migrate_disks, nbdPort, + NULLSTR(listenAddress), migrate_disks, nbdPort, NULLSTR(nbdURI), bandwidth, useParams, flags); /* Unlike the virDomainMigrateVersion3 counterpart, we don't need @@ -5937,12 +5937,12 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, int rc; VIR_DEBUG("driver=%p, sconn=%p, vm=%p, xmlin=%s, dconnuri=%s, uri=%s, " - "graphicsuri=%s, listenAddress=%s, nmigrate_disks=%zu, " + "graphicsuri=%s, listenAddress=%s, " "migrate_disks=%p, nbdPort=%d, nbdURI=%s, flags=0x%x, " "dname=%s, resource=%lu", driver, sconn, vm, NULLSTR(xmlin), NULLSTR(dconnuri), NULLSTR(uri), NULLSTR(graphicsuri), NULLSTR(listenAddress), - nmigrate_disks, migrate_disks, nbdPort, NULLSTR(nbdURI), + migrate_disks, nbdPort, NULLSTR(nbdURI), flags, NULLSTR(dname), resource); if (flags & VIR_MIGRATE_TUNNELLED && uri) { @@ -6304,13 +6304,13 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, VIR_DEBUG("driver=%p, conn=%p, vm=%p, xmlin=%s, dconnuri=%s, " "uri=%s, graphicsuri=%s, listenAddress=%s, " - "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, " + "migrate_disks=%p, nbdPort=%d, " "nbdURI=%s, " "cookiein=%s, cookieinlen=%d, cookieout=%p, cookieoutlen=%p, " "flags=0x%x, dname=%s, resource=%lu, v3proto=%d", driver, conn, vm, NULLSTR(xmlin), NULLSTR(dconnuri), NULLSTR(uri), NULLSTR(graphicsuri), NULLSTR(listenAddress), - nmigrate_disks, migrate_disks, nbdPort, NULLSTR(nbdURI), + migrate_disks, nbdPort, NULLSTR(nbdURI), NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, NULLSTR(dname), resource, v3proto); -- 2.46.0
'migration_disks' is a NULL-terminated string list, so the code can be converted to either iterate the string-list, use existing accessors or check the presence of the pointers instead of checking the count. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 48 +++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 506699333b..bb466bf1ee 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -382,18 +382,12 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, static bool qemuMigrationAnyCopyDisk(virDomainDiskDef const *disk, - size_t nmigrate_disks, const char **migrate_disks) + size_t nmigrate_disks G_GNUC_UNUSED, + const char **migrate_disks) { - size_t i; - - /* Check if the disk alias is in the list */ - if (nmigrate_disks) { - for (i = 0; i < nmigrate_disks; i++) { - if (STREQ(disk->dst, migrate_disks[i])) - return true; - } - return false; - } + /* List of disks to migrate takes priority if present */ + if (migrate_disks) + return g_strv_contains(migrate_disks, disk->dst); /* Default is to migrate only non-shared non-readonly disks * with source */ @@ -2666,19 +2660,20 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return NULL; } - if (nmigrate_disks) { - size_t i, j; + if (migrate_disks) { + size_t j; + const char **d; + /* Check user requested only known disk targets. */ - for (i = 0; i < nmigrate_disks; i++) { + for (d = migrate_disks; *d; d++) { for (j = 0; j < vm->def->ndisks; j++) { - if (STREQ(vm->def->disks[j]->dst, migrate_disks[i])) + if (STREQ(vm->def->disks[j]->dst, *d)) break; } if (j == vm->def->ndisks) { virReportError(VIR_ERR_INVALID_ARG, - _("disk target %1$s not found"), - migrate_disks[i]); + _("disk target %1$s not found"), *d); return NULL; } } @@ -2691,7 +2686,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, nmigrate_disks)) cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } else { - if (nmigrate_disks > 0) { + if (migrate_disks) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("use of 'VIR_MIGRATE_PARAM_MIGRATE_DISKS' requires use of 'VIR_MIGRATE_NON_SHARED_DISK' or 'VIR_MIGRATE_NON_SHARED_INC' flag")); return NULL; @@ -5566,7 +5561,6 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; - size_t i; bool offline = !!(flags & VIR_MIGRATE_OFFLINE); VIR_DEBUG("driver=%p, sconn=%p, dconn=%p, dconnuri=%s, vm=%p, xmlin=%s, " @@ -5625,11 +5619,15 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, VIR_MIGRATE_PARAM_LISTEN_ADDRESS, listenAddress) < 0) goto cleanup; - for (i = 0; i < nmigrate_disks; i++) - if (virTypedParamsAddString(¶ms, &nparams, &maxparams, - VIR_MIGRATE_PARAM_MIGRATE_DISKS, - migrate_disks[i]) < 0) - goto cleanup; + if (migrate_disks) { + const char **d; + + for (d = migrate_disks; *d; d++) + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS, + *d) < 0) + goto cleanup; + } if (nbdPort && virTypedParamsAddInt(¶ms, &nparams, &maxparams, VIR_MIGRATE_PARAM_DISKS_PORT, @@ -6020,7 +6018,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, /* Only xmlin, dname, uri, and bandwidth parameters can be used with * old-style APIs. */ - if (!useParams && (graphicsuri || listenAddress || nmigrate_disks)) { + if (!useParams && (graphicsuri || listenAddress || migrate_disks)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Migration APIs with extensible parameters are not supported but extended parameters were passed")); goto cleanup; -- 2.46.0
The migration code is checking the disk list provided via VIR_MIGRATE_PARAM_MIGRATE_DISKS against existing disks. Extract it to a helper function as we'll be passing another list of disk targets soon. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 52 +++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index bb466bf1ee..a4efd8bfa7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2576,6 +2576,37 @@ qemuMigrationSrcBeginXML(virDomainObj *vm, } +/** + * qemuMigrationSrcBeginPhaseValidateDiskTargetList: + * @vm: domain object + * @disks: NULL-terminated list of disk 'dst' strings to validate + * + * Validates that all members of the @disk list are valid disk targets. + */ +static int +qemuMigrationSrcBeginPhaseValidateDiskTargetList(virDomainObj *vm, + const char **disks) +{ + size_t i; + const char **d; + + for (d = disks; *d; d++) { + for (i = 0; i < vm->def->ndisks; i++) { + if (STREQ(vm->def->disks[i]->dst, *d)) + break; + } + + if (i == vm->def->ndisks) { + virReportError(VIR_ERR_INVALID_ARG, + _("disk target %1$s not found"), *d); + return -1; + } + } + + return 0; +} + + /* The caller is supposed to lock the vm and start a migration job. */ static char * qemuMigrationSrcBeginPhase(virQEMUDriver *driver, @@ -2660,24 +2691,9 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return NULL; } - if (migrate_disks) { - size_t j; - const char **d; - - /* Check user requested only known disk targets. */ - for (d = migrate_disks; *d; d++) { - for (j = 0; j < vm->def->ndisks; j++) { - if (STREQ(vm->def->disks[j]->dst, *d)) - break; - } - - if (j == vm->def->ndisks) { - virReportError(VIR_ERR_INVALID_ARG, - _("disk target %1$s not found"), *d); - return NULL; - } - } - } + if (migrate_disks && + qemuMigrationSrcBeginPhaseValidateDiskTargetList(vm, migrate_disks) < 0) + return NULL; priv->nbdPort = 0; -- 2.46.0
Now that none of the functions need it we can drop it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 47 ++++++----------- src/qemu/qemu_migration.c | 105 ++++++++++++++------------------------ src/qemu/qemu_migration.h | 3 -- 3 files changed, 53 insertions(+), 102 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 609263d026..cec700a36c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10732,7 +10732,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, return qemuMigrationDstPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, NULL, 0, NULL, 0, NULL, + &def, origname, NULL, NULL, 0, NULL, migParams, flags); } @@ -10782,7 +10782,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationSrcPerform(driver, dom->conn, vm, NULL, - NULL, dconnuri, uri, NULL, NULL, 0, NULL, 0, + NULL, dconnuri, uri, NULL, NULL, NULL, 0, NULL, migParams, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ @@ -10858,7 +10858,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, } return qemuMigrationSrcBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, 0, NULL, flags); + cookieout, cookieoutlen, NULL, flags); } static char * @@ -10872,7 +10872,6 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, const char *xmlin = NULL; const char *dname = NULL; g_autofree const char **migrate_disks = NULL; - int nmigrate_disks; virDomainObj *vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); @@ -10887,12 +10886,8 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, &dname) < 0) return NULL; - nmigrate_disks = virTypedParamsGetStringList(params, nparams, - VIR_MIGRATE_PARAM_MIGRATE_DISKS, - &migrate_disks); - - if (nmigrate_disks < 0) - return NULL; + virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, + &migrate_disks); if (!(vm = qemuDomainObjFromDomain(domain))) return NULL; @@ -10904,7 +10899,7 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, return qemuMigrationSrcBegin(domain->conn, vm, xmlin, dname, cookieout, cookieoutlen, - nmigrate_disks, migrate_disks, flags); + migrate_disks, flags); } @@ -10951,7 +10946,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, NULL, 0, NULL, 0, + &def, origname, NULL, NULL, 0, NULL, migParams, flags); } @@ -10974,7 +10969,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, const char *uri_in = NULL; const char *listenAddress = NULL; int nbdPort = 0; - int nmigrate_disks; g_autofree const char **migrate_disks = NULL; g_autofree char *origname = NULL; g_autoptr(qemuMigrationParams) migParams = NULL; @@ -11004,19 +10998,15 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, &nbdPort) < 0) return -1; - nmigrate_disks = virTypedParamsGetStringList(params, nparams, - VIR_MIGRATE_PARAM_MIGRATE_DISKS, - &migrate_disks); - - if (nmigrate_disks < 0) - return -1; + virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, + &migrate_disks); if (!(migParams = qemuMigrationParamsFromFlags(params, nparams, flags, QEMU_MIGRATION_DESTINATION))) return -1; if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) || - nmigrate_disks > 0) { + migrate_disks) { if (uri_in && STRPREFIX(uri_in, "unix:") && !nbdURI) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("NBD URI must be supplied when migration URI uses UNIX transport method")); @@ -11060,7 +11050,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, cookieout, cookieoutlen, uri_in, uri_out, &def, origname, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, + migrate_disks, nbdPort, nbdURI, migParams, flags); } @@ -11190,7 +11180,7 @@ qemuDomainMigratePerform3(virDomainPtr dom, goto cleanup; ret = qemuMigrationSrcPerform(driver, dom->conn, vm, xmlin, NULL, - dconnuri, uri, NULL, NULL, 0, NULL, 0, + dconnuri, uri, NULL, NULL, NULL, 0, NULL, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -11220,7 +11210,6 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, const char *uri = NULL; const char *graphicsuri = NULL; const char *listenAddress = NULL; - int nmigrate_disks; g_autofree const char **migrate_disks = NULL; unsigned long long bandwidth = 0; int nbdPort = 0; @@ -11276,15 +11265,11 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, } } - nmigrate_disks = virTypedParamsGetStringList(params, nparams, - VIR_MIGRATE_PARAM_MIGRATE_DISKS, - &migrate_disks); - - if (nmigrate_disks < 0) - goto cleanup; + virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, + &migrate_disks); if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) || - nmigrate_disks > 0) { + migrate_disks) { if (uri && STRPREFIX(uri, "unix:") && !nbdURI) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("NBD URI must be supplied when migration URI uses UNIX transport method")); @@ -11304,7 +11289,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, ret = qemuMigrationSrcPerform(driver, dom->conn, vm, dom_xml, persist_xml, dconnuri, uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, + migrate_disks, nbdPort, nbdURI, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, bandwidth, true); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a4efd8bfa7..94636e778d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -382,7 +382,6 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, static bool qemuMigrationAnyCopyDisk(virDomainDiskDef const *disk, - size_t nmigrate_disks G_GNUC_UNUSED, const char **migrate_disks) { /* List of disks to migrate takes priority if present */ @@ -398,13 +397,12 @@ qemuMigrationAnyCopyDisk(virDomainDiskDef const *disk, static bool qemuMigrationHasAnyStorageMigrationDisks(virDomainDef *def, - const char **migrate_disks, - size_t nmigrate_disks) + const char **migrate_disks) { size_t i; for (i = 0; i < def->ndisks; i++) { - if (qemuMigrationAnyCopyDisk(def->disks[i], nmigrate_disks, migrate_disks)) + if (qemuMigrationAnyCopyDisk(def->disks[i], migrate_disks)) return true; } @@ -415,7 +413,6 @@ qemuMigrationHasAnyStorageMigrationDisks(virDomainDef *def, static int qemuMigrationDstPrepareStorage(virDomainObj *vm, qemuMigrationCookieNBD *nbd, - size_t nmigrate_disks, const char **migrate_disks, bool incremental) { @@ -440,7 +437,7 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm, } /* Skip disks we don't want to migrate. */ - if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks)) + if (!qemuMigrationAnyCopyDisk(disk, migrate_disks)) continue; switch (virStorageSourceGetActualType(disk->src)) { @@ -547,7 +544,6 @@ static int qemuMigrationDstStartNBDServer(virQEMUDriver *driver, virDomainObj *vm, const char *listenAddr, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -614,7 +610,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, g_autofree char *diskAlias = NULL; /* check whether disk should be migrated */ - if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks)) + if (!qemuMigrationAnyCopyDisk(disk, migrate_disks)) continue; if (disk->src->readonly || virStorageSourceIsEmpty(disk->src)) { @@ -1186,7 +1182,6 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriver *driver, qemuMigrationCookie *mig, const char *host, unsigned long speed, - size_t nmigrate_disks, const char **migrate_disks, virConnectPtr dconn, const char *tlsAlias, @@ -1261,7 +1256,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriver *driver, virDomainDiskDef *disk = vm->def->disks[i]; /* check whether disk should be migrated */ - if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks)) + if (!qemuMigrationAnyCopyDisk(disk, migrate_disks)) continue; if (qemuMigrationSrcNBDStorageCopyOne(vm, disk, host, port, @@ -1614,7 +1609,6 @@ qemuMigrationSrcIsAllowed(virDomainObj *vm, static bool qemuMigrationSrcIsSafe(virDomainDef *def, virQEMUCaps *qemuCaps, - size_t nmigrate_disks, const char **migrate_disks, unsigned int flags) @@ -1638,7 +1632,7 @@ qemuMigrationSrcIsSafe(virDomainDef *def, /* Disks which are migrated by qemu are safe too. */ if (storagemigration && - qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks)) + qemuMigrationAnyCopyDisk(disk, migrate_disks)) continue; /* However, disks on local FS (e.g. ext4) are not safe. */ @@ -2437,8 +2431,7 @@ qemuMigrationAnyConnectionClosed(virDomainObj *vm, static int qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(qemuMigrationCookie *mig, virDomainObj *vm, - const char **migrate_disks, - size_t nmigrate_disks) + const char **migrate_disks) { GSList *disks = NULL; @@ -2460,7 +2453,7 @@ qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(qemuMigrationCookie *mig, if (!nodedata) continue; - if (!qemuMigrationAnyCopyDisk(diskdef, nmigrate_disks, migrate_disks)) + if (!qemuMigrationAnyCopyDisk(diskdef, migrate_disks)) continue; for (j = 0; j < nodedata->nbitmaps; j++) { @@ -2530,7 +2523,6 @@ qemuMigrationSrcBeginXML(virDomainObj *vm, int *cookieoutlen, unsigned int cookieFlags, const char **migrate_disks, - size_t nmigrate_disks, unsigned int flags) { qemuDomainObjPrivate *priv = vm->privateData; @@ -2548,8 +2540,7 @@ qemuMigrationSrcBeginXML(virDomainObj *vm, if (cookieFlags & QEMU_MIGRATION_COOKIE_NBD && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING) && - qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(mig, vm, migrate_disks, - nmigrate_disks) < 0) + qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(mig, vm, migrate_disks) < 0) return NULL; if (qemuMigrationCookieFormat(mig, driver, vm, @@ -2615,7 +2606,6 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, const char *dname, char **cookieout, int *cookieoutlen, - size_t nmigrate_disks, const char **migrate_disks, unsigned int flags) { @@ -2641,8 +2631,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return NULL; if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && - !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, - nmigrate_disks, migrate_disks, flags)) + !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, migrate_disks, flags)) return NULL; if (flags & VIR_MIGRATE_POSTCOPY && @@ -2697,9 +2686,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, priv->nbdPort = 0; - if (qemuMigrationHasAnyStorageMigrationDisks(vm->def, - migrate_disks, - nmigrate_disks)) + if (qemuMigrationHasAnyStorageMigrationDisks(vm->def, migrate_disks)) cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } else { if (migrate_disks) { @@ -2721,8 +2708,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return qemuMigrationSrcBeginXML(vm, xmlin, cookieout, cookieoutlen, cookieFlags, - migrate_disks, nmigrate_disks, - flags); + migrate_disks, flags); } @@ -2812,7 +2798,7 @@ qemuMigrationSrcBeginResume(virDomainObj *vm, } return qemuMigrationSrcBeginXML(vm, xmlin, - cookieout, cookieoutlen, 0, NULL, 0, flags); + cookieout, cookieoutlen, 0, NULL, flags); } @@ -2857,7 +2843,6 @@ qemuMigrationSrcBegin(virConnectPtr conn, const char *dname, char **cookieout, int *cookieoutlen, - size_t nmigrate_disks, const char **migrate_disks, unsigned int flags) { @@ -2908,7 +2893,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, if (!(xml = qemuMigrationSrcBeginPhase(driver, vm, xmlin, dname, cookieout, cookieoutlen, - nmigrate_disks, migrate_disks, flags))) + migrate_disks, flags))) goto endjob; if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) { @@ -3121,7 +3106,6 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, const char *protocol, unsigned short port, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -3149,8 +3133,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, goto error; } - if (qemuMigrationDstPrepareStorage(vm, mig->nbd, - nmigrate_disks, migrate_disks, + if (qemuMigrationDstPrepareStorage(vm, mig->nbd, migrate_disks, !!(flags & VIR_MIGRATE_NON_SHARED_INC)) < 0) goto error; @@ -3245,7 +3228,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, } if (qemuMigrationDstStartNBDServer(driver, vm, incoming->address, - nmigrate_disks, migrate_disks, + migrate_disks, nbdPort, nbdURI, nbdTLSAlias) < 0) { goto error; @@ -3316,7 +3299,6 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, unsigned short port, bool autoPort, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -3426,7 +3408,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, if (!(flags & VIR_MIGRATE_OFFLINE)) { if (qemuMigrationDstPrepareActive(driver, vm, dconn, mig, st, protocol, port, listenAddress, - nmigrate_disks, migrate_disks, + migrate_disks, nbdPort, nbdURI, migParams, flags) < 0) { goto stopjob; @@ -3589,7 +3571,6 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver, unsigned short port, bool autoPort, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -3652,7 +3633,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver, cookieout, cookieoutlen, def, origname, st, protocol, port, autoPort, listenAddress, - nmigrate_disks, migrate_disks, + migrate_disks, nbdPort, nbdURI, migParams, flags); } @@ -3689,7 +3670,7 @@ qemuMigrationDstPrepareTunnel(virQEMUDriver *driver, return qemuMigrationDstPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, - st, NULL, 0, false, NULL, 0, NULL, 0, + st, NULL, 0, false, NULL, NULL, 0, NULL, migParams, flags); } @@ -3728,7 +3709,6 @@ qemuMigrationDstPrepareDirect(virQEMUDriver *driver, virDomainDef **def, const char *origname, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -3859,7 +3839,7 @@ qemuMigrationDstPrepareDirect(virQEMUDriver *driver, cookieout, cookieoutlen, def, origname, NULL, uri ? uri->scheme : "tcp", port, autoPort, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, + migrate_disks, nbdPort, nbdURI, migParams, flags); cleanup: if (ret != 0) { @@ -4759,7 +4739,6 @@ qemuMigrationSrcRun(virQEMUDriver *driver, qemuMigrationSpec *spec, virConnectPtr dconn, const char *graphicsuri, - size_t nmigrate_disks, const char **migrate_disks, qemuMigrationParams *migParams, const char *nbdURI) @@ -4794,8 +4773,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, if (storageMigration) storageMigration = qemuMigrationHasAnyStorageMigrationDisks(vm->def, - migrate_disks, - nmigrate_disks); + migrate_disks); if (storageMigration) { cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; @@ -4928,7 +4906,6 @@ qemuMigrationSrcRun(virQEMUDriver *driver, if (qemuMigrationSrcNBDStorageCopy(driver, vm, mig, host, priv->migMaxBandwidth, - nmigrate_disks, migrate_disks, dconn, tlsAlias, tlsHostname, nbdURI, flags) < 0) { @@ -5198,7 +5175,6 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, unsigned long resource, virConnectPtr dconn, const char *graphicsuri, - size_t nmigrate_disks, const char **migrate_disks, qemuMigrationParams *migParams, const char *nbdURI) @@ -5279,7 +5255,7 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, - nmigrate_disks, migrate_disks, + migrate_disks, migParams, nbdURI); } @@ -5304,7 +5280,6 @@ qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, unsigned long resource, virConnectPtr dconn, const char *graphicsuri, - size_t nmigrate_disks, const char **migrate_disks, qemuMigrationParams *migParams) { @@ -5343,7 +5318,7 @@ qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, - dconn, graphicsuri, nmigrate_disks, migrate_disks, + dconn, graphicsuri, migrate_disks, migParams, NULL); cleanup: @@ -5383,7 +5358,7 @@ qemuMigrationSrcPerformResume(virQEMUDriver *driver, ret = qemuMigrationSrcPerformNative(driver, vm, NULL, NULL, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, - 0, NULL, NULL, 0, NULL, migParams, NULL); + 0, NULL, NULL, NULL, migParams, NULL); virCloseCallbacksDomainAdd(vm, conn, qemuMigrationAnyConnectionClosed); @@ -5485,12 +5460,12 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriver *driver, ret = qemuMigrationSrcPerformTunnel(driver, vm, st, NULL, NULL, NULL, 0, NULL, NULL, flags, resource, dconn, - NULL, 0, NULL, migParams); + NULL, NULL, migParams); else ret = qemuMigrationSrcPerformNative(driver, vm, NULL, NULL, uri_out, cookie, cookielen, NULL, NULL, /* No out cookie with v2 migration */ - flags, resource, dconn, NULL, 0, NULL, + flags, resource, dconn, NULL, NULL, migParams, NULL); /* Perform failed. Make sure Finish doesn't overwrite the error */ @@ -5553,7 +5528,6 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, const char *uri, const char *graphicsuri, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -5599,7 +5573,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, } else { dom_xml = qemuMigrationSrcBeginPhase(driver, vm, xmlin, dname, &cookieout, &cookieoutlen, - nmigrate_disks, migrate_disks, flags); + migrate_disks, flags); } if (!dom_xml) goto cleanup; @@ -5750,14 +5724,14 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, - nmigrate_disks, migrate_disks, + migrate_disks, migParams); } else { ret = qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, - nmigrate_disks, migrate_disks, + migrate_disks, migParams, nbdURI); } @@ -5930,7 +5904,6 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, const char *uri, const char *graphicsuri, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -6056,7 +6029,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, if (*v3proto) { ret = qemuMigrationSrcPerformPeer2Peer3(driver, sconn, dconn, dconnuri, vm, xmlin, persist_xml, dname, uri, graphicsuri, - listenAddress, nmigrate_disks, migrate_disks, + listenAddress, migrate_disks, nbdPort, nbdURI, migParams, resource, !!useParams, flags); } else { @@ -6092,7 +6065,6 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, const char *uri, const char *graphicsuri, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -6135,8 +6107,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, goto endjob; if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && - !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, - nmigrate_disks, migrate_disks, flags)) + !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, migrate_disks, flags)) goto endjob; qemuMigrationSrcStoreDomainState(vm); @@ -6145,7 +6116,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { ret = qemuMigrationSrcPerformPeer2Peer(driver, conn, vm, xmlin, persist_xml, dconnuri, uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, + migrate_disks, nbdPort, nbdURI, migParams, flags, dname, resource, &v3proto); @@ -6155,7 +6126,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, ret = qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, - flags, resource, NULL, NULL, 0, NULL, + flags, resource, NULL, NULL, NULL, migParams, nbdURI); } if (ret < 0) @@ -6222,7 +6193,6 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, const char *persist_xml, const char *uri, const char *graphicsuri, - size_t nmigrate_disks, const char **migrate_disks, qemuMigrationParams *migParams, const char *cookiein, @@ -6259,7 +6229,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, if (qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, graphicsuri, - nmigrate_disks, migrate_disks, migParams, nbdURI) < 0) + migrate_disks, migParams, nbdURI) < 0) goto cleanup; virCloseCallbacksDomainAdd(vm, conn, qemuMigrationAnyConnectionClosed); @@ -6300,7 +6270,6 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, const char *uri, const char *graphicsuri, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -6345,7 +6314,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, dconnuri, uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, + migrate_disks, nbdPort, nbdURI, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -6361,7 +6330,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, if (v3proto) { return qemuMigrationSrcPerformPhase(driver, conn, vm, xmlin, persist_xml, uri, graphicsuri, - nmigrate_disks, migrate_disks, + migrate_disks, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -6370,7 +6339,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, + migrate_disks, nbdPort, nbdURI, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, flags, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index ed62fd4a91..4dced4b166 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -122,7 +122,6 @@ qemuMigrationSrcBegin(virConnectPtr conn, const char *dname, char **cookieout, int *cookieoutlen, - size_t nmigrate_disks, const char **migrate_disks, unsigned int flags); @@ -158,7 +157,6 @@ qemuMigrationDstPrepareDirect(virQEMUDriver *driver, virDomainDef **def, const char *origname, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, @@ -175,7 +173,6 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, const char *uri, const char *graphicsuri, const char *listenAddress, - size_t nmigrate_disks, const char **migrate_disks, int nbdPort, const char *nbdURI, -- 2.46.0
The new 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' migration parameter allows users of migration to pass in a list of disks where zero-detection (which avoids transferring the zeroed-blocks) should be enabled for the migration connection. This comes at the cost of extra CPU cycles needed to check each block if it's all-zero. This is useful for storage backends where information about the allocation state of a block is not available and thus without this the image would become fully allocated on the destination. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 13 ++++ src/qemu/qemu_driver.c | 20 ++++-- src/qemu/qemu_migration.c | 105 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 4 ++ 4 files changed, 110 insertions(+), 32 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4266237abe..6d4cc69c5d 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1240,6 +1240,19 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_MIGRATE_DISKS "migrate_disks" +/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices for which zero detection (to avoid transferring zero blocks) + * is to be enabled. This may increase CPU overhead of the migration. At the + * moment this is only supported by the QEMU driver but not for the tunnelled + * migration. + * + * Since: 10.9.0 + */ +# define VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES "migrate_disks_detect_zeroes" + /** * VIR_MIGRATE_PARAM_DISKS_PORT: * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cec700a36c..472bcd1fd3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10782,7 +10782,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationSrcPerform(driver, dom->conn, vm, NULL, - NULL, dconnuri, uri, NULL, NULL, NULL, 0, + NULL, dconnuri, uri, NULL, NULL, NULL, NULL, 0, NULL, migParams, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ @@ -10858,7 +10858,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, } return qemuMigrationSrcBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, NULL, flags); + cookieout, cookieoutlen, NULL, NULL, flags); } static char * @@ -10872,6 +10872,7 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, const char *xmlin = NULL; const char *dname = NULL; g_autofree const char **migrate_disks = NULL; + g_autofree const char **migrate_disks_detect_zeroes = NULL; virDomainObj *vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); @@ -10889,6 +10890,10 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, &migrate_disks); + virTypedParamsGetStringList(params, nparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, + &migrate_disks_detect_zeroes); + if (!(vm = qemuDomainObjFromDomain(domain))) return NULL; @@ -10899,7 +10904,8 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, return qemuMigrationSrcBegin(domain->conn, vm, xmlin, dname, cookieout, cookieoutlen, - migrate_disks, flags); + migrate_disks, migrate_disks_detect_zeroes, + flags); } @@ -11180,7 +11186,7 @@ qemuDomainMigratePerform3(virDomainPtr dom, goto cleanup; ret = qemuMigrationSrcPerform(driver, dom->conn, vm, xmlin, NULL, - dconnuri, uri, NULL, NULL, NULL, 0, + dconnuri, uri, NULL, NULL, NULL, NULL, 0, NULL, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -11211,6 +11217,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, const char *graphicsuri = NULL; const char *listenAddress = NULL; g_autofree const char **migrate_disks = NULL; + g_autofree const char **migrate_disks_detect_zeroes = NULL; unsigned long long bandwidth = 0; int nbdPort = 0; g_autoptr(qemuMigrationParams) migParams = NULL; @@ -11267,6 +11274,9 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, &migrate_disks); + virTypedParamsGetStringList(params, nparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, + &migrate_disks_detect_zeroes); if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) || migrate_disks) { @@ -11289,7 +11299,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, ret = qemuMigrationSrcPerform(driver, dom->conn, vm, dom_xml, persist_xml, dconnuri, uri, graphicsuri, listenAddress, - migrate_disks, nbdPort, + migrate_disks, migrate_disks_detect_zeroes, nbdPort, nbdURI, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, bandwidth, true); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 94636e778d..7ae19dd0ce 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1022,7 +1022,8 @@ qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDef *disk, int port, const char *socket, const char *tlsAlias, - const char *tlsHostname) + const char *tlsHostname, + bool detect_zeroes) { g_autoptr(virStorageSource) copysrc = NULL; @@ -1031,6 +1032,9 @@ qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDef *disk, copysrc->protocol = VIR_STORAGE_NET_PROTOCOL_NBD; copysrc->format = VIR_STORAGE_FILE_RAW; + if (detect_zeroes) + copysrc->detect_zeroes = VIR_DOMAIN_DISK_DETECT_ZEROES_ON; + copysrc->backingStore = virStorageSourceNew(); if (!(copysrc->path = qemuAliasDiskDriveFromDisk(disk))) @@ -1067,7 +1071,8 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virDomainObj *vm, unsigned int mirror_shallow, const char *tlsAlias, const char *tlsHostname, - bool syncWrites) + bool syncWrites, + bool detect_zeroes) { g_autoptr(qemuBlockStorageSourceAttachData) data = NULL; qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); @@ -1081,7 +1086,8 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virDomainObj *vm, VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", disk->dst, host); if (!(copysrc = qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(disk, host, port, socket, - tlsAlias, tlsHostname))) + tlsAlias, tlsHostname, + detect_zeroes))) return -1; if (!(data = qemuBlockStorageSourceAttachPrepareBlockdev(copysrc, @@ -1123,6 +1129,7 @@ qemuMigrationSrcNBDStorageCopyOne(virDomainObj *vm, bool mirror_shallow, const char *tlsAlias, const char *tlsHostname, + bool detect_zeroes, unsigned int flags) { qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); @@ -1147,7 +1154,8 @@ qemuMigrationSrcNBDStorageCopyOne(virDomainObj *vm, mirror_shallow, tlsAlias, tlsHostname, - syncWrites); + syncWrites, + detect_zeroes); if (rc == 0) { diskPriv->migrating = true; @@ -1183,6 +1191,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriver *driver, const char *host, unsigned long speed, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, virConnectPtr dconn, const char *tlsAlias, const char *tlsHostname, @@ -1254,15 +1263,20 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriver *driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDef *disk = vm->def->disks[i]; + bool detect_zeroes = false; /* check whether disk should be migrated */ if (!qemuMigrationAnyCopyDisk(disk, migrate_disks)) continue; + if (migrate_disks_detect_zeroes) + detect_zeroes = g_strv_contains(migrate_disks_detect_zeroes, disk->dst); + if (qemuMigrationSrcNBDStorageCopyOne(vm, disk, host, port, socket, mirror_speed, mirror_shallow, - tlsAlias, tlsHostname, flags) < 0) + tlsAlias, tlsHostname, detect_zeroes, + flags) < 0) return -1; if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { @@ -2607,6 +2621,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, char **cookieout, int *cookieoutlen, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, unsigned int flags) { qemuDomainObjPrivate *priv = vm->privateData; @@ -2614,10 +2629,10 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s," " cookieout=%p, cookieoutlen=%p," - " migrate_disks=%p, flags=0x%x", + " migrate_disks=%p, migrate_disks_detect_zeroes=%p, flags=0x%x", driver, vm, NULLSTR(xmlin), NULLSTR(dname), cookieout, cookieoutlen, - migrate_disks, flags); + migrate_disks, migrate_disks_detect_zeroes, flags); /* Only set the phase if we are inside VIR_ASYNC_JOB_MIGRATION_OUT. * Otherwise we will start the async job later in the perform phase losing @@ -2684,6 +2699,10 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, qemuMigrationSrcBeginPhaseValidateDiskTargetList(vm, migrate_disks) < 0) return NULL; + if (migrate_disks_detect_zeroes && + qemuMigrationSrcBeginPhaseValidateDiskTargetList(vm, migrate_disks_detect_zeroes) < 0) + return NULL; + priv->nbdPort = 0; if (qemuMigrationHasAnyStorageMigrationDisks(vm->def, migrate_disks)) @@ -2694,6 +2713,13 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, _("use of 'VIR_MIGRATE_PARAM_MIGRATE_DISKS' requires use of 'VIR_MIGRATE_NON_SHARED_DISK' or 'VIR_MIGRATE_NON_SHARED_INC' flag")); return NULL; } + + if (migrate_disks_detect_zeroes) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("use of 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' requires use of 'VIR_MIGRATE_NON_SHARED_DISK' or 'VIR_MIGRATE_NON_SHARED_INC' flag")); + return NULL; + } + } if (virDomainDefHasMemoryHotplug(vm->def) || @@ -2844,6 +2870,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, char **cookieout, int *cookieoutlen, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, unsigned int flags) { virQEMUDriver *driver = conn->privateData; @@ -2893,7 +2920,9 @@ qemuMigrationSrcBegin(virConnectPtr conn, if (!(xml = qemuMigrationSrcBeginPhase(driver, vm, xmlin, dname, cookieout, cookieoutlen, - migrate_disks, flags))) + migrate_disks, + migrate_disks_detect_zeroes, + flags))) goto endjob; if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) { @@ -4740,6 +4769,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, virConnectPtr dconn, const char *graphicsuri, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, qemuMigrationParams *migParams, const char *nbdURI) { @@ -4765,11 +4795,11 @@ qemuMigrationSrcRun(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%x, resource=%lu, " "spec=%p (dest=%d, fwd=%d), dconn=%p, graphicsuri=%s, " - "migrate_disks=%p", + "migrate_disks=%p, migrate_disks_detect_zeroes=%p", driver, vm, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource, spec, spec->destType, spec->fwdType, dconn, - NULLSTR(graphicsuri), migrate_disks); + NULLSTR(graphicsuri), migrate_disks, migrate_disks_detect_zeroes); if (storageMigration) storageMigration = qemuMigrationHasAnyStorageMigrationDisks(vm->def, @@ -4907,6 +4937,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, host, priv->migMaxBandwidth, migrate_disks, + migrate_disks_detect_zeroes, dconn, tlsAlias, tlsHostname, nbdURI, flags) < 0) { goto error; @@ -5176,6 +5207,7 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, virConnectPtr dconn, const char *graphicsuri, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, qemuMigrationParams *migParams, const char *nbdURI) { @@ -5186,10 +5218,10 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, VIR_DEBUG("driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%x, resource=%lu, " - "graphicsuri=%s, migrate_disks=%p", + "graphicsuri=%s, migrate_disks=%p, migrate_disks_detect_zeroes=%p", driver, vm, uri, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource, - NULLSTR(graphicsuri), migrate_disks); + NULLSTR(graphicsuri), migrate_disks, migrate_disks_detect_zeroes); if (!(uribits = qemuMigrationAnyParseURI(uri, NULL))) return -1; @@ -5255,7 +5287,7 @@ qemuMigrationSrcPerformNative(virQEMUDriver *driver, ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, - migrate_disks, + migrate_disks, migrate_disks_detect_zeroes, migParams, nbdURI); } @@ -5316,9 +5348,11 @@ qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, goto cleanup; } + /* Migration with NBD is not supported with _TUNNELED, thus + * 'migrate_disks_detect_zeroes' is NULL here */ ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, - dconn, graphicsuri, migrate_disks, + dconn, graphicsuri, migrate_disks, NULL, migParams, NULL); cleanup: @@ -5358,7 +5392,7 @@ qemuMigrationSrcPerformResume(virQEMUDriver *driver, ret = qemuMigrationSrcPerformNative(driver, vm, NULL, NULL, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, - 0, NULL, NULL, NULL, migParams, NULL); + 0, NULL, NULL, NULL, NULL, migParams, NULL); virCloseCallbacksDomainAdd(vm, conn, qemuMigrationAnyConnectionClosed); @@ -5466,7 +5500,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriver *driver, cookie, cookielen, NULL, NULL, /* No out cookie with v2 migration */ flags, resource, dconn, NULL, NULL, - migParams, NULL); + NULL, migParams, NULL); /* Perform failed. Make sure Finish doesn't overwrite the error */ if (ret < 0) @@ -5529,6 +5563,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, const char *graphicsuri, const char *listenAddress, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, int nbdPort, const char *nbdURI, qemuMigrationParams *migParams, @@ -5555,11 +5590,11 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, VIR_DEBUG("driver=%p, sconn=%p, dconn=%p, dconnuri=%s, vm=%p, xmlin=%s, " "dname=%s, uri=%s, graphicsuri=%s, listenAddress=%s, " - "migrate_disks=%p, nbdPort=%d, nbdURI=%s, " + "migrate_disks=%p, migrate_disks_detect_zeroes=%p, nbdPort=%d, nbdURI=%s, " "bandwidth=%llu, useParams=%d, flags=0x%x", driver, sconn, dconn, NULLSTR(dconnuri), vm, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri), NULLSTR(graphicsuri), - NULLSTR(listenAddress), migrate_disks, nbdPort, + NULLSTR(listenAddress), migrate_disks, migrate_disks_detect_zeroes, nbdPort, NULLSTR(nbdURI), bandwidth, useParams, flags); /* Unlike the virDomainMigrateVersion3 counterpart, we don't need @@ -5573,7 +5608,9 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, } else { dom_xml = qemuMigrationSrcBeginPhase(driver, vm, xmlin, dname, &cookieout, &cookieoutlen, - migrate_disks, flags); + migrate_disks, + migrate_disks_detect_zeroes, + flags); } if (!dom_xml) goto cleanup; @@ -5618,6 +5655,15 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, *d) < 0) goto cleanup; } + if (migrate_disks_detect_zeroes) { + const char **d; + + for (d = migrate_disks_detect_zeroes; *d; d++) + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, + *d) < 0) + goto cleanup; + } if (nbdPort && virTypedParamsAddInt(¶ms, &nparams, &maxparams, VIR_MIGRATE_PARAM_DISKS_PORT, @@ -5731,7 +5777,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, - migrate_disks, + migrate_disks, migrate_disks_detect_zeroes, migParams, nbdURI); } @@ -5905,6 +5951,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, const char *graphicsuri, const char *listenAddress, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, int nbdPort, const char *nbdURI, qemuMigrationParams *migParams, @@ -6029,7 +6076,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, if (*v3proto) { ret = qemuMigrationSrcPerformPeer2Peer3(driver, sconn, dconn, dconnuri, vm, xmlin, persist_xml, dname, uri, graphicsuri, - listenAddress, migrate_disks, + listenAddress, migrate_disks, migrate_disks_detect_zeroes, nbdPort, nbdURI, migParams, resource, !!useParams, flags); } else { @@ -6066,6 +6113,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, const char *graphicsuri, const char *listenAddress, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, int nbdPort, const char *nbdURI, qemuMigrationParams *migParams, @@ -6116,7 +6164,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { ret = qemuMigrationSrcPerformPeer2Peer(driver, conn, vm, xmlin, persist_xml, dconnuri, uri, graphicsuri, listenAddress, - migrate_disks, nbdPort, + migrate_disks, migrate_disks_detect_zeroes, nbdPort, nbdURI, migParams, flags, dname, resource, &v3proto); @@ -6126,7 +6174,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, ret = qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, - flags, resource, NULL, NULL, NULL, + flags, resource, NULL, NULL, NULL, NULL, migParams, nbdURI); } if (ret < 0) @@ -6194,6 +6242,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, const char *uri, const char *graphicsuri, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, qemuMigrationParams *migParams, const char *cookiein, int cookieinlen, @@ -6229,7 +6278,8 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, if (qemuMigrationSrcPerformNative(driver, vm, xmlin, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, graphicsuri, - migrate_disks, migParams, nbdURI) < 0) + migrate_disks, migrate_disks_detect_zeroes, + migParams, nbdURI) < 0) goto cleanup; virCloseCallbacksDomainAdd(vm, conn, qemuMigrationAnyConnectionClosed); @@ -6271,6 +6321,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, const char *graphicsuri, const char *listenAddress, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, int nbdPort, const char *nbdURI, qemuMigrationParams *migParams, @@ -6314,7 +6365,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, dconnuri, uri, graphicsuri, listenAddress, - migrate_disks, nbdPort, + migrate_disks, migrate_disks_detect_zeroes, nbdPort, nbdURI, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -6330,7 +6381,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, if (v3proto) { return qemuMigrationSrcPerformPhase(driver, conn, vm, xmlin, persist_xml, uri, graphicsuri, - migrate_disks, + migrate_disks, migrate_disks_detect_zeroes, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -6339,7 +6390,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, uri, graphicsuri, listenAddress, - migrate_disks, nbdPort, + migrate_disks, migrate_disks_detect_zeroes, nbdPort, nbdURI, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, flags, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 4dced4b166..4b7ef9688a 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -74,6 +74,8 @@ VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_TYPED_PARAM_STRING, \ VIR_MIGRATE_PARAM_MIGRATE_DISKS, VIR_TYPED_PARAM_STRING | \ VIR_TYPED_PARAM_MULTIPLE, \ + VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, VIR_TYPED_PARAM_STRING | \ + VIR_TYPED_PARAM_MULTIPLE, \ VIR_MIGRATE_PARAM_DISKS_PORT, VIR_TYPED_PARAM_INT, \ VIR_MIGRATE_PARAM_COMPRESSION, VIR_TYPED_PARAM_STRING | \ VIR_TYPED_PARAM_MULTIPLE, \ @@ -123,6 +125,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, char **cookieout, int *cookieoutlen, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, unsigned int flags); virDomainDef * @@ -174,6 +177,7 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, const char *graphicsuri, const char *listenAddress, const char **migrate_disks, + const char **migrate_disks_detect_zeroes, int nbdPort, const char *nbdURI, qemuMigrationParams *migParams, -- 2.46.0
On Mon, Sep 30, 2024 at 03:29:34PM +0200, Peter Krempa wrote:
The new 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' migration parameter allows users of migration to pass in a list of disks where zero-detection (which avoids transferring the zeroed-blocks) should be enabled for the migration connection. This comes at the cost of extra CPU cycles needed to check each block if it's all-zero.
This is useful for storage backends where information about the allocation state of a block is not available and thus without this the image would become fully allocated on the destination.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 13 ++++ src/qemu/qemu_driver.c | 20 ++++-- src/qemu/qemu_migration.c | 105 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 4 ++ 4 files changed, 110 insertions(+), 32 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4266237abe..6d4cc69c5d 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1240,6 +1240,19 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_MIGRATE_DISKS "migrate_disks"
+/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices for which zero detection (to avoid transferring zero blocks) + * is to be enabled. This may increase CPU overhead of the migration. At the + * moment this is only supported by the QEMU driver but not for the tunnelled + * migration.
We should note that it has to be subset of migrate_disks values. I also wonder if we should add a code that will error out if it's not the case, currently it would be silently ignored.
+ * + * Since: 10.9.0 + */ +# define VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES "migrate_disks_detect_zeroes" + /** * VIR_MIGRATE_PARAM_DISKS_PORT: *
[...]
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 94636e778d..7ae19dd0ce 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5316,9 +5348,11 @@ qemuMigrationSrcPerformTunnel(virQEMUDriver *driver, goto cleanup; }
+ /* Migration with NBD is not supported with _TUNNELED, thus
s/_TUNNELED/_TUNNELLED/ To match the incorrect spelling of the actual flag.
+ * 'migrate_disks_detect_zeroes' is NULL here */ ret = qemuMigrationSrcRun(driver, vm, xmlin, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, - dconn, graphicsuri, migrate_disks, + dconn, graphicsuri, migrate_disks, NULL, migParams, NULL);
cleanup:
Pavel
On Mon, Sep 30, 2024 at 16:53:55 +0200, Pavel Hrdina wrote:
On Mon, Sep 30, 2024 at 03:29:34PM +0200, Peter Krempa wrote:
The new 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' migration parameter allows users of migration to pass in a list of disks where zero-detection (which avoids transferring the zeroed-blocks) should be enabled for the migration connection. This comes at the cost of extra CPU cycles needed to check each block if it's all-zero.
This is useful for storage backends where information about the allocation state of a block is not available and thus without this the image would become fully allocated on the destination.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 13 ++++ src/qemu/qemu_driver.c | 20 ++++-- src/qemu/qemu_migration.c | 105 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 4 ++ 4 files changed, 110 insertions(+), 32 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4266237abe..6d4cc69c5d 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1240,6 +1240,19 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_MIGRATE_DISKS "migrate_disks"
+/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices for which zero detection (to avoid transferring zero blocks) + * is to be enabled. This may increase CPU overhead of the migration. At the + * moment this is only supported by the QEMU driver but not for the tunnelled + * migration.
We should note that it has to be subset of migrate_disks values.
I'd argue that it's logical that this can apply only if the disk is being migrated.
I also wonder if we should add a code that will error out if it's not the case, currently it would be silently ignored.
I'm firmly in the 'no' region. This can be used also if 'migrate_disks' is not used so we'd have to cross check against the logic if a disk is even being migrated. As written above I'm okay declaring that we "did in fact enable zero-detection for given migration" if the disk was not migrated as all zeroes were properly 100% absolutely surely always detected during that migration. Yes I'm being sarcastic. No I'll not add this pointless checking code. I was borderline thinking not adding the check whether the disk targets are valid, but that could be easily extracted&reused.
On Mon, Sep 30, 2024 at 06:34:05PM +0200, Peter Krempa wrote:
On Mon, Sep 30, 2024 at 16:53:55 +0200, Pavel Hrdina wrote:
On Mon, Sep 30, 2024 at 03:29:34PM +0200, Peter Krempa wrote:
The new 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' migration parameter allows users of migration to pass in a list of disks where zero-detection (which avoids transferring the zeroed-blocks) should be enabled for the migration connection. This comes at the cost of extra CPU cycles needed to check each block if it's all-zero.
This is useful for storage backends where information about the allocation state of a block is not available and thus without this the image would become fully allocated on the destination.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 13 ++++ src/qemu/qemu_driver.c | 20 ++++-- src/qemu/qemu_migration.c | 105 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 4 ++ 4 files changed, 110 insertions(+), 32 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4266237abe..6d4cc69c5d 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1240,6 +1240,19 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_MIGRATE_DISKS "migrate_disks"
+/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices for which zero detection (to avoid transferring zero blocks) + * is to be enabled. This may increase CPU overhead of the migration. At the + * moment this is only supported by the QEMU driver but not for the tunnelled + * migration.
We should note that it has to be subset of migrate_disks values.
I'd argue that it's logical that this can apply only if the disk is being migrated.
My assumption was that it would apply only to disks listed in VIR_MIGRATE_PARAM_MIGRATE_DISKS but due to incorrectly reading how qemuMigrationAnyCopyDisk works it's not true so that comment would be false as well. With the _TUNNELLED fixed Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Expose the new parameter as '--migrate-disks-detect-zeroes' option. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 8 +++++++- tools/virsh-domain.c | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index f02a28156d..6665d46497 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3389,7 +3389,9 @@ migrate [--postcopy-after-precopy] [--postcopy-resume] [--zerocopy] domain desturi [migrateuri] [graphicsuri] [listen-address] [dname] [--timeout seconds [--timeout-suspend | --timeout-postcopy]] - [--xml file] [--migrate-disks disk-list] [--disks-port port] + [--xml file] + [--migrate-disks disk-list] [--migrate-disks-detect-zeroes disk-list] + [--disks-port port] [--compressed] [--comp-methods method-list] [--comp-mt-level] [--comp-mt-threads] [--comp-mt-dthreads] [--comp-xbzrle-cache] [--comp-zlib-level] [--comp-zstd-level] @@ -3420,6 +3422,10 @@ images on source host to the images found at the same place on the destination host. By default only non-shared non-readonly images are transferred. Use *--migrate-disks* to explicitly specify a list of disk targets to transfer via the comma separated ``disk-list`` argument. +The *--migrate-disks-detect-zeroes* option which takes a comma separated list of +disk target names enables zeroed block detection for the listed migrated disks. +These blocks are not transferred or allocated on destination, effectively +sparsifying the disk at the cost of CPU overhead. With *--copy-storage-synchronous-writes* flag used the disk data migration will synchronously handle guest disk writes to both the original source and the destination to ensure that the disk migration converges at the price of possibly diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 50e80689a2..e4923284af 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10691,6 +10691,11 @@ static const vshCmdOptDef opts_migrate[] = { .completer = virshDomainMigrateDisksCompleter, .help = N_("comma separated list of disks to be migrated") }, + {.name = "migrate-disks-detect-zeroes", + .type = VSH_OT_STRING, + .completer = virshDomainMigrateDisksCompleter, + .help = N_("comma separated list of disks to be migrated with zero detection enabled") + }, {.name = "disks-port", .type = VSH_OT_INT, .unwanted_positional = true, @@ -10909,6 +10914,27 @@ doMigrate(void *opaque) } } + if (vshCommandOptString(ctl, cmd, "migrate-disks-detect-zeroes", &opt) < 0) + goto out; + if (opt) { + g_autofree char **val = NULL; + + if (!(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC))) { + vshError(ctl, "'--migrate-disks-detect-zeroes' requires one of '--copy-storage-all', '--copy-storage-inc'"); + goto out; + } + + val = g_strsplit(opt, ",", 0); + + if (virTypedParamsAddStringList(¶ms, + &nparams, + &maxparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, + (const char **)val) < 0) { + goto save_error; + } + } + if (vshCommandOptString(ctl, cmd, "comp-methods", &opt) < 0) goto out; if (opt) { -- 2.46.0
On Mon, Sep 30, 2024 at 03:29:35PM +0200, Peter Krempa wrote:
Expose the new parameter as '--migrate-disks-detect-zeroes' option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 8 +++++++- tools/virsh-domain.c | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index f02a28156d..6665d46497 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3389,7 +3389,9 @@ migrate [--postcopy-after-precopy] [--postcopy-resume] [--zerocopy] domain desturi [migrateuri] [graphicsuri] [listen-address] [dname] [--timeout seconds [--timeout-suspend | --timeout-postcopy]] - [--xml file] [--migrate-disks disk-list] [--disks-port port] + [--xml file] + [--migrate-disks disk-list] [--migrate-disks-detect-zeroes disk-list] + [--disks-port port] [--compressed] [--comp-methods method-list] [--comp-mt-level] [--comp-mt-threads] [--comp-mt-dthreads] [--comp-xbzrle-cache] [--comp-zlib-level] [--comp-zstd-level] @@ -3420,6 +3422,10 @@ images on source host to the images found at the same place on the destination host. By default only non-shared non-readonly images are transferred. Use *--migrate-disks* to explicitly specify a list of disk targets to transfer via the comma separated ``disk-list`` argument. +The *--migrate-disks-detect-zeroes* option which takes a comma separated list of +disk target names enables zeroed block detection for the listed migrated disks.
How about this wording: Use *--migrate-disks-detect-zeroes* to specify which disks from *--migrate-disks* list of disks should have zeroed block detection enabled via the comma separated ``disk-list`` argument.
+These blocks are not transferred or allocated on destination, effectively +sparsifying the disk at the cost of CPU overhead. With *--copy-storage-synchronous-writes* flag used the disk data migration will synchronously handle guest disk writes to both the original source and the destination to ensure that the disk migration converges at the price of possibly
Pavel
On Mon, Sep 30, 2024 at 03:29:35PM +0200, Peter Krempa wrote:
Expose the new parameter as '--migrate-disks-detect-zeroes' option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 8 +++++++- tools/virsh-domain.c | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
On Mon, Sep 30, 2024 at 03:29:22PM +0200, Peter Krempa wrote:
See 12/13 for rationale.
The first patch fixes allocation setting when pre-creating qcow2 images for migration.
Next few patches fix/improve typed parameter list handling.
Next few refactor handling of 'migrate_disks' parameter and the final two implement the new feature.
Peter Krempa (13): qemu: migration: Pre-create QCOW2 images for non-shared storage with 0 allocation virTypedParamsFilter: Adjust return type and docs virTypedParamsGetStringList: Refactor and adjust docs virTypedParamsFilter: Introduce option to filter also by type virTypedParamsGetStringList: Ensure that returned array is NULL if there are no matching fields virTypedParamsGetStringList: Ensure that returned string list is NULL-terminated qemuMigrationSrcBeginPhaseBlockDirtyBitmaps: Use qemuMigrationAnyCopyDisk() qemu: migration: Don't log 'nmigrate_disks' qemu: migration: Avoid use of 'nmigration_disks' qemu: migration: Extract validation of disk target list qemu: migration: Remove 'nmigration_disks' variable from all places
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemu: Introduce and wire in 'VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES' virsh: Add support for VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES migration parameter
See comments for these two patches. Pavel
participants (2)
-
Pavel Hrdina -
Peter Krempa