Remove migrate_params_apply so the logic of setting migration
parameters is all in one spot.

Suggested-by: Prasad Pandit <ppandit@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/options.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 180ebed51c..733aae51a8 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1266,25 +1266,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
     return true;
 }
 
-static void migrate_params_apply(MigrationParameters *params)
-{
-    MigrationState *s = migrate_get_current();
-    MigrationParameters *cur = &s->parameters;
-
-    migrate_tls_opts_free(cur);
-    qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
-    qapi_free_strList(cur->cpr_exec_command);
-
-    /* mark all present, so they're all copied */
-    migrate_mark_all_params_present(params);
-    QAPI_CLONE_MEMBERS(MigrationParameters, cur, params);
-}
-
 void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
 {
     MigrationState *s = migrate_get_current();
-    g_autoptr(MigrationParameters) tmp = QAPI_CLONE(MigrationParameters,
-                                                    &s->parameters);
+    MigrationParameters *cur = &s->parameters;
+    g_autoptr(MigrationParameters) tmp = QAPI_CLONE(MigrationParameters, cur);
 
     /*
      * Convert QTYPE_QNULL and NULL to the empty string (""). Even
@@ -1300,8 +1286,17 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
 
     QAPI_MERGE(MigrationParameters, tmp, params);
 
-    if (migrate_params_check(tmp, errp)) {
-        migrate_params_apply(tmp);
-        migrate_post_update_params(params, errp);
+    if (!migrate_params_check(tmp, errp)) {
+        return;
     }
+
+    migrate_tls_opts_free(cur);
+    qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
+    qapi_free_strList(cur->cpr_exec_command);
+
+    /* mark all present, so they're all copied */
+    migrate_mark_all_params_present(tmp);
+    QAPI_CLONE_MEMBERS(MigrationParameters, cur, tmp);
+
+    migrate_post_update_params(params, errp);
 }
-- 
2.51.0

On Mon, Feb 02, 2026 at 07:40:59PM -0300, Fabiano Rosas wrote:
> Remove migrate_params_apply so the logic of setting migration
> parameters is all in one spot.
> 
> Suggested-by: Prasad Pandit <ppandit@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/options.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/migration/options.c b/migration/options.c
> index 180ebed51c..733aae51a8 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1266,25 +1266,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>      return true;
>  }
>  
> -static void migrate_params_apply(MigrationParameters *params)
> -{
> -    MigrationState *s = migrate_get_current();
> -    MigrationParameters *cur = &s->parameters;
> -
> -    migrate_tls_opts_free(cur);
> -    qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
> -    qapi_free_strList(cur->cpr_exec_command);
> -
> -    /* mark all present, so they're all copied */
> -    migrate_mark_all_params_present(params);
> -    QAPI_CLONE_MEMBERS(MigrationParameters, cur, params);
> -}

I actually like these smaller functions when their name can represent what
it does (so I read less but know what it is doing faster)..  But yeah if
both of you like to drop it, I'm OK.

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Peter Xu <peterx@redhat.com> writes:

> On Mon, Feb 02, 2026 at 07:40:59PM -0300, Fabiano Rosas wrote:
>> Remove migrate_params_apply so the logic of setting migration
>> parameters is all in one spot.
>> 
>> Suggested-by: Prasad Pandit <ppandit@redhat.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  migration/options.c | 33 ++++++++++++++-------------------
>>  1 file changed, 14 insertions(+), 19 deletions(-)
>> 
>> diff --git a/migration/options.c b/migration/options.c
>> index 180ebed51c..733aae51a8 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -1266,25 +1266,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>>      return true;
>>  }
>>  
>> -static void migrate_params_apply(MigrationParameters *params)
>> -{
>> -    MigrationState *s = migrate_get_current();
>> -    MigrationParameters *cur = &s->parameters;
>> -
>> -    migrate_tls_opts_free(cur);
>> -    qapi_free_BitmapMigrationNodeAliasList(cur->block_bitmap_mapping);
>> -    qapi_free_strList(cur->cpr_exec_command);
>> -
>> -    /* mark all present, so they're all copied */
>> -    migrate_mark_all_params_present(params);
>> -    QAPI_CLONE_MEMBERS(MigrationParameters, cur, params);
>> -}
>
> I actually like these smaller functions when their name can represent what
> it does (so I read less but know what it is doing faster)..  But yeah if
> both of you like to drop it, I'm OK.
>

Yeah, I tend to agree with you. It's not a big deal anyway.

I want to eventually add a QMP-specific file to migration/ and move all
of the QMP command handlers in there. Having this all in one function
will be (_very slightly_) simpler to move later on.

> Reviewed-by: Peter Xu <peterx@redhat.com>