Add support for parallel save and restore by mapping libvirt's
"parallel-connections" parameter to QEMU's "multifd-channels"
migration parameter.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
src/qemu/qemu_driver.c | 31 ++++++++++++++++++++-----------
src/qemu/qemu_migration_params.c | 25 +++++++++++++++++++++++--
src/qemu/qemu_migration_params.h | 5 ++++-
3 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0025bad6e7..fce599b321 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2608,6 +2608,8 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
int compressed,
virCommand *compressor,
const char *xmlin,
+ virTypedParameterPtr params,
+ int nparams,
unsigned int flags)
{
g_autofree char *xml = NULL;
@@ -2691,7 +2693,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
xml = NULL;
mappedRam = data->header.features & QEMU_SAVE_FEATURE_MAPPED_RAM;
- if (!(saveParams = qemuMigrationParamsForSave(mappedRam, flags)))
+ if (!(saveParams = qemuMigrationParamsForSave(params, nparams, mappedRam, flags)))
goto endjob;
ret = qemuSaveImageCreate(driver, vm, path, data, compressor,
@@ -2777,7 +2779,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, path);
if (qemuDomainSaveInternal(driver, vm, path, compressed,
- compressor, dxml, flags) < 0)
+ compressor, dxml, NULL, 0, flags) < 0)
return -1;
vm->hasManagedSave = true;
@@ -2816,7 +2818,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
goto cleanup;
ret = qemuDomainSaveInternal(driver, vm, path, compressed,
- compressor, dxml, flags);
+ compressor, dxml, NULL, 0, flags);
cleanup:
virDomainObjEndAPI(&vm);
@@ -2846,13 +2848,16 @@ qemuDomainSaveParams(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
VIR_DOMAIN_SAVE_RUNNING |
- VIR_DOMAIN_SAVE_PAUSED, -1);
+ VIR_DOMAIN_SAVE_PAUSED |
+ VIR_DOMAIN_SAVE_PARALLEL, -1);
if (virTypedParamsValidate(params, nparams,
VIR_DOMAIN_SAVE_PARAM_FILE,
VIR_TYPED_PARAM_STRING,
VIR_DOMAIN_SAVE_PARAM_DXML,
VIR_TYPED_PARAM_STRING,
+ VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS,
+ VIR_TYPED_PARAM_INT,
NULL) < 0)
return -1;
@@ -2884,7 +2889,7 @@ qemuDomainSaveParams(virDomainPtr dom,
goto cleanup;
ret = qemuDomainSaveInternal(driver, vm, to, compressed,
- compressor, dxml, flags);
+ compressor, dxml, params, nparams, flags);
cleanup:
virDomainObjEndAPI(&vm);
@@ -5759,6 +5764,8 @@ static int
qemuDomainRestoreInternal(virConnectPtr conn,
const char *path,
const char *dxml,
+ virTypedParameterPtr params,
+ int nparams,
unsigned int flags,
int (*ensureACL)(virConnectPtr, virDomainDef *))
{
@@ -5780,7 +5787,8 @@ qemuDomainRestoreInternal(virConnectPtr conn,
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED |
- VIR_DOMAIN_SAVE_RESET_NVRAM, -1);
+ VIR_DOMAIN_SAVE_RESET_NVRAM |
+ VIR_DOMAIN_SAVE_PARALLEL, -1);
if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM)
reset_nvram = true;
@@ -5789,7 +5797,7 @@ qemuDomainRestoreInternal(virConnectPtr conn,
goto cleanup;
mapped_ram = data->header.features & QEMU_SAVE_FEATURE_MAPPED_RAM;
- if (!(restoreParams = qemuMigrationParamsForSave(mapped_ram, flags)))
+ if (!(restoreParams = qemuMigrationParamsForSave(params, nparams, mapped_ram, flags)))
return -1;
fd = qemuSaveImageOpen(driver, path,
@@ -5871,7 +5879,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
const char *dxml,
unsigned int flags)
{
- return qemuDomainRestoreInternal(conn, path, dxml, flags,
+ return qemuDomainRestoreInternal(conn, path, dxml, NULL, 0, flags,
virDomainRestoreFlagsEnsureACL);
}
@@ -5879,7 +5887,7 @@ static int
qemuDomainRestore(virConnectPtr conn,
const char *path)
{
- return qemuDomainRestoreInternal(conn, path, NULL, 0,
+ return qemuDomainRestoreInternal(conn, path, NULL, NULL, 0, 0,
virDomainRestoreEnsureACL);
}
@@ -5895,6 +5903,7 @@ qemuDomainRestoreParams(virConnectPtr conn,
if (virTypedParamsValidate(params, nparams,
VIR_DOMAIN_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING,
VIR_DOMAIN_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING,
+ VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT,
NULL) < 0)
return -1;
@@ -5911,7 +5920,7 @@ qemuDomainRestoreParams(virConnectPtr conn,
return -1;
}
- ret = qemuDomainRestoreInternal(conn, path, dxml, flags,
+ ret = qemuDomainRestoreInternal(conn, path, dxml, params, nparams, flags,
virDomainRestoreParamsEnsureACL);
return ret;
}
@@ -6115,7 +6124,7 @@ qemuDomainObjRestore(virConnectPtr conn,
}
mapped_ram = data->header.features & QEMU_SAVE_FEATURE_MAPPED_RAM;
- if (!(restoreParams = qemuMigrationParamsForSave(mapped_ram,
+ if (!(restoreParams = qemuMigrationParamsForSave(NULL, 0, mapped_ram,
bypass_cache ? VIR_DOMAIN_SAVE_BYPASS_CACHE : 0)))
return -1;
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 8f6003005c..8c656af163 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -788,7 +788,10 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
qemuMigrationParams *
-qemuMigrationParamsForSave(bool mappedRam, unsigned int flags)
+qemuMigrationParamsForSave(virTypedParameterPtr params,
+ int nparams,
+ bool mappedRam,
+ unsigned int flags)
{
g_autoptr(qemuMigrationParams) saveParams = NULL;
@@ -800,7 +803,25 @@ qemuMigrationParamsForSave(bool mappedRam, unsigned int flags)
return NULL;
if (virBitmapSetBit(saveParams->caps, QEMU_MIGRATION_CAP_MULTIFD) < 0)
return NULL;
- saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = 1;
+
+ if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
+ int nchannels;
+
+ if (params && virTypedParamsGetInt(params, nparams,
+ VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS,
+ &nchannels) < 0)
+ return NULL;
+
+ if (nchannels < 1) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("number of parallel save channels cannot be less than 1"));
+ return NULL;
+ }
+
+ saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = nchannels;
+ } else {
+ saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = 1;
+ }
saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].set = true;
if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) {
diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h
index 9700469b5e..a672b3d875 100644
--- a/src/qemu/qemu_migration_params.h
+++ b/src/qemu/qemu_migration_params.h
@@ -88,7 +88,10 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
qemuMigrationParty party);
qemuMigrationParams *
-qemuMigrationParamsForSave(bool mappedRam, unsigned int flags);
+qemuMigrationParamsForSave(virTypedParameterPtr params,
+ int nparams,
+ bool mappedRam,
+ unsigned int flags);
int
qemuMigrationParamsDump(qemuMigrationParams *migParams,
--
2.35.3On Thu, Aug 08, 2024 at 05:38:11PM -0600, Jim Fehlig via Devel wrote:
> Add support for parallel save and restore by mapping libvirt's
> "parallel-connections" parameter to QEMU's "multifd-channels"
> migration parameter.
>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> src/qemu/qemu_driver.c | 31 ++++++++++++++++++++-----------
> src/qemu/qemu_migration_params.c | 25 +++++++++++++++++++++++--
> src/qemu/qemu_migration_params.h | 5 ++++-
> 3 files changed, 47 insertions(+), 14 deletions(-)
Unless I'm missing something, nothing in this patch is validating
the the image is about to be written in v3 format with MAPPED_RAM
set. We must reject VIR_DOMAIN_SAVE_PARALLEL if we're not using
MAPPED_RAM. Likewise when restoring we must reject PARALLEL if
the loaded image doesnt have MAPPED_RAM feature set.
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0025bad6e7..fce599b321 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2608,6 +2608,8 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
> int compressed,
> virCommand *compressor,
> const char *xmlin,
> + virTypedParameterPtr params,
> + int nparams,
> unsigned int flags)
> {
> g_autofree char *xml = NULL;
> @@ -2691,7 +2693,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
> xml = NULL;
>
> mappedRam = data->header.features & QEMU_SAVE_FEATURE_MAPPED_RAM;
> - if (!(saveParams = qemuMigrationParamsForSave(mappedRam, flags)))
> + if (!(saveParams = qemuMigrationParamsForSave(params, nparams, mappedRam, flags)))
> goto endjob;
>
> ret = qemuSaveImageCreate(driver, vm, path, data, compressor,
> @@ -2777,7 +2779,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
> VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, path);
>
> if (qemuDomainSaveInternal(driver, vm, path, compressed,
> - compressor, dxml, flags) < 0)
> + compressor, dxml, NULL, 0, flags) < 0)
> return -1;
>
> vm->hasManagedSave = true;
> @@ -2816,7 +2818,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
> goto cleanup;
>
> ret = qemuDomainSaveInternal(driver, vm, path, compressed,
> - compressor, dxml, flags);
> + compressor, dxml, NULL, 0, flags);
>
> cleanup:
> virDomainObjEndAPI(&vm);
> @@ -2846,13 +2848,16 @@ qemuDomainSaveParams(virDomainPtr dom,
>
> virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
> VIR_DOMAIN_SAVE_RUNNING |
> - VIR_DOMAIN_SAVE_PAUSED, -1);
> + VIR_DOMAIN_SAVE_PAUSED |
> + VIR_DOMAIN_SAVE_PARALLEL, -1);
>
> if (virTypedParamsValidate(params, nparams,
> VIR_DOMAIN_SAVE_PARAM_FILE,
> VIR_TYPED_PARAM_STRING,
> VIR_DOMAIN_SAVE_PARAM_DXML,
> VIR_TYPED_PARAM_STRING,
> + VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS,
> + VIR_TYPED_PARAM_INT,
> NULL) < 0)
> return -1;
>
> @@ -2884,7 +2889,7 @@ qemuDomainSaveParams(virDomainPtr dom,
> goto cleanup;
>
> ret = qemuDomainSaveInternal(driver, vm, to, compressed,
> - compressor, dxml, flags);
> + compressor, dxml, params, nparams, flags);
>
> cleanup:
> virDomainObjEndAPI(&vm);
> @@ -5759,6 +5764,8 @@ static int
> qemuDomainRestoreInternal(virConnectPtr conn,
> const char *path,
> const char *dxml,
> + virTypedParameterPtr params,
> + int nparams,
> unsigned int flags,
> int (*ensureACL)(virConnectPtr, virDomainDef *))
> {
> @@ -5780,7 +5787,8 @@ qemuDomainRestoreInternal(virConnectPtr conn,
> virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
> VIR_DOMAIN_SAVE_RUNNING |
> VIR_DOMAIN_SAVE_PAUSED |
> - VIR_DOMAIN_SAVE_RESET_NVRAM, -1);
> + VIR_DOMAIN_SAVE_RESET_NVRAM |
> + VIR_DOMAIN_SAVE_PARALLEL, -1);
>
> if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM)
> reset_nvram = true;
> @@ -5789,7 +5797,7 @@ qemuDomainRestoreInternal(virConnectPtr conn,
> goto cleanup;
>
> mapped_ram = data->header.features & QEMU_SAVE_FEATURE_MAPPED_RAM;
> - if (!(restoreParams = qemuMigrationParamsForSave(mapped_ram, flags)))
> + if (!(restoreParams = qemuMigrationParamsForSave(params, nparams, mapped_ram, flags)))
> return -1;
>
> fd = qemuSaveImageOpen(driver, path,
> @@ -5871,7 +5879,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
> const char *dxml,
> unsigned int flags)
> {
> - return qemuDomainRestoreInternal(conn, path, dxml, flags,
> + return qemuDomainRestoreInternal(conn, path, dxml, NULL, 0, flags,
> virDomainRestoreFlagsEnsureACL);
> }
>
> @@ -5879,7 +5887,7 @@ static int
> qemuDomainRestore(virConnectPtr conn,
> const char *path)
> {
> - return qemuDomainRestoreInternal(conn, path, NULL, 0,
> + return qemuDomainRestoreInternal(conn, path, NULL, NULL, 0, 0,
> virDomainRestoreEnsureACL);
> }
>
> @@ -5895,6 +5903,7 @@ qemuDomainRestoreParams(virConnectPtr conn,
> if (virTypedParamsValidate(params, nparams,
> VIR_DOMAIN_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING,
> VIR_DOMAIN_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING,
> + VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT,
> NULL) < 0)
> return -1;
>
> @@ -5911,7 +5920,7 @@ qemuDomainRestoreParams(virConnectPtr conn,
> return -1;
> }
>
> - ret = qemuDomainRestoreInternal(conn, path, dxml, flags,
> + ret = qemuDomainRestoreInternal(conn, path, dxml, params, nparams, flags,
> virDomainRestoreParamsEnsureACL);
> return ret;
> }
> @@ -6115,7 +6124,7 @@ qemuDomainObjRestore(virConnectPtr conn,
> }
>
> mapped_ram = data->header.features & QEMU_SAVE_FEATURE_MAPPED_RAM;
> - if (!(restoreParams = qemuMigrationParamsForSave(mapped_ram,
> + if (!(restoreParams = qemuMigrationParamsForSave(NULL, 0, mapped_ram,
> bypass_cache ? VIR_DOMAIN_SAVE_BYPASS_CACHE : 0)))
> return -1;
>
> diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
> index 8f6003005c..8c656af163 100644
> --- a/src/qemu/qemu_migration_params.c
> +++ b/src/qemu/qemu_migration_params.c
> @@ -788,7 +788,10 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
>
>
> qemuMigrationParams *
> -qemuMigrationParamsForSave(bool mappedRam, unsigned int flags)
> +qemuMigrationParamsForSave(virTypedParameterPtr params,
> + int nparams,
> + bool mappedRam,
> + unsigned int flags)
> {
> g_autoptr(qemuMigrationParams) saveParams = NULL;
>
> @@ -800,7 +803,25 @@ qemuMigrationParamsForSave(bool mappedRam, unsigned int flags)
> return NULL;
> if (virBitmapSetBit(saveParams->caps, QEMU_MIGRATION_CAP_MULTIFD) < 0)
> return NULL;
> - saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = 1;
> +
> + if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
> + int nchannels;
> +
> + if (params && virTypedParamsGetInt(params, nparams,
> + VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS,
> + &nchannels) < 0)
> + return NULL;
> +
> + if (nchannels < 1) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("number of parallel save channels cannot be less than 1"));
> + return NULL;
> + }
> +
> + saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = nchannels;
> + } else {
> + saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = 1;
> + }
> saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].set = true;
>
> if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) {
> diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h
> index 9700469b5e..a672b3d875 100644
> --- a/src/qemu/qemu_migration_params.h
> +++ b/src/qemu/qemu_migration_params.h
> @@ -88,7 +88,10 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
> qemuMigrationParty party);
>
> qemuMigrationParams *
> -qemuMigrationParamsForSave(bool mappedRam, unsigned int flags);
> +qemuMigrationParamsForSave(virTypedParameterPtr params,
> + int nparams,
> + bool mappedRam,
> + unsigned int flags);
>
> int
> qemuMigrationParamsDump(qemuMigrationParams *migParams,
> --
> 2.35.3
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 10/10/24 07:43, Daniel P. Berrangé wrote:
> On Thu, Aug 08, 2024 at 05:38:11PM -0600, Jim Fehlig via Devel wrote:
>> Add support for parallel save and restore by mapping libvirt's
>> "parallel-connections" parameter to QEMU's "multifd-channels"
>> migration parameter.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>> src/qemu/qemu_driver.c | 31 ++++++++++++++++++++-----------
>> src/qemu/qemu_migration_params.c | 25 +++++++++++++++++++++++--
>> src/qemu/qemu_migration_params.h | 5 ++++-
>> 3 files changed, 47 insertions(+), 14 deletions(-)
>
> Unless I'm missing something, nothing in this patch is validating
> the the image is about to be written in v3 format with MAPPED_RAM
> set. We must reject VIR_DOMAIN_SAVE_PARALLEL if we're not using
> MAPPED_RAM. Likewise when restoring we must reject PARALLEL if
> the loaded image doesnt have MAPPED_RAM feature set.
Yeah, good catch. The existing logic essentially ignores PARALLEL if mapped-ram
is not supported.
[snip]
>> diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
>> index 8f6003005c..8c656af163 100644
>> --- a/src/qemu/qemu_migration_params.c
>> +++ b/src/qemu/qemu_migration_params.c
>> @@ -788,7 +788,10 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
>>
>>
>> qemuMigrationParams *
>> -qemuMigrationParamsForSave(bool mappedRam, unsigned int flags)
>> +qemuMigrationParamsForSave(virTypedParameterPtr params,
>> + int nparams,
>> + bool mappedRam,
>> + unsigned int flags)
>> {
>> g_autoptr(qemuMigrationParams) saveParams = NULL;
>>
>> @@ -800,7 +803,25 @@ qemuMigrationParamsForSave(bool mappedRam, unsigned int flags)
>> return NULL;
>> if (virBitmapSetBit(saveParams->caps, QEMU_MIGRATION_CAP_MULTIFD) < 0)
>> return NULL;
>> - saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = 1;
>> +
>> + if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
>> + int nchannels;
>> +
>> + if (params && virTypedParamsGetInt(params, nparams,
>> + VIR_DOMAIN_SAVE_PARAM_PARALLEL_CONNECTIONS,
>> + &nchannels) < 0)
>> + return NULL;
>> +
>> + if (nchannels < 1) {
>> + virReportError(VIR_ERR_INVALID_ARG, "%s",
>> + _("number of parallel save channels cannot be less than 1"));
>> + return NULL;
>> + }
>> +
>> + saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = nchannels;
>> + } else {
>> + saveParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].value.i = 1;
>> + }
It's not shown in the diff, but this block is nestled within a 'if (mappedRam)'
block. I've squashed the blow hunk into this patch in my local branch.
Regards,
Jim
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 8c656af163..6fa09c272e 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -795,6 +795,12 @@ qemuMigrationParamsForSave(virTypedParameterPtr params,
{
g_autoptr(qemuMigrationParams) saveParams = NULL;
+ if (flags & VIR_DOMAIN_SAVE_PARALLEL && !mapped_ram) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Parallel save is not available with this QEMU binary"));
+ return NULL;
+ }
+
if (!(saveParams = qemuMigrationParamsNew()))
return NULL;
© 2016 - 2024 Red Hat, Inc.