Re: [Qemu-devel] [PATCH 01/12] savevm: Use a struct to pass all handlers

[Orit Wasserman]

On 06/28/2012 10:21 PM, Juan Quintela wrote:
> This would make easier to add more operations in the next patches.
> 
> Signed-off-by: Juan Quintela <address@hidden>
> ---
>  savevm.c  |   54 +++++++++++++++++++++++++-----------------------------
>  vmstate.h |    7 +++++++
>  2 files changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index a15c163..73626d4 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1171,10 +1171,7 @@ typedef struct SaveStateEntry {
>      int alias_id;
>      int version_id;
>      int section_id;
> -    SaveSetParamsHandler *set_params;
> -    SaveLiveStateHandler *save_live_state;
> -    SaveStateHandler *save_state;
> -    LoadStateHandler *load_state;
> +    SaveVMHandlers *ops;
>      const VMStateDescription *vmsd;
>      void *opaque;
>      CompatEntry *compat;
> @@ -1237,10 +1234,11 @@ int register_savevm_live(DeviceState *dev,
>      se = g_malloc0(sizeof(SaveStateEntry));
>      se->version_id = version_id;
>      se->section_id = global_section_id++;
> -    se->set_params = set_params;
> -    se->save_live_state = save_live_state;
> -    se->save_state = save_state;
> -    se->load_state = load_state;
> +    se->ops = g_malloc0(sizeof(SaveVMHandlers));
> +    se->ops->set_params = set_params;
> +    se->ops->save_live_state = save_live_state;
> +    se->ops->save_state = save_state;
> +    se->ops->load_state = load_state;
>      se->opaque = opaque;
>      se->vmsd = NULL;
>      se->no_migrate = 0;
> @@ -1309,6 +1307,7 @@ void unregister_savevm(DeviceState *dev, const char 
> *idstr, void *opaque)
>              if (se->compat) {
>                  g_free(se->compat);
>              }
> +            g_free(se->ops);
>              g_free(se);
>          }
>      }
> @@ -1327,9 +1326,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, 
> int instance_id,
>      se = g_malloc0(sizeof(SaveStateEntry));
>      se->version_id = vmsd->version_id;
>      se->section_id = global_section_id++;
> -    se->save_live_state = NULL;
> -    se->save_state = NULL;
> -    se->load_state = NULL;
>      se->opaque = opaque;
>      se->vmsd = vmsd;
>      se->alias_id = alias_id;
> @@ -1524,7 +1520,7 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
>  {
>      if (!se->vmsd) {         /* Old style */
> -        return se->load_state(f, se->opaque, version_id);
> +        return se->ops->load_state(f, se->opaque, version_id);
>      }
>      return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
>  }
> @@ -1532,7 +1528,7 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry 
> *se, int version_id)
>  static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
>  {
>      if (!se->vmsd) {         /* Old style */
> -        se->save_state(f, se->opaque);
> +        se->ops->save_state(f, se->opaque);
>          return;
>      }
>      vmstate_save_state(f,se->vmsd, se->opaque);
> @@ -1569,10 +1565,10 @@ int qemu_savevm_state_begin(QEMUFile *f,
>      int ret;
> 
>      QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> -        if(se->set_params == NULL) {
> +        if (!se->ops || !se->ops->set_params) {
>              continue;
>          }
> -        se->set_params(params, se->opaque);
> +        se->ops->set_params(params, se->opaque);
>      }
>      
>      qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
> @@ -1581,9 +1577,9 @@ int qemu_savevm_state_begin(QEMUFile *f,
>      QTAILQ_FOREACH(se, &savevm_handlers, entry) {
>          int len;
> 
> -        if (se->save_live_state == NULL)
> +        if (!se->ops || !se->ops->save_live_state) {
>              continue;
> -
> +        }
>          /* Section type */
>          qemu_put_byte(f, QEMU_VM_SECTION_START);
>          qemu_put_be32(f, se->section_id);
> @@ -1596,7 +1592,7 @@ int qemu_savevm_state_begin(QEMUFile *f,
>          qemu_put_be32(f, se->instance_id);
>          qemu_put_be32(f, se->version_id);
> 
> -        ret = se->save_live_state(f, QEMU_VM_SECTION_START, se->opaque);
> +        ret = se->ops->save_live_state(f, QEMU_VM_SECTION_START, se->opaque);
>          if (ret < 0) {
>              qemu_savevm_state_cancel(f);
>              return ret;
> @@ -1623,9 +1619,9 @@ int qemu_savevm_state_iterate(QEMUFile *f)
>      int ret = 1;
> 
>      QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> -        if (se->save_live_state == NULL)
> +        if (!se->ops || !se->ops->save_live_state) {
>              continue;
> -
> +        }
>          if (qemu_file_rate_limit(f)) {
>              return 0;
>          }
> @@ -1634,7 +1630,7 @@ int qemu_savevm_state_iterate(QEMUFile *f)
>          qemu_put_byte(f, QEMU_VM_SECTION_PART);
>          qemu_put_be32(f, se->section_id);
> 
> -        ret = se->save_live_state(f, QEMU_VM_SECTION_PART, se->opaque);
> +        ret = se->ops->save_live_state(f, QEMU_VM_SECTION_PART, se->opaque);
>          trace_savevm_section_end(se->section_id);
> 
>          if (ret <= 0) {
> @@ -1663,15 +1659,15 @@ int qemu_savevm_state_complete(QEMUFile *f)
>      cpu_synchronize_all_states();
> 
>      QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> -        if (se->save_live_state == NULL)
> +        if (!se->ops || !se->ops->save_live_state) {
>              continue;
> -
> +        }
>          trace_savevm_section_start();
>          /* Section type */
>          qemu_put_byte(f, QEMU_VM_SECTION_END);
>          qemu_put_be32(f, se->section_id);
> 
> -        ret = se->save_live_state(f, QEMU_VM_SECTION_END, se->opaque);
> +        ret = se->ops->save_live_state(f, QEMU_VM_SECTION_END, se->opaque);
>          trace_savevm_section_end(se->section_id);
>          if (ret < 0) {
>              return ret;
> @@ -1681,9 +1677,9 @@ int qemu_savevm_state_complete(QEMUFile *f)
>      QTAILQ_FOREACH(se, &savevm_handlers, entry) {
>          int len;
> 
> -     if (se->save_state == NULL && se->vmsd == NULL)
> +        if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
>           continue;
> -
> +        }
>          trace_savevm_section_start();
>          /* Section type */
>          qemu_put_byte(f, QEMU_VM_SECTION_FULL);
> @@ -1711,8 +1707,8 @@ void qemu_savevm_state_cancel(QEMUFile *f)
>      SaveStateEntry *se;
> 
>      QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> -        if (se->save_live_state) {
> -            se->save_live_state(f, -1, se->opaque);
> +        if (se->ops && se->ops->save_live_state) {
> +            se->ops->save_live_state(f, -1, se->opaque);
>          }
>      }
>  }
> @@ -1765,7 +1761,7 @@ static int qemu_save_device_state(QEMUFile *f)
>          if (se->is_ram) {
>              continue;
>          }
> -        if (se->save_state == NULL && se->vmsd == NULL) {
> +        if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
>              continue;
>          }
> 
> diff --git a/vmstate.h b/vmstate.h
> index 5af45e0..909af69 100644
> --- a/vmstate.h
> +++ b/vmstate.h
> @@ -31,6 +31,13 @@ typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>  typedef int SaveLiveStateHandler(QEMUFile *f, int stage, void *opaque);
>  typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
> 
> +typedef struct SaveVMHandlers {
> +    SaveSetParamsHandler *set_params;
> +    SaveStateHandler *save_state;
> +    SaveLiveStateHandler *save_live_state;
> +    LoadStateHandler *load_state;
> +} SaveVMHandlers;
> +
>  int register_savevm(DeviceState *dev,
>                      const char *idstr,
>                      int instance_id,
> 

Reviewed-by: Orit Wasserman <address@hidden>