* [Qemu-devel] [RFC PATCH v1 1/3] provenance: QMP command to store MigrationInfo from JSON
2014-02-18 5:53 [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination mrhines
@ 2014-02-18 5:53 ` mrhines
2014-02-18 5:53 ` [Qemu-devel] [RFC PATCH v1 2/3] provenance: serialize MigrationInfo across the wire mrhines
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: mrhines @ 2014-02-18 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
From: "Michael R. Hines" <mrhines@us.ibm.com>
This is the QMP documentation and schema for a command that allows
the migration protocol on the destination side to 'install' a whole
MigrationInfo json structure as received in-tact from the source
into the MigrationState struct so that query-migrate can print the
information as-is without having to install the values one-by-one.
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
qapi-schema.json | 14 ++++++++++++++
qmp-commands.hx | 33 +++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 7cfb5e5..c84ca0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -717,6 +717,20 @@
'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
##
+# @migrate-set-last-info
+#
+# Store migration info/statistics. Currently only used by the migration
+# protocol itself to tell the destination about the results before
+# the source QEMU is destroyed.
+#
+# @info: json array of MigrationInfo statistics from a previous migration.
+#
+# Since: 2.0
+##
+{ 'command': 'migrate-set-last-info',
+ 'data': { 'info': 'MigrationInfo' } }
+
+##
# @query-migrate-capabilities
#
# Returns information about the current migration capabilities status
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cce6b81..1108940 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3015,6 +3015,39 @@ EQMP
.params = "capability:s,state:b",
.mhandler.cmd_new = qmp_marshal_input_migrate_set_capabilities,
},
+
+SQMP
+migrate-set-last-info
+------------------------
+
+Store migration info/statistics. Currently only used by the migration
+protocol itself to tell the destination about the results before
+the source QEMU is destroyed.
+
+NOTE: There is no need to execute this command explicitly except for
+testing, as this information is already transmitted as part of the
+migration protocol. Thus, there is no HMP command made available for
+this option - only QMP.
+
+The example below is simple, but the migration protocol will actually
+transfer the entire JSON string that originated from the source, no
+matter what is inside of it, which should survive future version of
+QEMU when new statistics are added to the protocol.
+
+Arguments:
+- info: MigrationInfo statistics dictionary (json-object)
+
+Example:
+
+-> { "execute": "migrate-set-last-info" , "arguments":
+ { "info": { "downtime": 1234, "ram" : { "transferred" : 5678 } } } }
+EQMP
+ {
+ .name = "migrate-set-last-info",
+ .args_type = "info:O",
+ .mhandler.cmd_new = qmp_marshal_input_migrate_set_last_info,
+ },
+
SQMP
query-migrate-capabilities
--------------------------
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC PATCH v1 2/3] provenance: serialize MigrationInfo across the wire
2014-02-18 5:53 [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination mrhines
2014-02-18 5:53 ` [Qemu-devel] [RFC PATCH v1 1/3] provenance: QMP command to store MigrationInfo from JSON mrhines
@ 2014-02-18 5:53 ` mrhines
2014-02-18 5:53 ` [Qemu-devel] [RFC PATCH v1 3/3] provenance: expose the serialization save/load functions for migration mrhines
2014-02-18 13:40 ` [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination Eric Blake
3 siblings, 0 replies; 8+ messages in thread
From: mrhines @ 2014-02-18 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
From: "Michael R. Hines" <mrhines@us.ibm.com>
At the end of the migration, we use the existing QMP json manipulation
functions and the query-migrate QMP function to serialize the MigrationInfo
structure that was populated in the 'COMPLETED' state of the migration.
This code does not need to know anything about the actual contents of the
MigrationInfo structure, as the QMP code has taken care of all of the hard
work of serializing and visiting all of the individual fields of the structure.
Once we have a QObject, we convert that to JSON and send it across the
connection at the end of the migration.
The only exception to the serialization is the 'downtime' field. Previously
this was calculated inside the migration_thread, but because want to ensure
that this field is serialized as well, we need to calculate it just a little
bit earlier.
Once the json is received on the other side, we perform the reverse of
all the previous steps and convert the json string back into a MigrationInfo
structure just as it was before.
The information in this structure will only last as long as the next migration,
so if another migration occurs, it is the responsibility of the management
software to extract the information before the next migration begins or else
the information will be lost.
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
migration.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 121 insertions(+), 5 deletions(-)
diff --git a/migration.c b/migration.c
index 25add6f..8acabf0 100644
--- a/migration.c
+++ b/migration.c
@@ -24,6 +24,10 @@
#include "migration/block.h"
#include "qemu/thread.h"
#include "qmp-commands.h"
+#include "qapi/qmp/qobject.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qint.h"
+#include "qapi/qmp/qjson.h"
#include "trace.h"
//#define DEBUG_MIGRATION
@@ -183,6 +187,115 @@ static void get_xbzrle_cache_stats(MigrationInfo *info)
}
}
+/*
+ * 'last_info' is not a device, per-se, but the existing device state
+ * state transfer mechanism is very easy to use for this purpose, as
+ * we're only (currently) interested in communicating this information
+ * in-band after the last migration round has completed (as opposed to
+ * out-of-band, which would require more intelligence in the management
+ * software, for example).
+ */
+void migrate_info_save(QEMUFile *f, void *opaque)
+{
+ MigrationState *s = migrate_get_current();
+ QObject *info;
+ QString * qjson;
+ QInt *downtime;
+ uint32_t len;
+ const char * json;
+ int64_t end_time;
+
+ /*
+ * Use the existing QMP command to get the statistics.
+ */
+ qmp_marshal_input_query_migrate(NULL, NULL, &info);
+
+ /*
+ * Update the structure with the final downtime of the migration.
+ */
+ end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+ s->total_time = end_time - s->total_time;
+ s->downtime = end_time - s->start_time;
+ downtime = qint_from_int(s->downtime);
+
+ qdict_put_obj(qobject_to_qdict(info), "downtime", QOBJECT(downtime));
+
+ /*
+ * Serialize into raw JSON and send to other side only
+ * after the last migration round has completed.
+ */
+ qjson = qobject_to_json(info);
+ json = qstring_get_str(qjson);
+
+ len = strlen(json);
+ qemu_put_be32(f, len);
+ qemu_put_buffer(f, (uint8_t *)json, len);
+
+ qemu_fflush(f);
+
+ qobject_decref(QOBJECT(qjson));
+ qobject_decref(info);
+ qobject_decref(QOBJECT(downtime));
+}
+
+/*
+ * The source has nearly completed the migration and has sent
+ * out a JSON description of the migration performance statistics.
+ * Load it and use the QMP 'migrate-set-last-info' command to automatically
+ * convert it into a usable structure.
+ */
+int migrate_info_load(QEMUFile *f, void *opaque, int version_id)
+{
+ uint32_t len = qemu_get_be32(f);
+ char * json = g_malloc0(len + 1);
+ QDict *info = qdict_new();
+
+ qemu_get_buffer(f, (uint8_t *)json, len);
+
+ /*
+ * Construct a { 'info' : MigrationInfo } structure
+ * out of the resulting JSON for QMP to parse the input.
+ */
+ qdict_put_obj(info, "info", qobject_from_json(json));
+ DPRINTF("migrate_info_load: %s\n", json);
+ g_free(json);
+ qmp_marshal_input_migrate_set_last_info(NULL, info, NULL);
+ qobject_decref(QOBJECT(info));
+
+ return 0;
+}
+
+/*
+ * JSON from the migration source was received - now save it.
+ */
+void qmp_migrate_set_last_info(MigrationInfo *info, Error **errp)
+{
+ MigrationState *s = migrate_get_current();
+
+ if (!info) {
+ fprintf(stderr, "error: migrate-set-last-info dictionary is empty!\n");
+ return;
+ }
+
+ if (!s->last_info) {
+ s->last_info = g_malloc0(sizeof(*s->last_info));
+ } else {
+ g_free(s->last_info->ram);
+ }
+
+ memcpy(s->last_info, info, sizeof(*s->last_info));
+ s->last_info->ram = NULL;
+
+ if (info->has_ram) {
+ s->last_info->ram = g_malloc0(sizeof(*s->last_info->ram));
+ memcpy(s->last_info->ram, info->ram, sizeof(*s->last_info->ram));
+ }
+
+ s->last_info->has_status = false;
+ s->migrated_before = true;
+}
+
+
MigrationInfo *qmp_query_migrate(Error **errp)
{
MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -191,6 +304,11 @@ MigrationInfo *qmp_query_migrate(Error **errp)
switch (s->state) {
case MIG_STATE_NONE:
/* no migration has happened ever */
+ if (s->migrated_before) {
+ memcpy(info, s->last_info, sizeof(*info));
+ info->has_status = true;
+ info->status = g_strdup("last");
+ }
break;
case MIG_STATE_SETUP:
info->has_status = true;
@@ -580,9 +698,10 @@ static void *migration_thread(void *opaque)
int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
int64_t initial_bytes = 0;
int64_t max_size = 0;
- int64_t start_time = initial_time;
bool old_vm_running = false;
+ s->start_time = initial_time;
+
DPRINTF("beginning savevm\n");
qemu_savevm_state_begin(s->file, &s->params);
@@ -607,7 +726,7 @@ static void *migration_thread(void *opaque)
DPRINTF("done iterating\n");
qemu_mutex_lock_iothread();
- start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+ s->start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
old_vm_running = runstate_is_running();
@@ -665,9 +784,6 @@ static void *migration_thread(void *opaque)
qemu_mutex_lock_iothread();
if (s->state == MIG_STATE_COMPLETED) {
- int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
- s->total_time = end_time - s->total_time;
- s->downtime = end_time - start_time;
runstate_set(RUN_STATE_POSTMIGRATE);
} else {
if (old_vm_running) {
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC PATCH v1 3/3] provenance: expose the serialization save/load functions for migration
2014-02-18 5:53 [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination mrhines
2014-02-18 5:53 ` [Qemu-devel] [RFC PATCH v1 1/3] provenance: QMP command to store MigrationInfo from JSON mrhines
2014-02-18 5:53 ` [Qemu-devel] [RFC PATCH v1 2/3] provenance: serialize MigrationInfo across the wire mrhines
@ 2014-02-18 5:53 ` mrhines
2014-02-18 13:40 ` [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination Eric Blake
3 siblings, 0 replies; 8+ messages in thread
From: mrhines @ 2014-02-18 5:53 UTC (permalink / raw)
To: qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
From: "Michael R. Hines" <mrhines@us.ibm.com>
Expose the prototypes of the serialization code and register the
save/load functions during QEMU startup so that they can take effect.
Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
include/migration/migration.h | 8 ++++++++
vl.c | 2 ++
2 files changed, 10 insertions(+)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 3e1e6c7..b5d4e6f 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -53,6 +53,7 @@ struct MigrationState
int state;
MigrationParams params;
double mbps;
+ int64_t start_time;
int64_t total_time;
int64_t downtime;
int64_t expected_downtime;
@@ -61,6 +62,8 @@ struct MigrationState
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
int64_t xbzrle_cache_size;
int64_t setup_time;
+ MigrationInfo *last_info;
+ bool migrated_before;
};
void process_incoming_migration(QEMUFile *f);
@@ -174,4 +177,9 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
ram_addr_t offset, size_t size,
int *bytes_sent);
+#define STATS_VERSION 1
+
+int migrate_info_load(QEMUFile *f, void *opaque, int version_id);
+void migrate_info_save(QEMUFile *f, void *opaque);
+
#endif
diff --git a/vl.c b/vl.c
index 316de54..b9f3661 100644
--- a/vl.c
+++ b/vl.c
@@ -4138,6 +4138,8 @@ int main(int argc, char **argv, char **envp)
default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
+ register_savevm(NULL, "info", -1, STATS_VERSION, migrate_info_save,
+ migrate_info_load, NULL);
if (nb_numa_nodes > 0) {
int i;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination
2014-02-18 5:53 [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination mrhines
` (2 preceding siblings ...)
2014-02-18 5:53 ` [Qemu-devel] [RFC PATCH v1 3/3] provenance: expose the serialization save/load functions for migration mrhines
@ 2014-02-18 13:40 ` Eric Blake
2014-02-19 2:30 ` Michael R. Hines
3 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2014-02-18 13:40 UTC (permalink / raw)
To: mrhines, qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
[-- Attachment #1: Type: text/plain, Size: 3181 bytes --]
On 02/17/2014 10:53 PM, mrhines@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
>
> This series allows us to send the contents of the entire MigrationInfo structure to the
> destination once the migration is over. This is very useful for analyzing
> the result of a migration, and particularly useful for management software and
> policy. Normally, the information ins MigrationInfo is completely lost when
> the source VM is destroyed, but with this series, we can preserve the statistics
> for retrieval.
>
> The QMP architecture has sufficient functionality to serialize and
> deserialize arbitrary objects into JSON and back again (very nice).
>
> We only store the stats from the *last* migration, but not all previous migrations,
> as that could potentially get unwieldy. Thus, it's the management software's (or user's)
> responsibility to retrieve the statistics before another migration occurs.
I like the idea. It seems a little awkward that the final stats are
only on the destination, not the source, since it was the source that
triggered the migration. But on further thought, migration itself is
one-way (destination never sends anything to the source), so destination
really is the only point that can see the full picture without making
the migration protocol changed to be two-way. Meanwhile, libvirt should
be able to cope with it (libvirt does a handshake between source and
destination, and does not kill the source until after the destination
acknowledges success - so libvirt could query the destination stats,
then pass them back over the handshake, so that libvirt could present
the final stats from the source even though qemu can't).
Now, as to your assertion that serializing the MigrationInfo will always
work - I'm not quite sure about that. Let's suppose we add a new field
to the struct in qemu 2.1. For back-compat reasons, the new field MUST
be marked as optional in the qapi schema. Here are the possibilities:
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination
2014-02-18 13:40 ` [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination Eric Blake
@ 2014-02-19 2:30 ` Michael R. Hines
2014-02-19 2:40 ` Eric Blake
0 siblings, 1 reply; 8+ messages in thread
From: Michael R. Hines @ 2014-02-19 2:30 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
On 02/18/2014 09:40 PM, Eric Blake wrote:
> On 02/17/2014 10:53 PM, mrhines@linux.vnet.ibm.com wrote:
>> From: "Michael R. Hines" <mrhines@us.ibm.com>
>>
>> This series allows us to send the contents of the entire MigrationInfo structure to the
>> destination once the migration is over. This is very useful for analyzing
>> the result of a migration, and particularly useful for management software and
>> policy. Normally, the information ins MigrationInfo is completely lost when
>> the source VM is destroyed, but with this series, we can preserve the statistics
>> for retrieval.
>>
>> The QMP architecture has sufficient functionality to serialize and
>> deserialize arbitrary objects into JSON and back again (very nice).
>>
>> We only store the stats from the *last* migration, but not all previous migrations,
>> as that could potentially get unwieldy. Thus, it's the management software's (or user's)
>> responsibility to retrieve the statistics before another migration occurs.
> I like the idea. It seems a little awkward that the final stats are
> only on the destination, not the source, since it was the source that
> triggered the migration. But on further thought, migration itself is
> one-way (destination never sends anything to the source), so destination
> really is the only point that can see the full picture without making
> the migration protocol changed to be two-way. Meanwhile, libvirt should
> be able to cope with it (libvirt does a handshake between source and
> destination, and does not kill the source until after the destination
> acknowledges success - so libvirt could query the destination stats,
> then pass them back over the handshake, so that libvirt could present
> the final stats from the source even though qemu can't).
Yes, exactly. That makes sense. I would be happy to scrape together
a libvirt patch that does that. Sending the stats back over the handshake
seems very promising.
> Now, as to your assertion that serializing the MigrationInfo will always
> work - I'm not quite sure about that. Let's suppose we add a new field
> to the struct in qemu 2.1. For back-compat reasons, the new field MUST
> be marked as optional in the qapi schema. Here are the possibilities:
>
> 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.
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.......
- Michael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination
2014-02-19 2:30 ` Michael R. Hines
@ 2014-02-19 2:40 ` Eric Blake
2014-02-19 4:22 ` Michael R. Hines
0 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2014-02-19 2:40 UTC (permalink / raw)
To: Michael R. Hines, qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
[-- Attachment #1: Type: text/plain, Size: 3536 bytes --]
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.
>
> 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 0/3] provenance: save migration stats after completion to destination
2014-02-19 2:40 ` Eric Blake
@ 2014-02-19 4:22 ` Michael R. Hines
0 siblings, 0 replies; 8+ messages in thread
From: Michael R. Hines @ 2014-02-19 4:22 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: hinesmr, Michael R. Hines, quintela
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
^ permalink raw reply [flat|nested] 8+ messages in thread