Everything else assumes that we always load a device from its own
savevm handler.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index d96209b..2d1d4bc 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -746,13 +746,13 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
     }
 }
 
-static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
+static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
 {
     trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
     if (!se->vmsd) {         /* Old style */
-        return se->ops->load_state(f, se->opaque, version_id);
+        return se->ops->load_state(f, se->opaque, se->load_version_id);
     }
-    return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
+    return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
 }
 
 static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
@@ -1882,7 +1882,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
         return -EINVAL;
     }
 
-    ret = vmstate_load(f, se, se->load_version_id);
+    ret = vmstate_load(f, se);
     if (ret < 0) {
         error_report("error while loading state for instance 0x%x of"
                      " device '%s'", instance_id, idstr);
@@ -1915,7 +1915,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
         return -EINVAL;
     }
 
-    ret = vmstate_load(f, se, se->load_version_id);
+    ret = vmstate_load(f, se);
     if (ret < 0) {
         error_report("error while loading state section id %d(%s)",
                      section_id, se->idstr);
-- 
2.9.4

* Juan Quintela (quintela@redhat.com) wrote:
> Everything else assumes that we always load a device from its own
> savevm handler.

Except for the title (it's the version_id you're removing not
section_id) but now I think this series is ok because you're always
using
the load_* stuff.

I guess in manyways the LoadStateEntry list made some sense; it was
harder to accidentally use version_id rather than load_version_id if
it was on a different pointer.

But generally now for the series:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


Dave

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index d96209b..2d1d4bc 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -746,13 +746,13 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
>      }
>  }
>  
> -static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
> +static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
>  {
>      trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
>      if (!se->vmsd) {         /* Old style */
> -        return se->ops->load_state(f, se->opaque, version_id);
> +        return se->ops->load_state(f, se->opaque, se->load_version_id);
>      }
> -    return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
> +    return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
>  }
>  
>  static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
> @@ -1882,7 +1882,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
>          return -EINVAL;
>      }
>  
> -    ret = vmstate_load(f, se, se->load_version_id);
> +    ret = vmstate_load(f, se);
>      if (ret < 0) {
>          error_report("error while loading state for instance 0x%x of"
>                       " device '%s'", instance_id, idstr);
> @@ -1915,7 +1915,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
>          return -EINVAL;
>      }
>  
> -    ret = vmstate_load(f, se, se->load_version_id);
> +    ret = vmstate_load(f, se);
>      if (ret < 0) {
>          error_report("error while loading state section id %d(%s)",
>                       section_id, se->idstr);
> -- 
> 2.9.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Wed, May 31, 2017 at 12:39:25PM +0100, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
> > Everything else assumes that we always load a device from its own
> > savevm handler.
> 
> Except for the title (it's the version_id you're removing not
> section_id) but now I think this series is ok because you're always
> using
> the load_* stuff.
> 
> I guess in manyways the LoadStateEntry list made some sense; it was
> harder to accidentally use version_id rather than load_version_id if
> it was on a different pointer.

Indeed. After all it was:

struct LoadStateEntry {
    QLIST_ENTRY(LoadStateEntry) entry;
    SaveStateEntry *se;
    int section_id;
    int version_id;
};

And we moved the two variables into SaveStateEntry.

(I admit I didn't really notice the difference in v1...)

> 
> But generally now for the series:
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> 
> Dave
> 
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu