[PATCH v2 23/24] migration: Simplify qemu_save_device_state()

Peter Xu posted 24 patches 1 week, 5 days ago
Maintainers: Hailiang Zhang <zhanghailiang@xfusion.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Markus Armbruster <armbru@redhat.com>
[PATCH v2 23/24] migration: Simplify qemu_save_device_state()
Posted by Peter Xu 1 week, 5 days ago
This function is used by both COLO and Xen.  Simplify it with two changes:

- Remove checks on qemu_savevm_se_iterable(): this is not needed as
  vmstate_save() also checks for "save_state() || vmsd" instead.  Here,
  save_setup() (or say, iterable states) should be mutual exclusive to
  "save_state() || vmsd" [*].

- Remove migrate_error_propagate(): both of the users are not using live
  migration framework, but raw vmstate operations.  Error propagation is
  not needed for query-migrate persistence.

[*] One tricky user is VFIO, who provided _both_ save_state() and
save_setup().  However VFIO mustn't have been used in these paths or it
means both COLO and Xen have ignored VFIO data instead (that is,
qemu_savevm_se_iterable() will return true for VFIO). Hence, this change is
safe.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Paul Durrant <paul@xen.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 130b9764a7..b29272db3b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1897,13 +1897,8 @@ int qemu_save_device_state(QEMUFile *f)
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         int ret;
 
-        if (qemu_savevm_se_iterable(se)) {
-            continue;
-        }
         ret = vmstate_save(f, se, NULL, &local_err);
         if (ret) {
-            migrate_error_propagate(migrate_get_current(),
-                                    error_copy(local_err));
             error_report_err(local_err);
             return ret;
         }
-- 
2.50.1
Re: [PATCH v2 23/24] migration: Simplify qemu_save_device_state()
Posted by Fabiano Rosas 1 week, 4 days ago
Peter Xu <peterx@redhat.com> writes:

> This function is used by both COLO and Xen.  Simplify it with two changes:
>
> - Remove checks on qemu_savevm_se_iterable(): this is not needed as
>   vmstate_save() also checks for "save_state() || vmsd" instead.  Here,
>   save_setup() (or say, iterable states) should be mutual exclusive to
>   "save_state() || vmsd" [*].
>
> - Remove migrate_error_propagate(): both of the users are not using live
>   migration framework, but raw vmstate operations.  Error propagation is
>   not needed for query-migrate persistence.
>

s/not needed/only needed/

I can fixup if no repost.

> [*] One tricky user is VFIO, who provided _both_ save_state() and
> save_setup().  However VFIO mustn't have been used in these paths or it
> means both COLO and Xen have ignored VFIO data instead (that is,
> qemu_savevm_se_iterable() will return true for VFIO). Hence, this change is
> safe.
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Paul Durrant <paul@xen.org>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

> ---
>  migration/savevm.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 130b9764a7..b29272db3b 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1897,13 +1897,8 @@ int qemu_save_device_state(QEMUFile *f)
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          int ret;
>  
> -        if (qemu_savevm_se_iterable(se)) {
> -            continue;
> -        }
>          ret = vmstate_save(f, se, NULL, &local_err);
>          if (ret) {
> -            migrate_error_propagate(migrate_get_current(),
> -                                    error_copy(local_err));
>              error_report_err(local_err);
>              return ret;
>          }
Re: [PATCH v2 23/24] migration: Simplify qemu_save_device_state()
Posted by Peter Xu 1 week ago
On Wed, Jan 28, 2026 at 04:55:17PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > This function is used by both COLO and Xen.  Simplify it with two changes:
> >
> > - Remove checks on qemu_savevm_se_iterable(): this is not needed as
> >   vmstate_save() also checks for "save_state() || vmsd" instead.  Here,
> >   save_setup() (or say, iterable states) should be mutual exclusive to
> >   "save_state() || vmsd" [*].
> >
> > - Remove migrate_error_propagate(): both of the users are not using live
> >   migration framework, but raw vmstate operations.  Error propagation is
> >   not needed for query-migrate persistence.
> >
> 
> s/not needed/only needed/
> 
> I can fixup if no repost.

I'll fix both issues, including the one in the previous patch.

I'll wait for your review on the last patch to complete to see if I should
repost.

> 
> > [*] One tricky user is VFIO, who provided _both_ save_state() and
> > save_setup().  However VFIO mustn't have been used in these paths or it
> > means both COLO and Xen have ignored VFIO data instead (that is,
> > qemu_savevm_se_iterable() will return true for VFIO). Hence, this change is
> > safe.
> >
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Paul Durrant <paul@xen.org>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

Thanks.

-- 
Peter Xu