On Wed, Oct 19, 2022 at 06:43:46PM +0300, Nikolay Borisov wrote: > > > On 18.10.22 г. 13:06 ч., Daniel P. Berrangé wrote: > > On Mon, Oct 10, 2022 at 04:34:00PM +0300, Nikolay Borisov wrote: > > > This is required so that migration stream configuration is written > > > to the migration stream. This would allow analyze-migration to > > > parse enabled capabilities for the migration and adjust its behavior > > > accordingly. This is in preparation for analyze-migration.py to support > > > 'fixed-ram' capability format changes. > > > > > > Signed-off-by: Nikolay Borisov <nbori...@suse.com> > > > --- > > > migration/migration.c | 5 +++++ > > > migration/migration.h | 3 +++ > > > migration/savevm.c | 38 ++++++++++++++++++++++---------------- > > > 3 files changed, 30 insertions(+), 16 deletions(-) > > > > > > diff --git a/migration/migration.c b/migration/migration.c > > > index 140b0f1a54bd..d0779bbaf862 100644 > > > --- a/migration/migration.c > > > +++ b/migration/migration.c > > > @@ -1896,6 +1896,8 @@ static void migrate_fd_cleanup(MigrationState *s) > > > g_free(s->hostname); > > > s->hostname = NULL; > > > + json_writer_free(s->vmdesc); > > > + > > > qemu_savevm_state_cleanup(); > > > if (s->to_dst_file) { > > > @@ -2154,6 +2156,7 @@ void migrate_init(MigrationState *s) > > > error_free(s->error); > > > s->error = NULL; > > > s->hostname = NULL; > > > + s->vmdesc = NULL; > > > migrate_set_state(&s->state, MIGRATION_STATUS_NONE, > > > MIGRATION_STATUS_SETUP); > > > @@ -4269,6 +4272,8 @@ void migrate_fd_connect(MigrationState *s, Error > > > *error_in) > > > return; > > > } > > > + s->vmdesc = json_writer_new(false); > > > + > > > if (multifd_save_setup(&local_err) != 0) { > > > error_report_err(local_err); > > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > > diff --git a/migration/migration.h b/migration/migration.h > > > index cdad8aceaaab..96f27aba2210 100644 > > > --- a/migration/migration.h > > > +++ b/migration/migration.h > > > @@ -17,6 +17,7 @@ > > > #include "exec/cpu-common.h" > > > #include "hw/qdev-core.h" > > > #include "qapi/qapi-types-migration.h" > > > +#include "qapi/qmp/json-writer.h" > > > #include "qemu/thread.h" > > > #include "qemu/coroutine_int.h" > > > #include "io/channel.h" > > > @@ -261,6 +262,8 @@ struct MigrationState { > > > int state; > > > + JSONWriter *vmdesc; > > > + > > > /* State related to return path */ > > > struct { > > > /* Protected by qemu_file_lock */ > > > diff --git a/migration/savevm.c b/migration/savevm.c > > > index 48e85c052c2c..174cdbefc29d 100644 > > > --- a/migration/savevm.c > > > +++ b/migration/savevm.c > > > @@ -1137,13 +1137,18 @@ void qemu_savevm_non_migratable_list(strList > > > **reasons) > > > void qemu_savevm_state_header(QEMUFile *f) > > > { > > > + MigrationState *s = migrate_get_current(); > > > trace_savevm_state_header(); > > > qemu_put_be32(f, QEMU_VM_FILE_MAGIC); > > > qemu_put_be32(f, QEMU_VM_FILE_VERSION); > > > - if (migrate_get_current()->send_configuration) { > > > + if (s->send_configuration) { > > > qemu_put_byte(f, QEMU_VM_CONFIGURATION); > > > - vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0); > > > + json_writer_start_object(s->vmdesc, NULL); > > > + json_writer_start_object(s->vmdesc, "configuration"); > > > + vmstate_save_state(f, &vmstate_configuration, &savevm_state, > > > s->vmdesc); > > > + json_writer_end_object(s->vmdesc); > > > + > > > > IIUC, this is changing the info that is written in the VM > > configuration section, by adding an extra level of nesting > > to the object. > > > > Isn't this going to cause backwards compatibility problems ? > > > > Nothing in the patch seems to take account of the exctra > > 'configuiration' object that has been started > > The resulting json looks like: > > { > "configuration": { > "vmsd_name": "configuration", > "version": 1, > "fields": [ > { > "name": "len", > "type": "uint32", > "size": 4 > }, > { > "name": "name", > "type": "buffer", > "size": 13 > } > ], > "subsections": [ > { > "vmsd_name": "configuration/capabilities", > "version": 1, > "fields": [ > { > "name": "caps_count", > "type": "uint32", > "size": 4 > }, > { > "name": "capabilities", > "type": "capability", > "size": 10 > } > ] > } > ] > }, > "page_size": 4096, > "devices": [ > { > "name": "timer", > "instance_id": 0, > //ommitted > > So the "configuration" object is indeed added, but older versions of qemu > can ignore it without any problem.
IIUC, after looking further, this JSON is only used by the analyze-migration.py script ? If that's right, then we should have the change to analyze-migration.py that copes with the "configuration" option in the same patch. > > Also, there's two json_writer_start_object calls, but only > > one json_writer_end_object. > > That's intentional, the first one begins the top-level object and it is > actually paired with the final call to json_writer_end_object(s->vmdesc); in > qemu_savevm_state_complete_precopy_non_iterable . Oh right, can you add a comment to both locations, mentioning where their respective pair lives. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|