[PATCH 00/10] migration/colo: small cleanup series

[PATCH 00/10] migration/colo: small cleanup series

[Peter Xu]

CI: https://gitlab.com/peterx/qemu/-/pipelines/2277445319

While reading COLO in the past two days, I got a few small patches to clean
up here and there.  I also ran this with the COLO qtests [*] and it ran all fine.

Comments welcomed, thanks.

[*] https://lore.kernel.org/r/20260117-colo_unit_test_multifd-v2-0-ab521777fa51@web.de

Peter Xu (10):
  migration: Introduce qemu_savevm_send_* helpers
  migration: Use qemu_savevm_send_header() in qemu_save_device_state()
  migration: Remove one migration_in_colo_state() occurance
  migration/savevm: Remove SaveStateEntry.is_ram
  migration/colo: Unwrap qemu_savevm_live_state()
  migration/colo: Remove call to send switchover start event
  colo: Forbid VM resume during checkpointing
  migration/colo: Use the RAM iterable helper directly
  migration/colo: Move qemu_fflush() closer to its user for fb
  migration: Drop iterable_only in qemu_savevm_state_complete_precopy

 migration/savevm.h    |  4 +-
 migration/colo.c      |  9 ++---
 migration/migration.c |  2 +-
 migration/savevm.c    | 91 +++++++++++++++++++++----------------------
 monitor/qmp-cmds.c    |  3 ++
 5 files changed, 55 insertions(+), 54 deletions(-)

-- 
2.50.1

[PATCH 01/10] migration: Introduce qemu_savevm_send_* helpers

[Peter Xu]

Split qemu_savevm_state_header() into two parts.   This paves way for a
reuse elsewhere.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.h |  1 +
 migration/savevm.c | 58 +++++++++++++++++++++++++++-------------------
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/migration/savevm.h b/migration/savevm.h
index 125a2507b7..5d815af742 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -37,6 +37,7 @@ int qemu_savevm_state_prepare(Error **errp);
 int qemu_savevm_state_setup(QEMUFile *f, Error **errp);
 bool qemu_savevm_state_guest_unplug_pending(void);
 int qemu_savevm_state_resume_prepare(MigrationState *s);
+void qemu_savevm_send_header(QEMUFile *f);
 void qemu_savevm_state_header(QEMUFile *f);
 int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
 void qemu_savevm_state_cleanup(void);
diff --git a/migration/savevm.c b/migration/savevm.c
index 3dc812a7bb..e26656cca3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1282,38 +1282,48 @@ void qemu_savevm_non_migratable_list(strList **reasons)
     }
 }
 
-void qemu_savevm_state_header(QEMUFile *f)
+void qemu_savevm_send_header(QEMUFile *f)
 {
-    MigrationState *s = migrate_get_current();
-    JSONWriter *vmdesc = s->vmdesc;
-    Error *local_err = NULL;
-
     trace_savevm_state_header();
     qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
     qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+}
 
