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

[Fabiano Rosas <farosas@suse.de>]

Fabiano Rosas <farosas@suse.de> writes:

> Peter Xu <peterx@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.