On Thu, May 22, 2025 at 02:39:48PM -0300, Fabiano Rosas wrote:
> 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.
What I meant was we only conditionally ignore the has_* fields in below:
(1) migrate_params_check(), so that QEMU always checks all parameters in
the MigrationParameters* specified when invoking the function.
(2) MigrationState.parameters, so that as long as the parameters are
applied (it should only happen after sanity check all pass..) then we
ignore these has_* fields (until MigrationState.parameters can have a
better struct to not include these has_* fields).
We can keep the has_* checks in migrate_params_test_apply() and
migrate_params_apply(), so that we won't touch the ones the user didn't
specify in the QMP commands as you said.
The benefits of having above 1/2 ignoring has_* is some code removal where
we assume has_* always are TRUEs.
This can be still a bit confusing, but at least we don't need to init has_*
fields in migrate_params_init() anymore as they'll be all ignored, then
there's no chance we forget set TRUEs to any new params either.
--
Peter Xu