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


Reply via email to