Re: [Qemu-devel] [PATCH 2/3] migration: Remove use of old MigrationParams

[Eric Blake]

On 05/12/2017 05:55 AM, Juan Quintela wrote:
>>> @@ -1239,6 +1240,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
>>> blk,
>>>      }
>>>  
>>>      if (has_inc && inc) {
>>> +        migrate_set_block_enabled(s, true);
>>>          migrate_set_block_shared(s, true);
>>
>> [2]
>>
>> IIUC for [1] & [2] we are solving the same problem that "shared"
>> depends on "enabled" bit. Would it be good to unitfy this dependency
>> somewhere? E.g., by changing migrate_set_block_shared() into:
>>
>> void migrate_set_block_shared(MigrationState *s, bool value)
>> {
>>     s->enabled_capabilities[MIGRATION_CAPABILITY_BLOCK_SHARED] = value;
>>     if (value) {
>>         migrate_set_block_enabled(s, true);
>>     }
>> }
> 
> ok with this.

Or, as I commented on 1/3, maybe having a single property that is a
tri-state enum value, instead of 2 separate boolean properties, might be
nicer (but certainly a bit more complex to code up).

> I will add once here that when we disable block enabled, we also disable
> shared, or just let it that way?
> 
>> Another thing to mention: after switching to the capability interface,
>> we'll cache the "enabled" and "shared" bits now while we don't cache
>> it before, right? IIUC it'll affect behavior of such sequence:
>>
>> - 1st migrate with enabled=1, shared=1, then
>> - 2nd migrate with enabled=0, shared=0
>>
>> Before the series, the 2nd migrate will use enabled=shared=0, but
>> after the series it should be using enabled=shared=1. Not sure whether
>> this would be a problem (or I missed anything?).
> 
> We can't be consistent with both old/new way.
> 
> Old way: we always setup the capabilities on command line (that should
> have been deprecated long, long ago)

Well, the easy way out is to have the HMP migrate command (I assume
that's what you mean by "on command line") explicitly clear the
parameters if it is called without the -b/-i flag.  So the start of each
migration is what changes the properties, so long as you are still using
HMP to start the migration.  Or, on the QMP side, since 'migrate' has
optional 'blk' and 'inc' booleans, basically leave the settings alone if
the parameters were omitted, and explicitly update the property to the
value of those parameters if they were present.

Or is the proposal that we are also going to simplify the QMP 'migrate'
command to get rid of crufty parameters?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature