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 <faro...@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
