Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 02/19/2014 10:40 AM, Eric Blake wrote:
> On 02/18/2014 07:30 PM, Michael R. Hines wrote:
>
>>> qemu 2.0 -> 2.0: pass the smaller struct from source, expect the smaller
>>> struct on dest, no problem
>>> qemu 2.0 -> 2.1: pass the smaller struct from source, dest notices the
>>> optional field is missing and copes with no problem
>>> qemu 2.1 -> 2.1: pass the larger struct from source, dest handles the
>>> larger struct with no problem
>>> qemu 2.1 -> 2.0: pass the larger struct from source, but dest is only
>>> expecting the smaller struct.
>>>
>>> The last case is the one I worry about: does your implementation
>>> gracefully ignore fields that it was not expecting when reconstituting
>>> the MigrationInfo on the dest, or does it error out, losing all
>>> information in the process?
>>>
>>> On the other hand, upstream qemu seldom worries about down-version
>>> migrations (we strive hard to make sure 2.0 -> 2.1 works, but aren't too
>>> worried if 2.1 -> 2.0 fails) - it tends to be more of a situation that
>>> downstream distros provide value added by worrying about down-migration.
>>>    So my concern about what happens on down-migration is not a
>>> show-stopper for your patch idea.
>>>
>> Excellent question! I had not even considered that. I think this
>> could be solved with QObject arcitecture: So when the statistics
>> are received by the destination and deserialized, the conversion
>> from JSON to QObject would need to check to see if the struct
>> has all the expected fields, and if those fields are not there then
>> do not "bomb" or anything.
> Expected fields being present is not the problem.  As I said above, as
> long as we are careful to make all future additions to MigrationInfo be
> marked optional, then the field can be missing from an older source, and
> a newer destination will handle the missing fields just fine.
>
> The only problem is extra fields.  If a newer source sends to an older
> destination, the software will either die because of unexpected fields,
> or it will silently ignore the unexpected fields.  Normally in QObject,
> we want to die on unexpected fields (QMP should be up-front and tell
> users that they are passing in too much stuff) - but this case is the
> exception to the rule, and we want to ignore the extra stuff.  That's
> where you'll probably have to patch something up; I'm also not sure
> whether you should expose your actual migrate-set-last-info as a QMP
> command, or if you instead just do it internally as part of the
> migration stream but never let QMP modify it.  Doing it internally only,
> and not via QMP, will make it easier to stick to the rule of thumb that
> QMP should reject unknown dictionary members while your internal version
> silently ignores them.  And again, this only matters for downgrades,
> where upstream is already less concerned if it doesn't work.

OK, that makes sense. I'll remove the QMP command as well as
work on the patch to deal with downgrades gracefully so that we
don't die on unexpected fields.

>> A deeper question would be: Let's assume a migrate to a lower
>> version, as in your example: Should the QMP statistics also include
>> the version of of the source QEMU that the guest originated from?
>> I could easily modify the patch to include that value.......
> Not necessary.  Upgrades will already work without a version field, as
> long as all new fields are marked optional.  And management apps can
> already figure out the qemu version of both source and destination qemu
> outside of the migration stats, so that sticking redundant version
> information into MigrationInfo just becomes bloat.
>

On the other-hand: Did you notice the patch sets about "localhost"
migration that are in review? If someone is *explicity* using migration
in this way to perform QEMU upgrades, a field that indicates the
previous version for debugging purposes would probably be helpful.

But, I'll ignore it for now - as I don't have a compelling use case for it.

- Michael