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
Also, there's two json_writer_start_object calls, but only
one json_writer_end_object.
BTW, some <tab> crept into this patch.
> }
> }
>
> @@ -1364,15 +1369,16 @@ int
> qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> bool in_postcopy,
> bool inactivate_disks)
> {
> - g_autoptr(JSONWriter) vmdesc = NULL;
> + MigrationState *s = migrate_get_current();
> int vmdesc_len;
> SaveStateEntry *se;
> int ret;
>
> - vmdesc = json_writer_new(false);
> - json_writer_start_object(vmdesc, NULL);
> - json_writer_int64(vmdesc, "page_size", qemu_target_page_size());
> - json_writer_start_array(vmdesc, "devices");
> + if (!s->send_configuration) {
> + json_writer_start_object(s->vmdesc, NULL);
> + }
> + json_writer_int64(s->vmdesc, "page_size", qemu_target_page_size());
> + json_writer_start_array(s->vmdesc, "devices");
> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>
> if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
> @@ -1385,12 +1391,12 @@ int
> qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>
> trace_savevm_section_start(se->idstr, se->section_id);
>
> - json_writer_start_object(vmdesc, NULL);
> - json_writer_str(vmdesc, "name", se->idstr);
> - json_writer_int64(vmdesc, "instance_id", se->instance_id);
> + json_writer_start_object(s->vmdesc, NULL);
> + json_writer_str(s->vmdesc, "name", se->idstr);
> + json_writer_int64(s->vmdesc, "instance_id", se->instance_id);
>
> save_section_header(f, se, QEMU_VM_SECTION_FULL);
> - ret = vmstate_save(f, se, vmdesc);
> + ret = vmstate_save(f, se, s->vmdesc);
> if (ret) {
> qemu_file_set_error(f, ret);
> return ret;
> @@ -1398,7 +1404,7 @@ int
> qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> trace_savevm_section_end(se->idstr, se->section_id, 0);
> save_section_footer(f, se);
>
> - json_writer_end_object(vmdesc);
> + json_writer_end_object(s->vmdesc);
> }
>
> if (inactivate_disks) {
> @@ -1417,14 +1423,14 @@ int
> qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> qemu_put_byte(f, QEMU_VM_EOF);
> }
>
> - json_writer_end_array(vmdesc);
> - json_writer_end_object(vmdesc);
> - vmdesc_len = strlen(json_writer_get(vmdesc));
> + json_writer_end_array(s->vmdesc);
> + json_writer_end_object(s->vmdesc);
> + vmdesc_len = strlen(json_writer_get(s->vmdesc));
>
> if (should_send_vmdesc()) {
> qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
> qemu_put_be32(f, vmdesc_len);
> - qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), vmdesc_len);
> + qemu_put_buffer(f, (uint8_t *)json_writer_get(s->vmdesc),
> vmdesc_len);
> }
>
> return 0;
> --
> 2.34.1
>
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 :|
