On Fri, Jan 20, 2023 at 16:47:42 +0800, Jiang Jiacheng wrote:
Add 'qemuMigrationParamsSetParallelCompression' to support set parallel migration compression method. Depending on whether '--parallel' is set, we invoke different functions to select compression method from the same param 'VIR_MIGRATE_PARAM_COMPRESSION'.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/qemu/qemu_migration_params.c | 76 +++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 380fc7dccd..4980761712 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -71,6 +71,8 @@ struct _qemuMigrationParams { typedef enum { QEMU_MIGRATION_COMPRESS_XBZRLE = 0, QEMU_MIGRATION_COMPRESS_MT, + QEMU_MIGRATION_COMPRESS_ZLIB, + QEMU_MIGRATION_COMPRESS_ZSTD,
QEMU_MIGRATION_COMPRESS_LAST } qemuMigrationCompressMethod; @@ -79,6 +81,8 @@ VIR_ENUM_IMPL(qemuMigrationCompressMethod, QEMU_MIGRATION_COMPRESS_LAST, "xbzrle", "mt", + "zlib", + "zstd", );
VIR_ENUM_IMPL(qemuMigrationCapability, @@ -524,6 +528,60 @@ qemuMigrationParamsSetTPString(qemuMigrationParams *migParams, migParams->params[param].value.s); }
+static int +qemuMigrationParamsSetParallelCompression(virTypedParameterPtr params, + int nparams, + qemuMigrationParams *migParams) +{ + size_t i; + int method; + const char *value = NULL; + int rc; +
This function is very similar to qemuMigrationParamsSetCompression and you need to handle the same values in both functions. Merge them into a single one that would handle all compression methods and whether they are allowed or not depending on the API flags.
+ for (i = 0; i < nparams; i++) { + if (STRNEQ(params[i].field, VIR_MIGRATE_PARAM_COMPRESSION)) + continue; + if (migParams->params[QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION].set) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Only one compression method could be specified with " + "multifd compression")); + return -1; + } + + method = qemuMigrationCompressMethodTypeFromString(params[i].value.s); + if (method < 2) {
Even though method is declared as int, it is an enum in reality. We store it in int because the API for converting a string to the corresponding enum value can return -1. Thus you should always use enum items in comparisons rather than magic integer values. And exact checks are much better than < or > as they don't break when new method gets introduced. And I think it's better to let the for loop translate VIR_MIGRATE_PARAM_COMPRESSION into migParams->compMethods and do the checks afterwards.
+ virReportError(VIR_ERR_INVALID_ARG, + _("Unsupported compression method '%s' with multifd migration"), + params[i].value.s); + return -1; + } + + migParams->compMethods |= 1ULL << method; + + if ((rc = virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_COMPRESSION, &value)) < 0) + return -1; + + migParams->params[QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION].value.s = g_strdup(value); + migParams->params[QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION].set = !!rc; + } + + if (migParams->params[QEMU_MIGRATION_PARAM_MULTIFD_ZLIB_LEVEL].set && + !(migParams->compMethods & (1ULL << QEMU_MIGRATION_COMPRESS_ZLIB))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Turn zlib compression on to tune it")); + return -1; + } + + if (migParams->params[QEMU_MIGRATION_PARAM_MULTIFD_ZSTD_LEVEL].set && + !(migParams->compMethods & (1ULL << QEMU_MIGRATION_COMPRESS_ZSTD))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Turn zstd compression on to tune it")); + return -1; + } + + return 0; +}
static int @@ -548,6 +606,13 @@ qemuMigrationParamsSetCompression(virTypedParameterPtr params, return -1; }
+ if (method > 1) { + virReportError(VIR_ERR_INVALID_ARG, + _("Trun parallel migration on to use compression method '%s'"), + params[i].value.s); + return -1; + } + if (migParams->compMethods & (1ULL << method)) { virReportError(VIR_ERR_INVALID_ARG, _("Compression method '%s' is specified twice"), @@ -566,6 +631,8 @@ qemuMigrationParamsSetCompression(virTypedParameterPtr params, cap = QEMU_MIGRATION_CAP_COMPRESS; break;
+ case QEMU_MIGRATION_COMPRESS_ZLIB: + case QEMU_MIGRATION_COMPRESS_ZSTD: case QEMU_MIGRATION_COMPRESS_LAST: default: continue; @@ -691,8 +758,13 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params, return NULL; }
- if (qemuMigrationParamsSetCompression(params, nparams, flags, migParams) < 0) - return NULL; + if (flags & VIR_MIGRATE_PARALLEL) { + if (qemuMigrationParamsSetParallelCompression(params, nparams, migParams) < 0) + return NULL; + } else { + if (qemuMigrationParamsSetCompression(params, nparams, flags, migParams) < 0) + return NULL; + }
As I said just handle all compression methods and options in the existing qemuMigrationParamsSetCompression function.
return g_steal_pointer(&migParams); }
Jirka