diff mbox series

[1/6] migration/multifd: Add new migration option multifd-zero-page.

Message ID 20240206231908.1792529-2-hao.xiang@bytedance.com
State New
Headers show
Series Introduce multifd zero page checking. | expand

Commit Message

Hao Xiang Feb. 6, 2024, 11:19 p.m. UTC 
This new parameter controls where the zero page checking is running. If
this parameter is set to true, zero page checking is done in the multifd
sender threads. If this parameter is set to false, zero page checking is
done in the migration main thread.

Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
---
 migration/migration-hmp-cmds.c |  7 +++++++
 migration/options.c            | 20 ++++++++++++++++++++
 migration/options.h            |  1 +
 qapi/migration.json            | 24 +++++++++++++++++++++---
 4 files changed, 49 insertions(+), 3 deletions(-)

Comments

Peter Xu Feb. 7, 2024, 3:44 a.m. UTC | #1 
On Tue, Feb 06, 2024 at 11:19:03PM +0000, Hao Xiang wrote:
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 819708321d..ff033a0344 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -874,6 +874,11 @@
>  # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
>  #        (Since 8.2)
>  #
> +# @multifd-zero-page: Multifd zero page checking. If the parameter is true,
> +#     zero page checking is done on the multifd sender thread. If the parameter
> +#     is false, zero page checking is done on the migration main thread. Default
> +#     is set to true. (Since 9.0)

I replied somewhere before on this, but I can try again..

Do you think it'll be better to introduce a generic parameter for zero page
detection?

  - "none" if disabled,
  - "legacy" for main thread,
  - "multifd" for multifd (software-based).

A string could work, but maybe cleaner to introduce
@MigrationZeroPageDetector enum?

When you add more, you can keep extending that with the single field
("multifd-dsa", etc.).
Hao Xiang Feb. 8, 2024, 12:49 a.m. UTC | #2 
On Tue, Feb 6, 2024 at 7:45 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Tue, Feb 06, 2024 at 11:19:03PM +0000, Hao Xiang wrote:
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 819708321d..ff033a0344 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -874,6 +874,11 @@
> >  # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
> >  #        (Since 8.2)
> >  #
> > +# @multifd-zero-page: Multifd zero page checking. If the parameter is true,
> > +#     zero page checking is done on the multifd sender thread. If the parameter
> > +#     is false, zero page checking is done on the migration main thread. Default
> > +#     is set to true. (Since 9.0)
>
> I replied somewhere before on this, but I can try again..
>
> Do you think it'll be better to introduce a generic parameter for zero page
> detection?
>
>   - "none" if disabled,
>   - "legacy" for main thread,
>   - "multifd" for multifd (software-based).
>
> A string could work, but maybe cleaner to introduce
> @MigrationZeroPageDetector enum?
>
> When you add more, you can keep extending that with the single field
> ("multifd-dsa", etc.).
>
> --
> Peter Xu
>

Sorry I overlooked the previous email. This sounds like a good idea.
diff mbox series

Patch

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 99b49df5dd..8b0c205a41 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -344,6 +344,9 @@  void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "%s: %s\n",
             MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
             MultiFDCompression_str(params->multifd_compression));
+        monitor_printf(mon, "%s: %s\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_ZERO_PAGE),
+            params->multifd_zero_page ? "on" : "off");
         monitor_printf(mon, "%s: %" PRIu64 " bytes\n",
             MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
             params->xbzrle_cache_size);
@@ -634,6 +637,10 @@  void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_multifd_zstd_level = true;
         visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
         break;
+    case MIGRATION_PARAMETER_MULTIFD_ZERO_PAGE:
+        p->has_multifd_zero_page = true;
+        visit_type_bool(v, param, &p->multifd_zero_page, &err);
+        break;
     case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
         p->has_xbzrle_cache_size = true;
         if (!visit_type_size(v, param, &cache_size, &err)) {
diff --git a/migration/options.c b/migration/options.c
index 3e3e0b93b4..cb18a41267 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -179,6 +179,8 @@  Property migration_properties[] = {
     DEFINE_PROP_MIG_MODE("mode", MigrationState,
                       parameters.mode,
                       MIG_MODE_NORMAL),
+    DEFINE_PROP_BOOL("multifd-zero-page", MigrationState,
+                     parameters.multifd_zero_page, true),
 
     /* Migration capabilities */
     DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
@@ -903,6 +905,13 @@  uint64_t migrate_xbzrle_cache_size(void)
     return s->parameters.xbzrle_cache_size;
 }
 
+bool migrate_multifd_zero_page(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return s->parameters.multifd_zero_page;
+}
+
 /* parameter setters */
 
 void migrate_set_block_incremental(bool value)
@@ -1013,6 +1022,8 @@  MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
     params->has_mode = true;
     params->mode = s->parameters.mode;
+    params->has_multifd_zero_page = true;
+    params->multifd_zero_page = s->parameters.multifd_zero_page;
 
     return params;
 }
@@ -1049,6 +1060,7 @@  void migrate_params_init(MigrationParameters *params)
     params->has_x_vcpu_dirty_limit_period = true;
     params->has_vcpu_dirty_limit = true;
     params->has_mode = true;
+    params->has_multifd_zero_page = true;
 }
 
 /*
@@ -1350,6 +1362,10 @@  static void migrate_params_test_apply(MigrateSetParameters *params,
     if (params->has_mode) {
         dest->mode = params->mode;
     }
+
+    if (params->has_multifd_zero_page) {
+        dest->multifd_zero_page = params->multifd_zero_page;
+    }
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1494,6 +1510,10 @@  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_mode) {
         s->parameters.mode = params->mode;
     }
+
+    if (params->has_multifd_zero_page) {
+        s->parameters.multifd_zero_page = params->multifd_zero_page;
+    }
 }
 
 void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
diff --git a/migration/options.h b/migration/options.h
index 246c160aee..c080a6ba18 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -93,6 +93,7 @@  const char *migrate_tls_authz(void);
 const char *migrate_tls_creds(void);
 const char *migrate_tls_hostname(void);
 uint64_t migrate_xbzrle_cache_size(void);
+bool migrate_multifd_zero_page(void);
 
 /* parameters setters */
 
diff --git a/qapi/migration.json b/qapi/migration.json
index 819708321d..ff033a0344 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -874,6 +874,11 @@ 
 # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
 #        (Since 8.2)
 #
+# @multifd-zero-page: Multifd zero page checking. If the parameter is true,
+#     zero page checking is done on the multifd sender thread. If the parameter
+#     is false, zero page checking is done on the migration main thread. Default
+#     is set to true. (Since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @block-incremental is deprecated.  Use
@@ -907,7 +912,8 @@ 
            'block-bitmap-mapping',
            { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
            'vcpu-dirty-limit',
-           'mode'] }
+           'mode',
+           'multifd-zero-page'] }
 
 ##
 # @MigrateSetParameters:
@@ -1062,6 +1068,11 @@ 
 # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
 #        (Since 8.2)
 #
+# @multifd-zero-page: Multifd zero page checking. If the parameter is true,
+#     zero page checking is done on the multifd sender thread. If the parameter
+#     is false, zero page checking is done on the migration main thread. Default
+#     is set to true. (Since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @block-incremental is deprecated.  Use
@@ -1115,7 +1126,8 @@ 
             '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
                                             'features': [ 'unstable' ] },
             '*vcpu-dirty-limit': 'uint64',
-            '*mode': 'MigMode'} }
+            '*mode': 'MigMode',
+            '*multifd-zero-page': 'bool'} }
 
 ##
 # @migrate-set-parameters:
@@ -1290,6 +1302,11 @@ 
 # @mode: Migration mode. See description in @MigMode. Default is 'normal'.
 #        (Since 8.2)
 #
+# @multifd-zero-page: Multifd zero page checking. If the parameter is true,
+#     zero page checking is done on the multifd sender thread. If the parameter
+#     is false, zero page checking is done on the migration main thread. Default
+#     is set to true. (Since 9.0)
+#
 # Features:
 #
 # @deprecated: Member @block-incremental is deprecated.  Use
@@ -1340,7 +1357,8 @@ 
             '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
                                             'features': [ 'unstable' ] },
             '*vcpu-dirty-limit': 'uint64',
-            '*mode': 'MigMode'} }
+            '*mode': 'MigMode',
+            '*multifd-zero-page': 'bool'} }
 
 ##
 # @query-migrate-parameters: