qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats aft


From: Michael R. Hines
Subject: Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination
Date: Wed, 19 Feb 2014 12:22:55 +0800
User-agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.3.0

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]