1. Patchew
  2. QEMU
  3. View series

[PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline

Bin Guo posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250929080115.98072-1-guobin@linux.alibaba.com
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
migration/options.c | 70 ++-------------------------------------------
1 file changed, 3 insertions(+), 67 deletions(-)
[PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline
Posted by Bin Guo 1 month, 2 weeks ago 
It's better to use QAPI_CLONE() in qmp_query_migrate_parameters so that
the code is cleaner.

No functional changes intended.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 migration/options.c | 70 ++-------------------------------------------
 1 file changed, 3 insertions(+), 67 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 4e923a2e07..347d762b03 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -892,81 +892,16 @@ AnnounceParameters *migrate_announce_params(void)
 
 MigrationParameters *qmp_query_migrate_parameters(Error **errp)
 {
-    MigrationParameters *params;
     MigrationState *s = migrate_get_current();
 
-    /* TODO use QAPI_CLONE() instead of duplicating it inline */
-    params = g_malloc0(sizeof(*params));
-    params->has_throttle_trigger_threshold = true;
-    params->throttle_trigger_threshold = s->parameters.throttle_trigger_threshold;
-    params->has_cpu_throttle_initial = true;
-    params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
-    params->has_cpu_throttle_increment = true;
-    params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
-    params->has_cpu_throttle_tailslow = true;
-    params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
-    params->tls_creds = g_strdup(s->parameters.tls_creds);
-    params->tls_hostname = g_strdup(s->parameters.tls_hostname);
-    params->tls_authz = g_strdup(s->parameters.tls_authz ?
-                                 s->parameters.tls_authz : "");
-    params->has_max_bandwidth = true;
-    params->max_bandwidth = s->parameters.max_bandwidth;
-    params->has_avail_switchover_bandwidth = true;
-    params->avail_switchover_bandwidth = s->parameters.avail_switchover_bandwidth;
-    params->has_downtime_limit = true;
-    params->downtime_limit = s->parameters.downtime_limit;
-    params->has_x_checkpoint_delay = true;
-    params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
-    params->has_multifd_channels = true;
-    params->multifd_channels = s->parameters.multifd_channels;
-    params->has_multifd_compression = true;
-    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;
-    params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
-    params->has_max_postcopy_bandwidth = true;
-    params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
-    params->has_max_cpu_throttle = true;
-    params->max_cpu_throttle = s->parameters.max_cpu_throttle;
-    params->has_announce_initial = true;
-    params->announce_initial = s->parameters.announce_initial;
-    params->has_announce_max = true;
-    params->announce_max = s->parameters.announce_max;
-    params->has_announce_rounds = true;
-    params->announce_rounds = s->parameters.announce_rounds;
-    params->has_announce_step = true;
-    params->announce_step = s->parameters.announce_step;
-
-    if (s->parameters.has_block_bitmap_mapping) {
-        params->has_block_bitmap_mapping = true;
-        params->block_bitmap_mapping =
-            QAPI_CLONE(BitmapMigrationNodeAliasList,
-                       s->parameters.block_bitmap_mapping);
-    }
-
-    params->has_x_vcpu_dirty_limit_period = true;
-    params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
-    params->has_vcpu_dirty_limit = true;
-    params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
-    params->has_mode = true;
-    params->mode = s->parameters.mode;
-    params->has_zero_page_detection = true;
-    params->zero_page_detection = s->parameters.zero_page_detection;
-    params->has_direct_io = true;
-    params->direct_io = s->parameters.direct_io;
-
-    return params;
+    return QAPI_CLONE(MigrationParameters, &s->parameters);
 }
 
 void migrate_params_init(MigrationParameters *params)
 {
     params->tls_hostname = g_strdup("");
     params->tls_creds = g_strdup("");
+    params->tls_authz = g_strdup("");
 
     /* Set has_* up only for parameter checks */
     params->has_throttle_trigger_threshold = true;
@@ -974,6 +909,7 @@ void migrate_params_init(MigrationParameters *params)
     params->has_cpu_throttle_increment = true;
     params->has_cpu_throttle_tailslow = true;
     params->has_max_bandwidth = true;
+    params->has_avail_switchover_bandwidth = true;
     params->has_downtime_limit = true;
     params->has_x_checkpoint_delay = true;
     params->has_multifd_channels = true;
-- 
2.39.5
Re: [PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline
Posted by Fabiano Rosas 1 month, 2 weeks ago 
Bin Guo <guobin@linux.alibaba.com> writes:

> It's better to use QAPI_CLONE() in qmp_query_migrate_parameters so that
> the code is cleaner.
>

Thanks, but this is a can of worms that needs to be opened very
slowly. Could you help test/review my series instead?

https://lore.kernel.org/r/20250630195913.28033-1-farosas@suse.de

> No functional changes intended.
>
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  migration/options.c | 70 ++-------------------------------------------
>  1 file changed, 3 insertions(+), 67 deletions(-)
>
> diff --git a/migration/options.c b/migration/options.c
> index 4e923a2e07..347d762b03 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -892,81 +892,16 @@ AnnounceParameters *migrate_announce_params(void)
>  
>  MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>  {
> -    MigrationParameters *params;
>      MigrationState *s = migrate_get_current();
>  
> -    /* TODO use QAPI_CLONE() instead of duplicating it inline */
> -    params = g_malloc0(sizeof(*params));
> -    params->has_throttle_trigger_threshold = true;
> -    params->throttle_trigger_threshold = s->parameters.throttle_trigger_threshold;
> -    params->has_cpu_throttle_initial = true;
> -    params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
> -    params->has_cpu_throttle_increment = true;
> -    params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
> -    params->has_cpu_throttle_tailslow = true;
> -    params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
> -    params->tls_creds = g_strdup(s->parameters.tls_creds);
> -    params->tls_hostname = g_strdup(s->parameters.tls_hostname);
> -    params->tls_authz = g_strdup(s->parameters.tls_authz ?
> -                                 s->parameters.tls_authz : "");
> -    params->has_max_bandwidth = true;
> -    params->max_bandwidth = s->parameters.max_bandwidth;
> -    params->has_avail_switchover_bandwidth = true;
> -    params->avail_switchover_bandwidth = s->parameters.avail_switchover_bandwidth;
> -    params->has_downtime_limit = true;
> -    params->downtime_limit = s->parameters.downtime_limit;
> -    params->has_x_checkpoint_delay = true;
> -    params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
> -    params->has_multifd_channels = true;
> -    params->multifd_channels = s->parameters.multifd_channels;
> -    params->has_multifd_compression = true;
> -    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;
> -    params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
> -    params->has_max_postcopy_bandwidth = true;
> -    params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
> -    params->has_max_cpu_throttle = true;
> -    params->max_cpu_throttle = s->parameters.max_cpu_throttle;
> -    params->has_announce_initial = true;
> -    params->announce_initial = s->parameters.announce_initial;
> -    params->has_announce_max = true;
> -    params->announce_max = s->parameters.announce_max;
> -    params->has_announce_rounds = true;
> -    params->announce_rounds = s->parameters.announce_rounds;
> -    params->has_announce_step = true;
> -    params->announce_step = s->parameters.announce_step;
> -
> -    if (s->parameters.has_block_bitmap_mapping) {
> -        params->has_block_bitmap_mapping = true;
> -        params->block_bitmap_mapping =
> -            QAPI_CLONE(BitmapMigrationNodeAliasList,
> -                       s->parameters.block_bitmap_mapping);
> -    }
> -
> -    params->has_x_vcpu_dirty_limit_period = true;
> -    params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
> -    params->has_vcpu_dirty_limit = true;
> -    params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
> -    params->has_mode = true;
> -    params->mode = s->parameters.mode;
> -    params->has_zero_page_detection = true;
> -    params->zero_page_detection = s->parameters.zero_page_detection;
> -    params->has_direct_io = true;
> -    params->direct_io = s->parameters.direct_io;
> -
> -    return params;
> +    return QAPI_CLONE(MigrationParameters, &s->parameters);
>  }
>  
>  void migrate_params_init(MigrationParameters *params)
>  {
>      params->tls_hostname = g_strdup("");
>      params->tls_creds = g_strdup("");
> +    params->tls_authz = g_strdup("");
>  
>      /* Set has_* up only for parameter checks */
>      params->has_throttle_trigger_threshold = true;
> @@ -974,6 +909,7 @@ void migrate_params_init(MigrationParameters *params)
>      params->has_cpu_throttle_increment = true;
>      params->has_cpu_throttle_tailslow = true;
>      params->has_max_bandwidth = true;
> +    params->has_avail_switchover_bandwidth = true;
>      params->has_downtime_limit = true;
>      params->has_x_checkpoint_delay = true;
>      params->has_multifd_channels = true;
回复：[PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline
Posted by GuoBin 1 month ago 
ok, nice job, I will try to test it.
When will the v3 version be released?
------------------------------------------------------------------
发件人：Fabiano Rosas <farosas@suse.de>
发送时间：2025年9月29日(周一) 22:27
收件人：Bin Guo<guobin@linux.alibaba.com>; "qemu-devel"<qemu-devel@nongnu.org>
抄　送：peterx<peterx@redhat.com>
主　题：Re: [PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline
Bin Guo <guobin@linux.alibaba.com> writes:
> It's better to use QAPI_CLONE() in qmp_query_migrate_parameters so that
> the code is cleaner.
>
Thanks, but this is a can of worms that needs to be opened very
slowly. Could you help test/review my series instead?
https://lore.kernel.org/r/20250630195913.28033-1-farosas@suse.de <https://lore.kernel.org/r/20250630195913.28033-1-farosas@suse.de >
> No functional changes intended.
>
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
> migration/options.c | 70 ++-------------------------------------------
> 1 file changed, 3 insertions(+), 67 deletions(-)
>
> diff --git a/migration/options.c b/migration/options.c
> index 4e923a2e07..347d762b03 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -892,81 +892,16 @@ AnnounceParameters *migrate_announce_params(void)
> 
> MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> {
> - MigrationParameters *params;
> MigrationState *s = migrate_get_current();
> 
> - /* TODO use QAPI_CLONE() instead of duplicating it inline */
> - params = g_malloc0(sizeof(*params));
> - params->has_throttle_trigger_threshold = true;
> - params->throttle_trigger_threshold = s->parameters.throttle_trigger_threshold;
> - params->has_cpu_throttle_initial = true;
> - params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
> - params->has_cpu_throttle_increment = true;
> - params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
> - params->has_cpu_throttle_tailslow = true;
> - params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
> - params->tls_creds = g_strdup(s->parameters.tls_creds);
> - params->tls_hostname = g_strdup(s->parameters.tls_hostname);
> - params->tls_authz = g_strdup(s->parameters.tls_authz ?
> - s->parameters.tls_authz : "");
> - params->has_max_bandwidth = true;
> - params->max_bandwidth = s->parameters.max_bandwidth;
> - params->has_avail_switchover_bandwidth = true;
> - params->avail_switchover_bandwidth = s->parameters.avail_switchover_bandwidth;
> - params->has_downtime_limit = true;
> - params->downtime_limit = s->parameters.downtime_limit;
> - params->has_x_checkpoint_delay = true;
> - params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
> - params->has_multifd_channels = true;
> - params->multifd_channels = s->parameters.multifd_channels;
> - params->has_multifd_compression = true;
> - 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;
> - params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
> - params->has_max_postcopy_bandwidth = true;
> - params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
> - params->has_max_cpu_throttle = true;
> - params->max_cpu_throttle = s->parameters.max_cpu_throttle;
> - params->has_announce_initial = true;
> - params->announce_initial = s->parameters.announce_initial;
> - params->has_announce_max = true;
> - params->announce_max = s->parameters.announce_max;
> - params->has_announce_rounds = true;
> - params->announce_rounds = s->parameters.announce_rounds;
> - params->has_announce_step = true;
> - params->announce_step = s->parameters.announce_step;
> -
> - if (s->parameters.has_block_bitmap_mapping) {
> - params->has_block_bitmap_mapping = true;
> - params->block_bitmap_mapping =
> - QAPI_CLONE(BitmapMigrationNodeAliasList,
> - s->parameters.block_bitmap_mapping);
> - }
> -
> - params->has_x_vcpu_dirty_limit_period = true;
> - params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
> - params->has_vcpu_dirty_limit = true;
> - params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
> - params->has_mode = true;
> - params->mode = s->parameters.mode;
> - params->has_zero_page_detection = true;
> - params->zero_page_detection = s->parameters.zero_page_detection;
> - params->has_direct_io = true;
> - params->direct_io = s->parameters.direct_io;
> -
> - return params;
> + return QAPI_CLONE(MigrationParameters, &s->parameters);
> }
> 
> void migrate_params_init(MigrationParameters *params)
> {
> params->tls_hostname = g_strdup("");
> params->tls_creds = g_strdup("");
> + params->tls_authz = g_strdup("");
> 
> /* Set has_* up only for parameter checks */
> params->has_throttle_trigger_threshold = true;
> @@ -974,6 +909,7 @@ void migrate_params_init(MigrationParameters *params)
> params->has_cpu_throttle_increment = true;
> params->has_cpu_throttle_tailslow = true;
> params->has_max_bandwidth = true;
> + params->has_avail_switchover_bandwidth = true;
> params->has_downtime_limit = true;
> params->has_x_checkpoint_delay = true;
> params->has_multifd_channels = true;
Re: 回复：[PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline
Posted by Fabiano Rosas 1 month ago 
"GuoBin" <guobin@linux.alibaba.com> writes:

> ok, nice job, I will try to test it.
> When will the v3 version be released?

I hope to send it late this week. The series is ready in terms of code,
I just need to organize it a bit, make it palatable for review, etc.
Re: [PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline
Posted by Peter Xu 1 month, 2 weeks ago 
On Mon, Sep 29, 2025 at 04:01:15PM +0800, Bin Guo wrote:
> It's better to use QAPI_CLONE() in qmp_query_migrate_parameters so that
> the code is cleaner.
> 
> No functional changes intended.
> 
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>

Fabiano has this in an older patch:

https://lore.kernel.org/all/20250630195913.28033-12-farosas@suse.de/

Even though I don't remember why the subject said "QAPI_CLONE_MEMBERS"..

IIRC it has some dependency to convert tls entries first, likely patch 3 of
that series.  So the easiest is to wait him repost v3, then this will be
converted as part of the whole series.

Thanks for the patch.

-- 
Peter Xu

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.