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

[Juan Quintela]

Markus Armbruster <address@hidden> wrote:
> Juan Quintela <address@hidden> writes:

...

>> As qmp command is asynchronous, you can think that -d is *always* on in
>> QMP O:-)
>
> Yes.  The existence of "detach" in QMP is owed to limitations of early
> QMP infrastructure.  It's flagged as "invalid" and "should not be
> used" since 2010.
>
> Perhaps we should start a section on QMP in
> <http://wiki.qemu.org/Features/LegacyRemoval>.  But I'd like to first
> have a way to communicate "you're using a deprecated feature" warnings
> via QMP.

+1

>> Tristates will complicate it.  I still think that:
>>
>> - capability: block_migration
>> - parameter: block_shared
>>
>> Makes more sense, no?
>>
>> If block_migration is not enabled, we ignore the shared parameter.  We
>> already do that for other parameters.
>
> My impression as a superficial reader is that migration configuration is
> a historically grown mess.  Perhaps we shouldn't try to interpret too
> much intent into it :)
>
> If we redo migration as an instance of the "job" abstraction once we
> have it, then migration configuration & control should become more less
> messy.  Of course, the old messes will stay with us for a while in the
> form of backward compatibility messes.
>
> I'm not too particular on how we do the tri-state now, as long as it
> reasonably fits what we have, and is documented clearly.

>>> If the new interface isn't used, the old one still needs to work.  If it
>>> is used, the old one either has to do "the right thing", or fail
>>> cleanly.
>>>
>>> We approximate "new interface isn't used" by "block migration is off in
>>> global state".  When it is off, the migration command needs to honor its
>>> two flags for compatibility.  It must leave block migration off in
>>> global state.  Yes, this will complicate the implementation until we
>>> actually remove the deprecated flags.  Par for the backward compatility
>>> course.
>>>
>>> When block migration isn't off in global state, we can either
>>>
>>> * let the flags take precedence over the global state (one
>>>   interpretation of "do the right thing"), or
>>>
>>> * reject flags that conflict with global state (another interpretation),
>>>   or
>>>
>>> * reject *all* flags (fail cleanly).
>>>
>>> The last one looks perfectly servicable to me.
>>
>> Yeap,  I think that makes sense.  If you use capabilities, parameters,
>> old interface don't work at all.
>>
>> We still have a problem that is what happens if the user does:
>>
>> migrate -b <foo>
>> migrate_cancel (or error)
>> migrate <bar> (without -b)
>>
>> With current patches, it will still use -b.  Fixing it requires still
>> anding more code.  But I think that this use case is so weird what we
>> should not even care about it.
>
> It's a compatibility break.  Whether it's tolerable is a judgement call,
> and not for me to make.
>
> Compatibility breaks need documentation, including release notes.
>
> Say you run migrate with -b by accident (say by recalling a prior
> command from persistent command history, such as qmp-shell's or rlwrap's
> or socat READLINE's), immediately realize what you've done and cancel
> the migration.  Are you then stuck with -b forever?

migrate_set_capability block off

and you are done.


But I think that adding documentation would be longer that just adding
the code to clean it.

Later, Juan.