On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.w...@bytedance.com> wrote:
> Adds support for migration parameters to control QATzip compression
> level and to enable/disable software fallback when QAT hardware is
> unavailable. This is a preparatory commit for a subsequent commit that
> will actually use QATzip compression.
* Is the check whether "QAT hardware is available" happening in this
patch? (I couldn't spot it).
* The informatory notice "This is a preparatory commit for..." could
be moved to the cover-letter, instead of commit message. (Not sure how
it helps to log it in git history)
> Acked-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Bryan Zhang <bryan.zh...@bytedance.com>
> Signed-off-by: Hao Xiang <hao.xi...@linux.dev>
> Signed-off-by: Yichen Wang <yichen.w...@bytedance.com>
> ---
> migration/migration-hmp-cmds.c | 4 ++++
> migration/options.c | 34 ++++++++++++++++++++++++++++++++++
> migration/options.h | 1 +
> qapi/migration.json | 18 ++++++++++++++++++
> 4 files changed, 57 insertions(+)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 7d608d26e1..28165cfc9e 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -576,6 +576,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict
> *qdict)
> p->has_multifd_zlib_level = true;
> visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
> break;
> + case MIGRATION_PARAMETER_MULTIFD_QATZIP_LEVEL:
> + p->has_multifd_qatzip_level = true;
> + visit_type_uint8(v, param, &p->multifd_qatzip_level, &err);
> + break;
> case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
> p->has_multifd_zstd_level = true;
> visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
> diff --git a/migration/options.c b/migration/options.c
> index 645f55003d..147cd2b8fd 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -55,6 +55,13 @@
> #define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
> /* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
> #define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
> +/*
> + * 1: best speed, ... 9: best compress ratio
> + * There is some nuance here. Refer to QATzip documentation to understand
> + * the mapping of QATzip levels to standard deflate levels.
> + */
> +#define DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL 1
> +
> /* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
> #define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
>
> @@ -123,6 +130,9 @@ Property migration_properties[] = {
> DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
> parameters.multifd_zlib_level,
> DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
> + DEFINE_PROP_UINT8("multifd-qatzip-level", MigrationState,
> + parameters.multifd_qatzip_level,
> + DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL),
> DEFINE_PROP_UINT8("multifd-zstd-level", MigrationState,
> parameters.multifd_zstd_level,
> DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL),
> @@ -787,6 +797,13 @@ int migrate_multifd_zlib_level(void)
> return s->parameters.multifd_zlib_level;
> }
>
> +int migrate_multifd_qatzip_level(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return s->parameters.multifd_qatzip_level;
> +}
> +
> int migrate_multifd_zstd_level(void)
> {
> MigrationState *s = migrate_get_current();
> @@ -892,6 +909,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
> **errp)
> params->multifd_compression = s->parameters.multifd_compression;
> params->has_multifd_zlib_level = true;
> params->multifd_zlib_level = s->parameters.multifd_zlib_level;
> + params->has_multifd_qatzip_level = true;
> + params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
> params->has_multifd_zstd_level = true;
> params->multifd_zstd_level = s->parameters.multifd_zstd_level;
> params->has_xbzrle_cache_size = true;
> @@ -946,6 +965,7 @@ void migrate_params_init(MigrationParameters *params)
> params->has_multifd_channels = true;
> params->has_multifd_compression = true;
> params->has_multifd_zlib_level = true;
> + params->has_multifd_qatzip_level = true;
> params->has_multifd_zstd_level = true;
> params->has_xbzrle_cache_size = true;
> params->has_max_postcopy_bandwidth = true;
> @@ -1038,6 +1058,14 @@ bool migrate_params_check(MigrationParameters *params,
> Error **errp)
> return false;
> }
>
> + if (params->has_multifd_qatzip_level &&
> + ((params->multifd_qatzip_level > 9) ||
> + (params->multifd_qatzip_level < 1))) {
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> "multifd_qatzip_level",
> + "a value between 1 and 9");
> + return false;
> + }
> +
> if (params->has_multifd_zstd_level &&
> (params->multifd_zstd_level > 20)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
> @@ -1195,6 +1223,9 @@ static void
> migrate_params_test_apply(MigrateSetParameters *params,
> if (params->has_multifd_compression) {
> dest->multifd_compression = params->multifd_compression;
> }
> + if (params->has_multifd_qatzip_level) {
> + dest->multifd_qatzip_level = params->multifd_qatzip_level;
> + }
> if (params->has_multifd_zlib_level) {
> dest->multifd_zlib_level = params->multifd_zlib_level;
> }
> @@ -1315,6 +1346,9 @@ static void migrate_params_apply(MigrateSetParameters
> *params, Error **errp)
> if (params->has_multifd_compression) {
> s->parameters.multifd_compression = params->multifd_compression;
> }
> + if (params->has_multifd_qatzip_level) {
> + s->parameters.multifd_qatzip_level = params->multifd_qatzip_level;
> + }
> if (params->has_multifd_zlib_level) {
> s->parameters.multifd_zlib_level = params->multifd_zlib_level;
> }
> diff --git a/migration/options.h b/migration/options.h
> index a2397026db..a0bd6edc06 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -78,6 +78,7 @@ uint64_t migrate_max_postcopy_bandwidth(void);
> int migrate_multifd_channels(void);
> MultiFDCompression migrate_multifd_compression(void);
> int migrate_multifd_zlib_level(void);
> +int migrate_multifd_qatzip_level(void);
> int migrate_multifd_zstd_level(void);
> uint8_t migrate_throttle_trigger_threshold(void);
> const char *migrate_tls_authz(void);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7324571e92..f4c27426c8 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -792,6 +792,11 @@
> # speed, and 9 means best compression ratio which will consume
> # more CPU. Defaults to 1. (Since 5.0)
> #
> +# @multifd-qatzip-level: Set the compression level to be used in live
> +# migration. The level is an integer between 1 and 9, where 1 means
> +# the best compression speed, and 9 means the best compression
> +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> +#
> # @multifd-zstd-level: Set the compression level to be used in live
> # migration, the compression level is an integer between 0 and 20,
> # where 0 means no compression, 1 means the best compression
> @@ -852,6 +857,7 @@
> 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> 'max-cpu-throttle', 'multifd-compression',
> 'multifd-zlib-level', 'multifd-zstd-level',
> + 'multifd-qatzip-level',
> 'block-bitmap-mapping',
> { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
> 'vcpu-dirty-limit',
> @@ -967,6 +973,11 @@
> # speed, and 9 means best compression ratio which will consume
> # more CPU. Defaults to 1. (Since 5.0)
> #
> +# @multifd-qatzip-level: Set the compression level to be used in live
> +# migration. The level is an integer between 1 and 9, where 1 means
> +# the best compression speed, and 9 means the best compression
> +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> +#
> # @multifd-zstd-level: Set the compression level to be used in live
> # migration, the compression level is an integer between 0 and 20,
> # where 0 means no compression, 1 means the best compression
> @@ -1040,6 +1051,7 @@
> '*max-cpu-throttle': 'uint8',
> '*multifd-compression': 'MultiFDCompression',
> '*multifd-zlib-level': 'uint8',
> + '*multifd-qatzip-level': 'uint8',
> '*multifd-zstd-level': 'uint8',
> '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
> '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
> @@ -1171,6 +1183,11 @@
> # speed, and 9 means best compression ratio which will consume
> # more CPU. Defaults to 1. (Since 5.0)
> #
> +# @multifd-qatzip-level: Set the compression level to be used in live
> +# migration. The level is an integer between 1 and 9, where 1 means
> +# the best compression speed, and 9 means the best compression
> +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> +#
> # @multifd-zstd-level: Set the compression level to be used in live
> # migration, the compression level is an integer between 0 and 20,
> # where 0 means no compression, 1 means the best compression
> @@ -1241,6 +1258,7 @@
> '*max-cpu-throttle': 'uint8',
> '*multifd-compression': 'MultiFDCompression',
> '*multifd-zlib-level': 'uint8',
> + '*multifd-qatzip-level': 'uint8',
> '*multifd-zstd-level': 'uint8',
> '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
> '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
> --
> Yichen Wang
'multifd-qatzip-level' related changes look okay.
Reviewed-by: Prasad Pandit <p...@fedoraproject.org>
Thank you.
---
- Prasad