-    if (s->send_configuration) {
-        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
+static void qemu_savevm_send_configuration(MigrationState *s, QEMUFile *f)
+{
+    JSONWriter *vmdesc = s->vmdesc;
+    Error *local_err = NULL;
 
-        if (vmdesc) {
-            /*
-             * This starts the main json object and is paired with the
-             * json_writer_end_object in
-             * qemu_savevm_state_complete_precopy_non_iterable
-             */
-            json_writer_start_object(vmdesc, NULL);
-            json_writer_start_object(vmdesc, "configuration");
-        }
+    qemu_put_byte(f, QEMU_VM_CONFIGURATION);
 
-        vmstate_save_state(f, &vmstate_configuration, &savevm_state,
-                           vmdesc, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-        }
+    if (vmdesc) {
+        /*
+         * This starts the main json object and is paired with the
+         * json_writer_end_object in
+         * qemu_savevm_state_complete_precopy_non_iterable
+         */
+        json_writer_start_object(vmdesc, NULL);
+        json_writer_start_object(vmdesc, "configuration");
+    }
 
-        if (vmdesc) {
-            json_writer_end_object(vmdesc);
-        }
+    vmstate_save_state(f, &vmstate_configuration, &savevm_state,
+                       vmdesc, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+    }
+
+    if (vmdesc) {
+        json_writer_end_object(vmdesc);
+    }
+}
+
+void qemu_savevm_state_header(QEMUFile *f)
+{
+    MigrationState *s = migrate_get_current();
+
+    qemu_savevm_send_header(f);
+    if (s->send_configuration) {
+        qemu_savevm_send_configuration(s, f);
     }
 }
 
-- 
2.50.1

Re: [PATCH 01/10] migration: Introduce qemu_savevm_send_* helpers

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> Split qemu_savevm_state_header() into two parts.   This paves way for a
> reuse elsewhere.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h |  1 +
>  migration/savevm.c | 58 +++++++++++++++++++++++++++-------------------
>  2 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 125a2507b7..5d815af742 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -37,6 +37,7 @@ int qemu_savevm_state_prepare(Error **errp);
>  int qemu_savevm_state_setup(QEMUFile *f, Error **errp);
>  bool qemu_savevm_state_guest_unplug_pending(void);
>  int qemu_savevm_state_resume_prepare(MigrationState *s);
> +void qemu_savevm_send_header(QEMUFile *f);
>  void qemu_savevm_state_header(QEMUFile *f);
>  int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
>  void qemu_savevm_state_cleanup(void);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3dc812a7bb..e26656cca3 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1282,38 +1282,48 @@ void qemu_savevm_non_migratable_list(strList **reasons)
>      }
>  }
>  
> -void qemu_savevm_state_header(QEMUFile *f)
> +void qemu_savevm_send_header(QEMUFile *f)
>  {
> -    MigrationState *s = migrate_get_current();
> -    JSONWriter *vmdesc = s->vmdesc;
> -    Error *local_err = NULL;
> -
>      trace_savevm_state_header();
>      qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
>      qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> +}
>  
> -    if (s->send_configuration) {
> -        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> +static void qemu_savevm_send_configuration(MigrationState *s, QEMUFile *f)
> +{
> +    JSONWriter *vmdesc = s->vmdesc;
> +    Error *local_err = NULL;
>  
> -        if (vmdesc) {
> -            /*
> -             * This starts the main json object and is paired with the
> -             * json_writer_end_object in
> -             * qemu_savevm_state_complete_precopy_non_iterable
> -             */
> -            json_writer_start_object(vmdesc, NULL);
> -            json_writer_start_object(vmdesc, "configuration");
> -        }
> +    qemu_put_byte(f, QEMU_VM_CONFIGURATION);
>  
> -        vmstate_save_state(f, &vmstate_configuration, &savevm_state,
> -                           vmdesc, &local_err);
> -        if (local_err) {
> -            error_report_err(local_err);
> -        }
> +    if (vmdesc) {
> +        /*
> +         * This starts the main json object and is paired with the
> +         * json_writer_end_object in
> +         * qemu_savevm_state_complete_precopy_non_iterable
> +         */
> +        json_writer_start_object(vmdesc, NULL);
> +        json_writer_start_object(vmdesc, "configuration");
> +    }
>  
> -        if (vmdesc) {
> -            json_writer_end_object(vmdesc);
> -        }
> +    vmstate_save_state(f, &vmstate_configuration, &savevm_state,
> +                       vmdesc, &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +    }
> +
> +    if (vmdesc) {
> +        json_writer_end_object(vmdesc);
> +    }
> +}
> +
> +void qemu_savevm_state_header(QEMUFile *f)
> +{
> +    MigrationState *s = migrate_get_current();
> +
> +    qemu_savevm_send_header(f);
> +    if (s->send_configuration) {
> +        qemu_savevm_send_configuration(s, f);
>      }
>  }

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

[PATCH 02/10] migration: Use qemu_savevm_send_header() in qemu_save_device_state()

[Peter Xu]

Reduces duplication of the other path where we also send the same header.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index e26656cca3..64bf445c98 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1872,8 +1872,7 @@ int qemu_save_device_state(QEMUFile *f)
     SaveStateEntry *se;
 
     if (!migration_in_colo_state()) {
-        qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
-        qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+        qemu_savevm_send_header(f);
     }
     cpu_synchronize_all_states();
 
-- 
2.50.1

Re: [PATCH 02/10] migration: Use qemu_savevm_send_header() in qemu_save_device_state()

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> Reduces duplication of the other path where we also send the same header.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index e26656cca3..64bf445c98 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1872,8 +1872,7 @@ int qemu_save_device_state(QEMUFile *f)
>      SaveStateEntry *se;
>  
>      if (!migration_in_colo_state()) {
> -        qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
> -        qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> +        qemu_savevm_send_header(f);
>      }
>      cpu_synchronize_all_states();

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

[PATCH 03/10] migration: Remove one migration_in_colo_state() occurance

[Peter Xu]

Move the send header operation directly into Xen's QMP command, as COLO
doesn't need it.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 64bf445c98..61e873d90c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1871,9 +1871,6 @@ int qemu_save_device_state(QEMUFile *f)
     Error *local_err = NULL;
     SaveStateEntry *se;
 
-    if (!migration_in_colo_state()) {
-        qemu_savevm_send_header(f);
-    }
     cpu_synchronize_all_states();
 
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -3335,6 +3332,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
     qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
     f = qemu_file_new_output(QIO_CHANNEL(ioc));
     object_unref(OBJECT(ioc));
+    qemu_savevm_send_header(f);
     ret = qemu_save_device_state(f);
     if (ret < 0 || qemu_fclose(f) < 0) {
         error_setg(errp, "saving Xen device state failed");
-- 
2.50.1

Re: [PATCH 03/10] migration: Remove one migration_in_colo_state() occurance

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> Move the send header operation directly into Xen's QMP command, as COLO
> doesn't need it.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

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

We should make an effort to unify qemu_save_device_state and
qemu_savevm_state_complete_precopy_non_iterable, they're almost the
same.

> ---
>  migration/savevm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 64bf445c98..61e873d90c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1871,9 +1871,6 @@ int qemu_save_device_state(QEMUFile *f)
>      Error *local_err = NULL;
>      SaveStateEntry *se;
>  
> -    if (!migration_in_colo_state()) {
> -        qemu_savevm_send_header(f);
> -    }
>      cpu_synchronize_all_states();
>  
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> @@ -3335,6 +3332,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
>      qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
>      f = qemu_file_new_output(QIO_CHANNEL(ioc));
>      object_unref(OBJECT(ioc));
> +    qemu_savevm_send_header(f);
>      ret = qemu_save_device_state(f);
>      if (ret < 0 || qemu_fclose(f) < 0) {
>          error_setg(errp, "saving Xen device state failed");

Re: [PATCH 03/10] migration: Remove one migration_in_colo_state() occurance

[Peter Xu]

On Fri, Jan 23, 2026 at 10:32:27AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > Move the send header operation directly into Xen's QMP command, as COLO
> > doesn't need it.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

Thanks!

> 
> We should make an effort to unify qemu_save_device_state and
> qemu_savevm_state_complete_precopy_non_iterable, they're almost the
> same.

Let me give it a shot.

-- 
Peter Xu

[PATCH 04/10] migration/savevm: Remove SaveStateEntry.is_ram

[Peter Xu]

It's neither accurate nor necessary.  Use a proper helper to detect if it's
an iterable savevm state entry instead.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 61e873d90c..f1cd8c913d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -249,7 +249,6 @@ typedef struct SaveStateEntry {
     const VMStateDescription *vmsd;
     void *opaque;
     CompatEntry *compat;
-    int is_ram;
 } SaveStateEntry;
 
 typedef struct SaveState {
@@ -816,10 +815,6 @@ int register_savevm_live(const char *idstr,
     se->ops = ops;
     se->opaque = opaque;
     se->vmsd = NULL;
-    /* if this is a live_savem then set is_ram */
-    if (ops->save_setup != NULL) {
-        se->is_ram = 1;
-    }
 
     pstrcat(se->idstr, sizeof(se->idstr), idstr);
 
@@ -1866,6 +1861,12 @@ void qemu_savevm_live_state(QEMUFile *f)
     qemu_put_byte(f, QEMU_VM_EOF);
 }
 
+/* Is a save state entry iterable (e.g. RAM)? */
+static bool qemu_savevm_se_iterable(SaveStateEntry *se)
+{
+    return se->ops && se->ops->save_setup;
+}
+
 int qemu_save_device_state(QEMUFile *f)
 {
     Error *local_err = NULL;
@@ -1876,7 +1877,7 @@ int qemu_save_device_state(QEMUFile *f)
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         int ret;
 
-        if (se->is_ram) {
+        if (qemu_savevm_se_iterable(se)) {
             continue;
         }
         ret = vmstate_save(f, se, NULL, &local_err);
@@ -2648,7 +2649,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
     se->load_section_id = section_id;
 
     /* Validate if it is a device's state */
-    if (xen_enabled() && se->is_ram) {
+    if (xen_enabled() && qemu_savevm_se_iterable(se)) {
         error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
         return -EINVAL;
     }
-- 
2.50.1

Re: [PATCH 04/10] migration/savevm: Remove SaveStateEntry.is_ram

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> It's neither accurate nor necessary.  Use a proper helper to detect if it's
> an iterable savevm state entry instead.

Could you explain what exactly is_ram was supposed to mean?

>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 61e873d90c..f1cd8c913d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -249,7 +249,6 @@ typedef struct SaveStateEntry {
>      const VMStateDescription *vmsd;
>      void *opaque;
>      CompatEntry *compat;
> -    int is_ram;
>  } SaveStateEntry;
>  
>  typedef struct SaveState {
> @@ -816,10 +815,6 @@ int register_savevm_live(const char *idstr,
>      se->ops = ops;
>      se->opaque = opaque;
>      se->vmsd = NULL;
> -    /* if this is a live_savem then set is_ram */
> -    if (ops->save_setup != NULL) {
> -        se->is_ram = 1;
> -    }
>  
>      pstrcat(se->idstr, sizeof(se->idstr), idstr);
>  
> @@ -1866,6 +1861,12 @@ void qemu_savevm_live_state(QEMUFile *f)
>      qemu_put_byte(f, QEMU_VM_EOF);
>  }
>  
> +/* Is a save state entry iterable (e.g. RAM)? */
> +static bool qemu_savevm_se_iterable(SaveStateEntry *se)
> +{
> +    return se->ops && se->ops->save_setup;

So it could be iterable and not have .save_live_iterate?

Also, what's the deal with slirp? AIUI, it would be considered is_ram?

> +}
> +
>  int qemu_save_device_state(QEMUFile *f)
>  {
>      Error *local_err = NULL;
> @@ -1876,7 +1877,7 @@ int qemu_save_device_state(QEMUFile *f)
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          int ret;
>  
> -        if (se->is_ram) {
> +        if (qemu_savevm_se_iterable(se)) {

Hmm, qemu_savevm_state_setup has:

    if (!se->ops || !se->ops->save_setup) {
            continue;
    }

therefore:

    if (!qemu_savevm_se_iterable(se)) {
            continue;
    }

Maybe it would be more practical to put these in a separate list already
while registering.

>              continue;
>          }
>          ret = vmstate_save(f, se, NULL, &local_err);
> @@ -2648,7 +2649,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
>      se->load_section_id = section_id;
>  
>      /* Validate if it is a device's state */
> -    if (xen_enabled() && se->is_ram) {
> +    if (xen_enabled() && qemu_savevm_se_iterable(se)) {
>          error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
>          return -EINVAL;
>      }

Re: [PATCH 04/10] migration/savevm: Remove SaveStateEntry.is_ram

[Peter Xu]

On Fri, Jan 23, 2026 at 10:47:10AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > It's neither accurate nor necessary.  Use a proper helper to detect if it's
> > an iterable savevm state entry instead.
> 
> Could you explain what exactly is_ram was supposed to mean?

IIUC when introduced it was trying to capture the ram only, and that's also
why I said it's inaccurate, because ram is not the only thing that can
provide a save_setup() nowadays..

> 
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/savevm.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 61e873d90c..f1cd8c913d 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -249,7 +249,6 @@ typedef struct SaveStateEntry {
> >      const VMStateDescription *vmsd;
> >      void *opaque;
> >      CompatEntry *compat;
> > -    int is_ram;
> >  } SaveStateEntry;
> >  
> >  typedef struct SaveState {
> > @@ -816,10 +815,6 @@ int register_savevm_live(const char *idstr,
> >      se->ops = ops;
> >      se->opaque = opaque;
> >      se->vmsd = NULL;
> > -    /* if this is a live_savem then set is_ram */
> > -    if (ops->save_setup != NULL) {
> > -        se->is_ram = 1;
> > -    }
> >  
> >      pstrcat(se->idstr, sizeof(se->idstr), idstr);
> >  
> > @@ -1866,6 +1861,12 @@ void qemu_savevm_live_state(QEMUFile *f)
> >      qemu_put_byte(f, QEMU_VM_EOF);
> >  }
> >  
> > +/* Is a save state entry iterable (e.g. RAM)? */
> > +static bool qemu_savevm_se_iterable(SaveStateEntry *se)
> > +{
> > +    return se->ops && se->ops->save_setup;
> 
> So it could be iterable and not have .save_live_iterate?
> 
> Also, what's the deal with slirp? AIUI, it would be considered is_ram?

I guess no. Either in term of when it's introduced, or in theory of how I
understand this piece of code should work.. Because savevm_slirp_state
doesn't provide save_setup().

Note that this patch didn't really change the fact on how it works, even if
I _think_ this is broken..  I want to leave it for later, making sure this
patch (while doing the cleanup) doesn't introduce functional changes.

So I expect things to break if e.g. COLO is used with VFIO devices (that
supports migrations), where it also provides save_setup(), but actually
COLO will not properly sync VFIO states.  Likely nobody is using it anyway.

It's also the same to Xen's xen-save-devices-state QMP command.  It looks
to me Xen is playing some "split savevm" work here, however I don't know
whether it'll always workout these days when there're special devices that
provide save_setup().

> 
> > +}
> > +
> >  int qemu_save_device_state(QEMUFile *f)
> >  {
> >      Error *local_err = NULL;
> > @@ -1876,7 +1877,7 @@ int qemu_save_device_state(QEMUFile *f)
> >      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> >          int ret;
> >  
> > -        if (se->is_ram) {
> > +        if (qemu_savevm_se_iterable(se)) {
> 
> Hmm, qemu_savevm_state_setup has:
> 
>     if (!se->ops || !se->ops->save_setup) {
>             continue;
>     }
> 
> therefore:
> 
>     if (!qemu_savevm_se_iterable(se)) {
>             continue;
>     }
> 
> Maybe it would be more practical to put these in a separate list already
> while registering.

This is a question about performance, my gut feeling is looping over with
one queue would be fine for now, but even if not, it'll be further question
to ask after above (on correctness..).

So I plan to be focused on the cleanup part for now in this series..

> 
> >              continue;
> >          }
> >          ret = vmstate_save(f, se, NULL, &local_err);
> > @@ -2648,7 +2649,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
> >      se->load_section_id = section_id;
> >  
> >      /* Validate if it is a device's state */
> > -    if (xen_enabled() && se->is_ram) {
> > +    if (xen_enabled() && qemu_savevm_se_iterable(se)) {
> >          error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
> >          return -EINVAL;
> >      }
> 

-- 
Peter Xu

Re: [PATCH 04/10] migration/savevm: Remove SaveStateEntry.is_ram

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Fri, Jan 23, 2026 at 10:47:10AM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > It's neither accurate nor necessary.  Use a proper helper to detect if it's
>> > an iterable savevm state entry instead.
>> 
>> Could you explain what exactly is_ram was supposed to mean?
>
> IIUC when introduced it was trying to capture the ram only, and that's also
> why I said it's inaccurate, because ram is not the only thing that can
> provide a save_setup() nowadays..
>
>> 
>> >
>> > Signed-off-by: Peter Xu <peterx@redhat.com>
>> > ---
>> >  migration/savevm.c | 15 ++++++++-------
>> >  1 file changed, 8 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/migration/savevm.c b/migration/savevm.c
>> > index 61e873d90c..f1cd8c913d 100644
>> > --- a/migration/savevm.c
>> > +++ b/migration/savevm.c
>> > @@ -249,7 +249,6 @@ typedef struct SaveStateEntry {
>> >      const VMStateDescription *vmsd;
>> >      void *opaque;
>> >      CompatEntry *compat;
>> > -    int is_ram;
>> >  } SaveStateEntry;
>> >  
>> >  typedef struct SaveState {
>> > @@ -816,10 +815,6 @@ int register_savevm_live(const char *idstr,
>> >      se->ops = ops;
>> >      se->opaque = opaque;
>> >      se->vmsd = NULL;
>> > -    /* if this is a live_savem then set is_ram */
>> > -    if (ops->save_setup != NULL) {
>> > -        se->is_ram = 1;
>> > -    }
>> >  
>> >      pstrcat(se->idstr, sizeof(se->idstr), idstr);
>> >  
>> > @@ -1866,6 +1861,12 @@ void qemu_savevm_live_state(QEMUFile *f)
>> >      qemu_put_byte(f, QEMU_VM_EOF);
>> >  }
>> >  
>> > +/* Is a save state entry iterable (e.g. RAM)? */
>> > +static bool qemu_savevm_se_iterable(SaveStateEntry *se)
>> > +{
>> > +    return se->ops && se->ops->save_setup;
>> 
>> So it could be iterable and not have .save_live_iterate?
>> 
>> Also, what's the deal with slirp? AIUI, it would be considered is_ram?
>
> I guess no. Either in term of when it's introduced, or in theory of how I
> understand this piece of code should work.. Because savevm_slirp_state
> doesn't provide save_setup().
>

Oops, I misread save_state vs. save_setup.

> Note that this patch didn't really change the fact on how it works, even if
> I _think_ this is broken..  I want to leave it for later, making sure this
> patch (while doing the cleanup) doesn't introduce functional changes.
>

That's always fine.

> So I expect things to break if e.g. COLO is used with VFIO devices (that
> supports migrations), where it also provides save_setup(), but actually
> COLO will not properly sync VFIO states.  Likely nobody is using it anyway.
>
> It's also the same to Xen's xen-save-devices-state QMP command.  It looks
> to me Xen is playing some "split savevm" work here, however I don't know
> whether it'll always workout these days when there're special devices that
> provide save_setup().
>

Hm, ok. Let's leave it then.

>> 
>> > +}
>> > +
>> >  int qemu_save_device_state(QEMUFile *f)
>> >  {
>> >      Error *local_err = NULL;
>> > @@ -1876,7 +1877,7 @@ int qemu_save_device_state(QEMUFile *f)
>> >      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> >          int ret;
>> >  
>> > -        if (se->is_ram) {
>> > +        if (qemu_savevm_se_iterable(se)) {
>> 
>> Hmm, qemu_savevm_state_setup has:
>> 
>>     if (!se->ops || !se->ops->save_setup) {
>>             continue;
>>     }
>> 
>> therefore:
>> 
>>     if (!qemu_savevm_se_iterable(se)) {
>>             continue;
>>     }
>> 
>> Maybe it would be more practical to put these in a separate list already
>> while registering.
>
> This is a question about performance, my gut feeling is looping over with
> one queue would be fine for now, but even if not, it'll be further question
> to ask after above (on correctness..).
>
> So I plan to be focused on the cleanup part for now in this series..
>

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

[PATCH 05/10] migration/colo: Unwrap qemu_savevm_live_state()

[Peter Xu]

It's only used in COLO path and only contains two calls.  Unwrap the
function.  It paves way for further reduce special COLO paths on sync.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.h | 1 -
 migration/colo.c   | 3 ++-
 migration/savevm.c | 7 -------
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/migration/savevm.h b/migration/savevm.h
index 5d815af742..528607f09e 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -64,7 +64,6 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
                                            uint64_t *start_list,
                                            uint64_t *length_list);
 void qemu_savevm_send_colo_enable(QEMUFile *f);
-void qemu_savevm_live_state(QEMUFile *f);
 int qemu_save_device_state(QEMUFile *f);
 
 int qemu_loadvm_state(QEMUFile *f, Error **errp);
diff --git a/migration/colo.c b/migration/colo.c
index db783f6fa7..e05736ecf0 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -471,7 +471,8 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
      * TODO: We may need a timeout mechanism to prevent COLO process
      * to be blocked here.
      */
-    qemu_savevm_live_state(s->to_dst_file);
+    qemu_savevm_state_complete_precopy(s->to_dst_file, true);
+    qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
 
     qemu_fflush(fb);
 
diff --git a/migration/savevm.c b/migration/savevm.c
index f1cd8c913d..529cf310e0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1854,13 +1854,6 @@ cleanup:
     return ret;
 }
 
-void qemu_savevm_live_state(QEMUFile *f)
-{
-    /* save QEMU_VM_SECTION_END section */
-    qemu_savevm_state_complete_precopy(f, true);
-    qemu_put_byte(f, QEMU_VM_EOF);
-}
-
 /* Is a save state entry iterable (e.g. RAM)? */
 static bool qemu_savevm_se_iterable(SaveStateEntry *se)
 {
-- 
2.50.1

Re: [PATCH 05/10] migration/colo: Unwrap qemu_savevm_live_state()

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> It's only used in COLO path and only contains two calls.  Unwrap the
> function.  It paves way for further reduce special COLO paths on sync.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h | 1 -
>  migration/colo.c   | 3 ++-
>  migration/savevm.c | 7 -------
>  3 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 5d815af742..528607f09e 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -64,7 +64,6 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>                                             uint64_t *start_list,
>                                             uint64_t *length_list);
>  void qemu_savevm_send_colo_enable(QEMUFile *f);
> -void qemu_savevm_live_state(QEMUFile *f);
>  int qemu_save_device_state(QEMUFile *f);
>  
>  int qemu_loadvm_state(QEMUFile *f, Error **errp);
> diff --git a/migration/colo.c b/migration/colo.c
> index db783f6fa7..e05736ecf0 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -471,7 +471,8 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>       * TODO: We may need a timeout mechanism to prevent COLO process
>       * to be blocked here.
>       */
> -    qemu_savevm_live_state(s->to_dst_file);
> +    qemu_savevm_state_complete_precopy(s->to_dst_file, true);

Could maybe drop this complete from the name eventually.

> +    qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
>  
>      qemu_fflush(fb);
>  
> diff --git a/migration/savevm.c b/migration/savevm.c
> index f1cd8c913d..529cf310e0 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1854,13 +1854,6 @@ cleanup:
>      return ret;
>  }
>  
> -void qemu_savevm_live_state(QEMUFile *f)
> -{
> -    /* save QEMU_VM_SECTION_END section */
> -    qemu_savevm_state_complete_precopy(f, true);
> -    qemu_put_byte(f, QEMU_VM_EOF);
> -}
> -
>  /* Is a save state entry iterable (e.g. RAM)? */
>  static bool qemu_savevm_se_iterable(SaveStateEntry *se)
>  {

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

Re: [PATCH 05/10] migration/colo: Unwrap qemu_savevm_live_state()

[Peter Xu]

On Fri, Jan 23, 2026 at 10:48:53AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > It's only used in COLO path and only contains two calls.  Unwrap the
> > function.  It paves way for further reduce special COLO paths on sync.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/savevm.h | 1 -
> >  migration/colo.c   | 3 ++-
> >  migration/savevm.c | 7 -------
> >  3 files changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/migration/savevm.h b/migration/savevm.h
> > index 5d815af742..528607f09e 100644
> > --- a/migration/savevm.h
> > +++ b/migration/savevm.h
> > @@ -64,7 +64,6 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
> >                                             uint64_t *start_list,
> >                                             uint64_t *length_list);
> >  void qemu_savevm_send_colo_enable(QEMUFile *f);
> > -void qemu_savevm_live_state(QEMUFile *f);
> >  int qemu_save_device_state(QEMUFile *f);
> >  
> >  int qemu_loadvm_state(QEMUFile *f, Error **errp);
> > diff --git a/migration/colo.c b/migration/colo.c
> > index db783f6fa7..e05736ecf0 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -471,7 +471,8 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
> >       * TODO: We may need a timeout mechanism to prevent COLO process
> >       * to be blocked here.
> >       */
> > -    qemu_savevm_live_state(s->to_dst_file);
> > +    qemu_savevm_state_complete_precopy(s->to_dst_file, true);
> 
> Could maybe drop this complete from the name eventually.

Maybe it's good to keep it?  As it describes it's in completion phase.  For
instance, for iterable devices we invoke different hooks when it's
completing (save_complete()) v.s. iterating (save_iterate()).

> 
> > +    qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
> >  
> >      qemu_fflush(fb);
> >  
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index f1cd8c913d..529cf310e0 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -1854,13 +1854,6 @@ cleanup:
> >      return ret;
> >  }
> >  
> > -void qemu_savevm_live_state(QEMUFile *f)
> > -{
> > -    /* save QEMU_VM_SECTION_END section */
> > -    qemu_savevm_state_complete_precopy(f, true);
> > -    qemu_put_byte(f, QEMU_VM_EOF);
> > -}
> > -
> >  /* Is a save state entry iterable (e.g. RAM)? */
> >  static bool qemu_savevm_se_iterable(SaveStateEntry *se)
> >  {
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

I took this. :)

-- 
Peter Xu

[PATCH 06/10] migration/colo: Remove call to send switchover start event

[Peter Xu]

COLO (in case of periodically checkpointing) already have switchover
happened before hand.  This switchover_start feature never applies to COLO.
Remove it.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/colo.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index e05736ecf0..c344943173 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -453,8 +453,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
         goto out;
     }
 
-    qemu_savevm_maybe_send_switchover_start(s->to_dst_file);
-
     /* Note: device state is saved into buffer */
     ret = qemu_save_device_state(fb);
 
-- 
2.50.1

Re: [PATCH 06/10] migration/colo: Remove call to send switchover start event

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> COLO (in case of periodically checkpointing) already have switchover
> happened before hand.  This switchover_start feature never applies to COLO.
> Remove it.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/colo.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index e05736ecf0..c344943173 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -453,8 +453,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>          goto out;
>      }
>  
> -    qemu_savevm_maybe_send_switchover_start(s->to_dst_file);
> -
>      /* Note: device state is saved into buffer */
>      ret = qemu_save_device_state(fb);

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

[PATCH 07/10] colo: Forbid VM resume during checkpointing

[Peter Xu]

COLO will stop the VM during each checkpoint on either PVM or SVM.

Accidentally resuming the VM during the window might be fatal because it
may cause the RAM and devices state to misalign, corrupting the checkpoint.

Hence forbid VM resume during the process.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 monitor/qmp-cmds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 1ca44fbd72..0c409c27dc 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -84,6 +84,9 @@ void qmp_cont(Error **errp)
     } else if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
         error_setg(errp, "Migration is not finalized yet");
         return;
+    } else if (runstate_check(RUN_STATE_COLO)) {
+        error_setg(errp, "COLO checkpoint in progress");
+        return;
     }
 
     for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
-- 
2.50.1

Re: [PATCH 07/10] colo: Forbid VM resume during checkpointing

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> COLO will stop the VM during each checkpoint on either PVM or SVM.
>
> Accidentally resuming the VM during the window might be fatal because it
> may cause the RAM and devices state to misalign, corrupting the checkpoint.
>
> Hence forbid VM resume during the process.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  monitor/qmp-cmds.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 1ca44fbd72..0c409c27dc 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -84,6 +84,9 @@ void qmp_cont(Error **errp)
>      } else if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
>          error_setg(errp, "Migration is not finalized yet");
>          return;
> +    } else if (runstate_check(RUN_STATE_COLO)) {
> +        error_setg(errp, "COLO checkpoint in progress");
> +        return;
>      }
>  
>      for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {

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

[PATCH 08/10] migration/colo: Use the RAM iterable helper directly

[Peter Xu]

qemu_savevm_state_complete_precopy() has a weird parameter called
"iterable_only".  It's needed because COLO saves device states in advance.

To make dropping that weird parameter easier, let COLO directly use the RAM
iterator helper instead, which should make the code easier to read too.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/colo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/colo.c b/migration/colo.c
index c344943173..f92803dd29 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -469,7 +469,7 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
      * TODO: We may need a timeout mechanism to prevent COLO process
      * to be blocked here.
      */
-    qemu_savevm_state_complete_precopy(s->to_dst_file, true);
+    qemu_savevm_state_complete_precopy_iterable(s->to_dst_file, false);
     qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
 
     qemu_fflush(fb);
-- 
2.50.1

Re: [PATCH 08/10] migration/colo: Use the RAM iterable helper directly

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> qemu_savevm_state_complete_precopy() has a weird parameter called
> "iterable_only".  It's needed because COLO saves device states in advance.
>
> To make dropping that weird parameter easier, let COLO directly use the RAM
> iterator helper instead, which should make the code easier to read too.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/colo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index c344943173..f92803dd29 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -469,7 +469,7 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>       * TODO: We may need a timeout mechanism to prevent COLO process
>       * to be blocked here.
>       */
> -    qemu_savevm_state_complete_precopy(s->to_dst_file, true);
> +    qemu_savevm_state_complete_precopy_iterable(s->to_dst_file, false);
>      qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
>  
>      qemu_fflush(fb);

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

[PATCH 09/10] migration/colo: Move qemu_fflush() closer to its user for fb

[Peter Xu]

COLO caches all device states in a buffer channel `fb'.  Flush it right
before pushing the buffer to make sure everything needed will be flushed,
add a comment explaning it.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/colo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index f92803dd29..e2307f1907 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -472,8 +472,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
     qemu_savevm_state_complete_precopy_iterable(s->to_dst_file, false);
     qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
 
-    qemu_fflush(fb);
-
     /*
      * We need the size of the VMstate data in Secondary side,
      * With which we can decide how much data should be read.
@@ -484,6 +482,8 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
         goto out;
     }
 
+    /* Flush to make sure everything lands bioc->data */
+    qemu_fflush(fb);
     qemu_put_buffer(s->to_dst_file, bioc->data, bioc->usage);
     ret = qemu_fflush(s->to_dst_file);
     if (ret < 0) {
-- 
2.50.1

Re: [PATCH 09/10] migration/colo: Move qemu_fflush() closer to its user for fb

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> COLO caches all device states in a buffer channel `fb'.  Flush it right
> before pushing the buffer to make sure everything needed will be flushed,
> add a comment explaning it.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/colo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index f92803dd29..e2307f1907 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -472,8 +472,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>      qemu_savevm_state_complete_precopy_iterable(s->to_dst_file, false);
>      qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
>  
> -    qemu_fflush(fb);
> -
>      /*
>       * We need the size of the VMstate data in Secondary side,
>       * With which we can decide how much data should be read.
> @@ -484,6 +482,8 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>          goto out;
>      }
>  
> +    /* Flush to make sure everything lands bioc->data */
> +    qemu_fflush(fb);
>      qemu_put_buffer(s->to_dst_file, bioc->data, bioc->usage);
>      ret = qemu_fflush(s->to_dst_file);

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

Wait, aren't we wasting a memcpy here? Isn't this the same, but without
the copy of bioc->data to s->to_dst_file->buf:

qemu_fflush(fb);                                        // ensure bioc->data is populated
qemu_fflush(s->to_dst_file);                            // send buffered data from migration stream
qemu_put_buffer_async(s->to_dst_file, bioc->data, ...); // point s->to_dst_file->iov to bioc->data
qemu_fflush(s->to_dst_file);                            // send s->to_dst_file->iov contents

>      if (ret < 0) {

Re: [PATCH 09/10] migration/colo: Move qemu_fflush() closer to its user for fb

[Peter Xu]

On Fri, Jan 23, 2026 at 11:10:54AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > COLO caches all device states in a buffer channel `fb'.  Flush it right
> > before pushing the buffer to make sure everything needed will be flushed,
> > add a comment explaning it.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/colo.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index f92803dd29..e2307f1907 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -472,8 +472,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
> >      qemu_savevm_state_complete_precopy_iterable(s->to_dst_file, false);
> >      qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
> >  
> > -    qemu_fflush(fb);
> > -
> >      /*
> >       * We need the size of the VMstate data in Secondary side,
> >       * With which we can decide how much data should be read.
> > @@ -484,6 +482,8 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
> >          goto out;
> >      }
> >  
> > +    /* Flush to make sure everything lands bioc->data */
> > +    qemu_fflush(fb);
> >      qemu_put_buffer(s->to_dst_file, bioc->data, bioc->usage);
> >      ret = qemu_fflush(s->to_dst_file);
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> 
> Wait, aren't we wasting a memcpy here? Isn't this the same, but without
> the copy of bioc->data to s->to_dst_file->buf:
> 
> qemu_fflush(fb);                                        // ensure bioc->data is populated
> qemu_fflush(s->to_dst_file);                            // send buffered data from migration stream
> qemu_put_buffer_async(s->to_dst_file, bioc->data, ...); // point s->to_dst_file->iov to bioc->data
> qemu_fflush(s->to_dst_file);                            // send s->to_dst_file->iov contents

Good point!  Same to postcopy..

I quickly measured a small VM in postcopy path, async put can shrink the
time needed (plus the flush) from 91us to 38us.  Not a huge difference, but
I don't see why we don't do it either.  I'll test more and add those when I
repost.

-- 
Peter Xu

[PATCH 10/10] migration: Drop iterable_only in qemu_savevm_state_complete_precopy

[Peter Xu]

Now after removing the special case in COLO, we can drop this parameter.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.h    |  2 +-
 migration/migration.c |  2 +-
 migration/savevm.c    | 12 +++++-------
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/migration/savevm.h b/migration/savevm.h
index 528607f09e..ea01ca63ec 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -42,7 +42,7 @@ void qemu_savevm_state_header(QEMUFile *f);
 int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
 void qemu_savevm_state_cleanup(void);
 void qemu_savevm_state_complete_postcopy(QEMUFile *f);
-int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only);
+int qemu_savevm_state_complete_precopy(QEMUFile *f);
 void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
                                      uint64_t *can_postcopy);
 void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
diff --git a/migration/migration.c b/migration/migration.c
index 1bcde301f7..5d21d6dc11 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3018,7 +3018,7 @@ static int migration_completion_precopy(MigrationState *s)
         goto out_unlock;
     }
 
-    ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false);
+    ret = qemu_savevm_state_complete_precopy(s->to_dst_file);
 out_unlock:
     bql_unlock();
     return ret;
diff --git a/migration/savevm.c b/migration/savevm.c
index 529cf310e0..1bf1037ce1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1717,7 +1717,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
     return 0;
 }
 
-int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
+int qemu_savevm_state_complete_precopy(QEMUFile *f)
 {
     int ret;
 
@@ -1726,11 +1726,9 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
         return ret;
     }
 
-    if (!iterable_only) {
-        ret = qemu_savevm_state_complete_precopy_non_iterable(f, false);
-        if (ret) {
-            return ret;
-        }
+    ret = qemu_savevm_state_complete_precopy_non_iterable(f, false);
+    if (ret) {
+        return ret;
     }
 
     return qemu_fflush(f);
@@ -1831,7 +1829,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
     ret = qemu_file_get_error(f);
     if (ret == 0) {
         qemu_savevm_maybe_send_switchover_start(f);
-        qemu_savevm_state_complete_precopy(f, false);
+        qemu_savevm_state_complete_precopy(f);
         ret = qemu_file_get_error(f);
     }
     if (ret != 0) {
-- 
2.50.1

Re: [PATCH 10/10] migration: Drop iterable_only in qemu_savevm_state_complete_precopy

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> Now after removing the special case in COLO, we can drop this parameter.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h    |  2 +-
>  migration/migration.c |  2 +-
>  migration/savevm.c    | 12 +++++-------
>  3 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 528607f09e..ea01ca63ec 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -42,7 +42,7 @@ void qemu_savevm_state_header(QEMUFile *f);
>  int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
>  void qemu_savevm_state_cleanup(void);
>  void qemu_savevm_state_complete_postcopy(QEMUFile *f);
> -int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only);
> +int qemu_savevm_state_complete_precopy(QEMUFile *f);
>  void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
>                                       uint64_t *can_postcopy);
>  void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
> diff --git a/migration/migration.c b/migration/migration.c
> index 1bcde301f7..5d21d6dc11 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3018,7 +3018,7 @@ static int migration_completion_precopy(MigrationState *s)
>          goto out_unlock;
>      }
>  
> -    ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false);
> +    ret = qemu_savevm_state_complete_precopy(s->to_dst_file);
>  out_unlock:
>      bql_unlock();
>      return ret;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 529cf310e0..1bf1037ce1 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1717,7 +1717,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>      return 0;
>  }
>  
> -int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
> +int qemu_savevm_state_complete_precopy(QEMUFile *f)
>  {
>      int ret;
>  
> @@ -1726,11 +1726,9 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
>          return ret;
>      }
>  
> -    if (!iterable_only) {
> -        ret = qemu_savevm_state_complete_precopy_non_iterable(f, false);
> -        if (ret) {
> -            return ret;
> -        }
> +    ret = qemu_savevm_state_complete_precopy_non_iterable(f, false);
> +    if (ret) {
> +        return ret;
>      }
>  
>      return qemu_fflush(f);
> @@ -1831,7 +1829,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>      ret = qemu_file_get_error(f);
>      if (ret == 0) {
>          qemu_savevm_maybe_send_switchover_start(f);
> -        qemu_savevm_state_complete_precopy(f, false);
> +        qemu_savevm_state_complete_precopy(f);
>          ret = qemu_file_get_error(f);
>      }
>      if (ret != 0) {

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

Re: [PATCH 00/10] migration/colo: small cleanup series

[Lukas Straub]

[-- Attachment #1: Type: text/plain, Size: 2969 bytes --]

On Wed, 21 Jan 2026 17:33:25 -0500
Peter Xu <peterx@redhat.com> wrote:

> CI: https://gitlab.com/peterx/qemu/-/pipelines/2277445319
> 
> While reading COLO in the past two days, I got a few small patches to clean
> up here and there.  I also ran this with the COLO qtests [*] and it ran all fine.
> 
> Comments welcomed, thanks.
> 
> [*] https://lore.kernel.org/r/20260117-colo_unit_test_multifd-v2-0-ab521777fa51@web.de

Hmm,
It fails for me on top of current master, but works fine wthout this patchset:
# Running /x86_64/migration/colo/plain/primary_failover
# Using machine type: pc-i440fx-11.0
# starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-3157.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3157.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-i440fx-11.0, -name source,debug-threads=on -machine memory-backend=mig.mem -object memory-backend-ram,id=mig.mem,size=150M,share=off -serial file:/tmp/migration-test-OZFSJ3/src_serial -drive if=none,id=d0,file=/tmp/migration-test-OZFSJ3/bootsect,format=raw -device ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot  -accel qtest
# starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-3157.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3157.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-i440fx-11.0, -name target,debug-threads=on -machine memory-backend=mig.mem -object memory-backend-ram,id=mig.mem,size=150M,share=off -serial file:/tmp/migration-test-OZFSJ3/dest_serial -incoming tcp:127.0.0.1:0  -drive if=none,id=d0,file=/tmp/migration-test-OZFSJ3/bootsect,format=raw -device ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot  -accel qtest
qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:03.0/e1000': Loading VM subsection 'e1000/full_mac_state' in 'e1000' failed: -5: Failed to load e1000/full_mac_state state: stream error: -5

> 
> Peter Xu (10):
>   migration: Introduce qemu_savevm_send_* helpers
>   migration: Use qemu_savevm_send_header() in qemu_save_device_state()
>   migration: Remove one migration_in_colo_state() occurance
>   migration/savevm: Remove SaveStateEntry.is_ram
>   migration/colo: Unwrap qemu_savevm_live_state()
>   migration/colo: Remove call to send switchover start event
>   colo: Forbid VM resume during checkpointing
>   migration/colo: Use the RAM iterable helper directly
>   migration/colo: Move qemu_fflush() closer to its user for fb
>   migration: Drop iterable_only in qemu_savevm_state_complete_precopy
> 
>  migration/savevm.h    |  4 +-
>  migration/colo.c      |  9 ++---
>  migration/migration.c |  2 +-
>  migration/savevm.c    | 91 +++++++++++++++++++++----------------------
>  monitor/qmp-cmds.c    |  3 ++
>  5 files changed, 55 insertions(+), 54 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 00/10] migration/colo: small cleanup series

[Peter Xu]

On Sun, Jan 25, 2026 at 08:29:15PM +0100, Lukas Straub wrote:
> On Wed, 21 Jan 2026 17:33:25 -0500
> Peter Xu <peterx@redhat.com> wrote:
> 
> > CI: https://gitlab.com/peterx/qemu/-/pipelines/2277445319
> > 
> > While reading COLO in the past two days, I got a few small patches to clean
> > up here and there.  I also ran this with the COLO qtests [*] and it ran all fine.
> > 
> > Comments welcomed, thanks.
> > 
> > [*] https://lore.kernel.org/r/20260117-colo_unit_test_multifd-v2-0-ab521777fa51@web.de
> 
> Hmm,
> It fails for me on top of current master, but works fine wthout this patchset:
> # Running /x86_64/migration/colo/plain/primary_failover
> # Using machine type: pc-i440fx-11.0
> # starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-3157.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3157.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-i440fx-11.0, -name source,debug-threads=on -machine memory-backend=mig.mem -object memory-backend-ram,id=mig.mem,size=150M,share=off -serial file:/tmp/migration-test-OZFSJ3/src_serial -drive if=none,id=d0,file=/tmp/migration-test-OZFSJ3/bootsect,format=raw -device ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot  -accel qtest
> # starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-3157.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3157.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -run-with exit-with-parent=on -accel kvm -accel tcg -machine pc-i440fx-11.0, -name target,debug-threads=on -machine memory-backend=mig.mem -object memory-backend-ram,id=mig.mem,size=150M,share=off -serial file:/tmp/migration-test-OZFSJ3/dest_serial -incoming tcp:127.0.0.1:0  -drive if=none,id=d0,file=/tmp/migration-test-OZFSJ3/bootsect,format=raw -device ide-hd,drive=d0,secs=1,cyls=1,heads=1 -snapshot  -accel qtest
> qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:03.0/e1000': Loading VM subsection 'e1000/full_mac_state' in 'e1000' failed: -5: Failed to load e1000/full_mac_state state: stream error: -5

Somehow I didn't hit it previously, but at least when using your latest
patches it hangs for me (rather than failing..).

Patch 9 is broken, because I overlooked COLO_MESSAGE_VMSTATE_SIZE also
needs to reference bioc->usage.

I'll fix patch 9 when repost.  Thanks for the report.

--
Peter Xu