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;
+
+ 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) {
+ 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;
+ }
return g_steal_pointer(&migParams);
}
--
2.33.0On 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
On 2023/2/7 23:37, Jiri Denemark wrote:
> 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.
Thanks for your suggestion. I will try to merge this function into
'qemuMigrationParamsSetCompression ' in my next version.
I realize that zlib/zstd are compression method same with the others. I
should process them together and using flags to choose, not using flags
to choose the way I process them.
>
>> + 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.
Thanks for your suggestion, I wiil improve the logic here in my next
version.
Thanks, Jiang Jiacheng
>
>> + 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
>
© 2016 - 2026 Red Hat, Inc.