Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor

[Dr. David Alan Gilbert]

* Eric Blake (address@hidden) wrote:

> -static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON 
> *vmdesc)
> +static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
> +                                   Visitor *vmdesc)
>  {
>      int64_t old_offset, size;
> +    const char *tmp;
> 
>      old_offset = qemu_ftell_fast(f);
>      se->ops->save_state(f, se->opaque);
>      size = qemu_ftell_fast(f) - old_offset;
> 
>      if (vmdesc) {
> -        json_prop_int(vmdesc, "size", size);
> -        json_start_array(vmdesc, "fields");
> -        json_start_object(vmdesc, NULL);
> -        json_prop_str(vmdesc, "name", "data");
> -        json_prop_int(vmdesc, "size", size);
> -        json_prop_str(vmdesc, "type", "buffer");
> -        json_end_object(vmdesc);
> -        json_end_array(vmdesc);
> +        visit_type_int(vmdesc, "size", &size, &error_abort);
> +        visit_start_list(vmdesc, "fields", NULL, 0, &error_abort);
> +        visit_start_struct(vmdesc, NULL, NULL, 0, &error_abort);

Please avoid error_abort in migration code, especially on the source side.
You've got an apparently happily working VM, we must never kill it 
while attempting migration.

Dave

> +        tmp = "data";
> +        visit_type_str(vmdesc, "name", (char **)&tmp, &error_abort);
> +        visit_type_int(vmdesc, "size", &size, &error_abort);
> +        tmp = "buffer";
> +        visit_type_str(vmdesc, "type", (char **)&tmp, &error_abort);
> +        visit_check_struct(vmdesc, &error_abort);
> +        visit_end_struct(vmdesc);
> +        visit_end_list(vmdesc);
>      }
>  }
> 
> -static void vmstate_save(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
> +static void vmstate_save(QEMUFile *f, SaveStateEntry *se, Visitor *vmdesc)
>  {
>      trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
>      if (!se->vmsd) {
> @@ -891,7 +896,7 @@ void qemu_savevm_state_header(QEMUFile *f)
> 
>      if (!savevm_state.skip_configuration || enforce_config_section()) {
>          qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> -        vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> +        vmstate_save_state(f, &vmstate_configuration, &savevm_state, NULL);
>      }
> 
>  }
> @@ -1033,11 +1038,15 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
> 
>  void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
>  {
> -    QJSON *vmdesc;
> +    JsonOutputVisitor *vmdesc_jov;
> +    Visitor *vmdesc;
> +    char *vmdesc_str;
>      int vmdesc_len;
>      SaveStateEntry *se;
>      int ret;
>      bool in_postcopy = migration_in_postcopy(migrate_get_current());
> +    int64_t tmp_i;
> +    char *tmp_s;
> 
>      trace_savevm_state_complete_precopy();
> 
> @@ -1073,9 +1082,12 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, 
> bool iterable_only)
>          return;
>      }
> 
> -    vmdesc = qjson_new();
> -    json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
> -    json_start_array(vmdesc, "devices");
> +    vmdesc_jov = json_output_visitor_new();
> +    vmdesc = json_output_get_visitor(vmdesc_jov);
> +    visit_start_struct(vmdesc, NULL, NULL, 0, &error_abort);
> +    tmp_i = TARGET_PAGE_SIZE;
> +    visit_type_int(vmdesc, "page_size", &tmp_i, &error_abort);
> +    visit_start_list(vmdesc, "devices", NULL, 0, &error_abort);
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> 
>          if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
> @@ -1088,16 +1100,19 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, 
> bool iterable_only)
> 
>          trace_savevm_section_start(se->idstr, se->section_id);
> 
> -        json_start_object(vmdesc, NULL);
> -        json_prop_str(vmdesc, "name", se->idstr);
> -        json_prop_int(vmdesc, "instance_id", se->instance_id);
> +        visit_start_struct(vmdesc, NULL, NULL, 0, &error_abort);
> +        tmp_s = se->idstr;
> +        visit_type_str(vmdesc, "name", &tmp_s, &error_abort);
> +        tmp_i = se->instance_id;
> +        visit_type_int(vmdesc, "instance_id", &tmp_i, &error_abort);
> 
>          save_section_header(f, se, QEMU_VM_SECTION_FULL);
>          vmstate_save(f, se, vmdesc);
>          trace_savevm_section_end(se->idstr, se->section_id, 0);
>          save_section_footer(f, se);
> 
> -        json_end_object(vmdesc);
> +        visit_check_struct(vmdesc, &error_abort);
> +        visit_end_struct(vmdesc);
>      }
> 
>      if (!in_postcopy) {
> @@ -1105,16 +1120,19 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, 
> bool iterable_only)
>          qemu_put_byte(f, QEMU_VM_EOF);
>      }
> 
> -    json_end_array(vmdesc);
> -    qjson_finish(vmdesc);
> -    vmdesc_len = strlen(qjson_get_str(vmdesc));
> +    visit_end_list(vmdesc);
> +    visit_check_struct(vmdesc, &error_abort);
> +    visit_end_struct(vmdesc);
> +    vmdesc_str = json_output_get_string(vmdesc_jov);
> +    vmdesc_len = strlen(vmdesc_str);
> 
>      if (should_send_vmdesc()) {
>          qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
>          qemu_put_be32(f, vmdesc_len);
> -        qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
> +        qemu_put_buffer(f, (uint8_t *)vmdesc_str, vmdesc_len);
>      }
> -    object_unref(OBJECT(vmdesc));
> +    g_free(vmdesc_str);
> +    json_output_visitor_cleanup(vmdesc_jov);
> 
>      qemu_fflush(f);
>  }
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index bf3d5db..e7f894c 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -1,15 +1,15 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
> +#include "qapi/error.h"
>  #include "migration/migration.h"
>  #include "migration/qemu-file.h"
>  #include "migration/vmstate.h"
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
>  #include "trace.h"
> -#include "qjson.h"
> 
>  static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription 
> *vmsd,
> -                                    void *opaque, QJSON *vmdesc);
> +                                    void *opaque, Visitor *vmdesc);
>  static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription 
> *vmsd,
>                                     void *opaque);
> 
> @@ -226,12 +226,15 @@ static bool vmsd_can_compress(VMStateField *field)
>      return true;
>  }
> 
> -static void vmsd_desc_field_start(const VMStateDescription *vmsd, QJSON 
> *vmdesc,
> +static void vmsd_desc_field_start(const VMStateDescription *vmsd,
> +                                  Visitor *vmdesc,
>                                    VMStateField *field, int i, int max)
>  {
>      char *name, *old_name;
>      bool is_array = max > 1;
>      bool can_compress = vmsd_can_compress(field);
> +    int64_t tmp;
> +    const char *type;
> 
>      if (!vmdesc) {
>          return;
> @@ -247,38 +250,47 @@ static void vmsd_desc_field_start(const 
> VMStateDescription *vmsd, QJSON *vmdesc,
>          g_free(old_name);
>      }
> 
> -    json_start_object(vmdesc, NULL);
> -    json_prop_str(vmdesc, "name", name);
> +    visit_start_struct(vmdesc, NULL, NULL, 0, &error_abort);
> +    visit_type_str(vmdesc, "name", &name, &error_abort);
>      if (is_array) {
>          if (can_compress) {
> -            json_prop_int(vmdesc, "array_len", max);
> +            tmp = max;
> +            visit_type_int(vmdesc, "array_len", &tmp, &error_abort);
>          } else {
> -            json_prop_int(vmdesc, "index", i);
> +            tmp = i;
> +            visit_type_int(vmdesc, "index", &tmp, &error_abort);
>          }
>      }
> -    json_prop_str(vmdesc, "type", vmfield_get_type_name(field));
> +    type = vmfield_get_type_name(field);
> +    visit_type_str(vmdesc, "type", (char **)&type, &error_abort);
> 
>      if (field->flags & VMS_STRUCT) {
> -        json_start_object(vmdesc, "struct");
> +        visit_start_struct(vmdesc, "struct", NULL, 0, &error_abort);
>      }
> 
>      g_free(name);
>  }
> 
> -static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON 
> *vmdesc,
> +static void vmsd_desc_field_end(const VMStateDescription *vmsd,
> +                                Visitor *vmdesc,
>                                  VMStateField *field, size_t size, int i)
>  {
> +    int64_t tmp;
> +
>      if (!vmdesc) {
>          return;
>      }
> 
>      if (field->flags & VMS_STRUCT) {
>          /* We printed a struct in between, close its child object */
> -        json_end_object(vmdesc);
> +        visit_check_struct(vmdesc, &error_abort);
> +        visit_end_struct(vmdesc);
>      }
> 
> -    json_prop_int(vmdesc, "size", size);
> -    json_end_object(vmdesc);
> +    tmp = size;
> +    visit_type_int(vmdesc, "size", &tmp, &error_abort);
> +    visit_check_struct(vmdesc, &error_abort);
> +    visit_end_struct(vmdesc);
>  }
> 
> 
> @@ -293,7 +305,7 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, 
> void *opaque)
> 
> 
>  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
> -                        void *opaque, QJSON *vmdesc)
> +                        void *opaque, Visitor *vmdesc)
>  {
>      VMStateField *field = vmsd->fields;
> 
> @@ -302,9 +314,11 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>      }
> 
>      if (vmdesc) {
> -        json_prop_str(vmdesc, "vmsd_name", vmsd->name);
> -        json_prop_int(vmdesc, "version", vmsd->version_id);
> -        json_start_array(vmdesc, "fields");
> +        const char *tmp_s = vmsd->name;
> +        int64_t tmp_i = vmsd->version_id;
> +        visit_type_str(vmdesc, "vmsd_name", (char **)&tmp_s, &error_abort);
> +        visit_type_int(vmdesc, "version", &tmp_i, &error_abort);
> +        visit_start_list(vmdesc, "fields", NULL, 0, &error_abort);
>      }
> 
>      while (field->name) {
> @@ -314,7 +328,7 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>              int i, n_elems = vmstate_n_elems(opaque, field);
>              int size = vmstate_size(opaque, field);
>              int64_t old_offset, written_bytes;
> -            QJSON *vmdesc_loop = vmdesc;
> +            Visitor *vmdesc_loop = vmdesc;
> 
>              for (i = 0; i < n_elems; i++) {
>                  void *addr = base_addr + size * i;
> @@ -350,7 +364,7 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>      }
> 
>      if (vmdesc) {
> -        json_end_array(vmdesc);
> +        visit_end_list(vmdesc);
>      }
> 
>      vmstate_subsection_save(f, vmsd, opaque, vmdesc);
> @@ -420,7 +434,7 @@ static int vmstate_subsection_load(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  }
> 
>  static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription 
> *vmsd,
> -                                    void *opaque, QJSON *vmdesc)
> +                                    void *opaque, Visitor *vmdesc)
>  {
>      const VMStateDescription **sub = vmsd->subsections;
>      bool subsection_found = false;
> @@ -433,11 +447,12 @@ static void vmstate_subsection_save(QEMUFile *f, const 
> VMStateDescription *vmsd,
>              if (vmdesc) {
>                  /* Only create subsection array when we have any */
>                  if (!subsection_found) {
> -                    json_start_array(vmdesc, "subsections");
> +                    visit_start_list(vmdesc, "subsections", NULL, 0,
> +                                     &error_abort);
>                      subsection_found = true;
>                  }
> 
> -                json_start_object(vmdesc, NULL);
> +                visit_start_struct(vmdesc, NULL, NULL, 0, &error_abort);
>              }
> 
>              qemu_put_byte(f, QEMU_VM_SUBSECTION);
> @@ -448,14 +463,15 @@ static void vmstate_subsection_save(QEMUFile *f, const 
> VMStateDescription *vmsd,
>              vmstate_save_state(f, vmsd, opaque, vmdesc);
> 
>              if (vmdesc) {
> -                json_end_object(vmdesc);
> +                visit_check_struct(vmdesc, &error_abort);
> +                visit_end_struct(vmdesc);
>              }
>          }
>          sub++;
>      }
> 
>      if (vmdesc && subsection_found) {
> -        json_end_array(vmdesc);
> +        visit_end_list(vmdesc);
>      }
>  }
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 0b5a7a8..1f8a39d 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -442,7 +442,7 @@ tests/test-qdev-global-props$(EXESUF): 
> tests/test-qdev-global-props.o \
>       $(test-qapi-obj-y)
>  tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
>       migration/vmstate.o migration/qemu-file.o migration/qemu-file-buf.o \
> -        migration/qemu-file-unix.o qjson.o \
> +        migration/qemu-file-unix.o qapi/json-output-visitor.o \
>       $(test-qom-obj-y)
>  tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
>       $(test-util-obj-y)
> -- 
> 2.5.5
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK