Re: [RFC PATCH 04/13] migration: Fix parameter validation

[Peter Xu <peterx@redhat.com>]

On Fri, Apr 11, 2025 at 04:14:34PM -0300, Fabiano Rosas wrote:
> The migration parameters validation involves producing a temporary
> structure which merges the current parameter values with the new
> parameters set by the user.
> 
> The has_ boolean fields of MigrateSetParameter are taken into
> consideration when writing the temporary structure, however the copy
> of the current parameters also copies all the has_ fields of
> s->parameters and those are (almost) all true due to being initialized
> by migrate_params_init().
> 
> Since the temporary structure copy does not carry over the has_ fields
> from MigrateSetParameters, only the values which were initialized in
> migrate_params_init() will end up being validated. This causes
> (almost) all of the migration parameters to be validated again every
> time a parameter is set, which could be considered a bug. But it also
> skips validation of those values which are not set in
> migrate_params_init(), which is a worse issue.

IMHO it's ok to double check all parameters in slow path.  Definitely not
ok to skip them.. So now the question is, if migrate_params_test_apply() so
far should check all params anyway...

Shall we drop the checking for all has_ there, then IIUC we also don't need
any initializations for has_* in migrate_params_init() here?

So, admittedly s->parameters.has_* is still ugly to be present.. we declare
all of them not used and ignore them always at least in s->parameters.

> 
> Fix by initializing the missing values in migrate_params_init().
> Currently 'avail_switchover_bandwidth' and 'block_bitmap_mapping' are
> affected.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/options.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/migration/options.c b/migration/options.c
> index cac28540dd..625d597a85 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -987,6 +987,8 @@ void migrate_params_init(MigrationParameters *params)
>      params->has_mode = true;
>      params->has_zero_page_detection = true;
>      params->has_direct_io = true;
> +    params->has_avail_switchover_bandwidth = true;
> +    params->has_block_bitmap_mapping = true;
>  }
>  
>  /*
> -- 
> 2.35.3
> 

-- 
Peter Xu