diff mbox series

[v8,3/5] migration: Add migration parameters for QATzip

Message ID 20240820170907.6788-4-yichen.wang@bytedance.com
State New
Headers show
Series Implement QATzip compression method | expand

Commit Message

Yichen Wang Aug. 20, 2024, 5:09 p.m. UTC
From: Bryan Zhang <bryan.zhang@bytedance.com>

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.

Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
Signed-off-by: Yichen Wang <yichen.wang@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(+)

Comments

Prasad Pandit Aug. 21, 2024, 11:56 a.m. UTC | #1
On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.wang@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 <armbru@redhat.com>
> Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
> Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
> Signed-off-by: Yichen Wang <yichen.wang@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 <pjp@fedoraproject.org>

Thank you.
---
  - Prasad
Yichen Wang Aug. 21, 2024, 8:42 p.m. UTC | #2
On Wed, Aug 21, 2024 at 4:56 AM Prasad Pandit <ppandit@redhat.com> wrote:
>
> On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.wang@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).

After discussing with Intel folks, I decided to align to the existing
QPL behavior. In QPL, the code path of compression will always go
through regardless. When acceleration hardware is initialized
properly, use it. If failed, fallback to software path automatically.
So in this QAT case, I do the same. The line of "ret =
qzInit(&q->sess, true);" will do the auto software fallback.

> * 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)
>

Oh, this line is purely for breaking the big patch into two commits.
This one plus the following commit [4/5] together implements the full
feature. I can remove this from the commit message if you prefer.

> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
> > Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
> > Signed-off-by: Yichen Wang <yichen.wang@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 <pjp@fedoraproject.org>
>
> Thank you.
> ---
>   - Prasad
>
Prasad Pandit Aug. 22, 2024, 10:23 a.m. UTC | #3
Hi,

On Thu, 22 Aug 2024 at 02:13, Yichen Wang <yichen.wang@bytedance.com> wrote:
> After discussing with Intel folks, I decided to align to the existing
> QPL behavior. In QPL, the code path of compression will always go
> through regardless. When acceleration hardware is initialized
> properly, use it. If failed, fallback to software path automatically.
> So in this QAT case, I do the same. The line of "ret =
> qzInit(&q->sess, true);" will do the auto software fallback.

* I see.

> Oh, this line is purely for breaking the big patch into two commits.
> This one plus the following commit [4/5] together implements the full
> feature. I can remove this from the commit message if you prefer.

* Yes, that'll be nice.

Thank you.
---
  - Prasad
diff mbox series

Patch

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',