Fabiano Rosas <faro...@suse.de> writes: > Peter Xu <pet...@redhat.com> writes: > >> 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...
Actually, this doesn't work... The migrate-set-* commands have optional fields, so we need some form of checking has_* to know which fields the user is setting. Otherwise MigrationSetParameters will have zeros all over that will trip the check. Then, we need some form of checking has_* to be able to enventually get the values into s->config (former s->parameters/capabilities), otherwise we'll overwrite the already-set values with the potentially empty ones coming from QAPI. Then there's also the issue of knowing whether a value is 0 because the user set it 0 or because it was never set. We also can't apply an invalid value to s->config and validate it after because some parameters are allowed to be changed during migration.
