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@xxxxxxxx>
---
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;
