Re: [Qemu-devel] [PATCH v3 12/13] migration: move skip_configuration out

[Peter Xu <peterx@redhat.com>]

On Mon, Jun 19, 2017 at 05:56:29PM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > It was in SaveState but now moved to MigrationState altogether, reverted
> > its meaning, then renamed to "send_configuration". Again, using
> > HW_COMPAT_2_3 for old PC/SPAPR machines, and accel_register_prop() for
> > xen_init().
> >
> > Removing savevm_skip_configuration().
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>

Thanks!

> 
> 
> > -    if (!savevm_state.skip_configuration || enforce_config_section()) {
> > +    if (migrate_get_current()->send_configuration ||
> > +        enforce_config_section()) {
> >          qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> >          vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> >      }
> 
> It is a different problem, but this patch makes enforce_config_section
> optional, no?  We can get the same behaviour with this new option,
> right?
> 
> Looking at the code, it is not clear to me if it is easier to put
> enforce_config_option on top of this patch, or let things as they are.
> 
> Opinions?

There can be a very tiny difference. Consider this cmdline:

  qemu -M enforce-config-section=true -global migration.send-configuration=false

For current patch, enforce-config-section=true will make sure this
cmdline will send the config section no matter what we provided in
-global section.

If we remove the enforce_config_section() above and also use
MigrationState.send_configuration, the enforce-config-section=true
will be replaced by the latter send-configuration=false, then config
section will not be sent.

However, I'll vote for your suggestion since after all this cmdline is
not really making much sense (considering it's providing two conflict
confugrations). And anyone who starts to use "-global migration.*"
then he/she should know the possible side effect.

-- 
Peter Xu