Skip to contentSkip to conversation

migration: more bool+errp APIs

PATCH Applied Vladimir Sementsov-Ogievskiy 10 participants 41 messages Mar 4 – Apr 28 Rust

lore
Share & tools
lore
#0 lore raw

v3 cover migration: more bool+errp APIs

v3:
01,02: add r-bs by Fabiano and Peter
03: improve comment, add r-b by Peter
04: add r-bs by Fabiano and Peter
05: new, replacement for dropped "[PATCH v2 05/16] migration: vmstate_save/load_state(): stop tracing errors" 06: rebase on kept trace-points, keep r-b by Peter
07-09: add r-b by Peter
10: rebase on kept trace-points, keep r-b by Peter
11-12: add r-b by Peter
13: rebase ion kept traces and logic around trace_vmstate_load_field_error() already refactored, fix indent, drop r-b 14: add r-b by Peter
15: add r-b by Peter, use "else { if () {} }"
16-17: new
18: rebased on dropped err_hint, drop r-b

Vladimir Sementsov-Ogievskiy (18):
migration: vmstate_save_state_v: fix double error_setg
migration: make vmstate_save_state_v() static
migration: make .post_save() a void function
migration: vmstate_load_state(): add some newlines
migration: vmstate_save/load_state(): refactor tracing errors
migration: factor out vmstate_pre_save() from vmstate_save_state()
migration: factor out vmstate_save_field() from vmstate_save_state()
migration: factor out vmstate_pre_load() from vmstate_load_state()
migration: factor out vmstate_load_field() from vmstate_load_state()
migration: factor out vmstate_post_load() from vmstate_load_state()
migration: convert vmstate_subsection_save/load functions to bool
migration: VMStateInfo: introduce new handlers with errp
migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd() migration/cpr: move to new migration APIs migration/savevm: move to new migration APIs hw/s390x/css: drop use of .err_hint for vmstate
migration: drop VMStateField.err_hint migration/vmstate-types: move to new migration APIs

 docs/devel/migration/main.rst |   2 +-
 hw/block/fdc.c                |   2 +-
 hw/display/qxl.c              |   4 +-
 hw/display/vga.c              |   2 +-
 hw/display/virtio-gpu.c       |   2 +-
 hw/display/vmware_vga.c       |   2 +-
 hw/i386/vmmouse.c             |   2 +-
 hw/ide/ahci.c                 |   2 +-
 hw/intc/openpic.c             |   2 +-
 hw/intc/spapr_xive.c          |   2 +-
 hw/intc/xics.c                |   2 +-
 hw/intc/xive.c                |   2 +-
 hw/nvram/eeprom93xx.c         |   2 +-
 hw/pci/pci.c                  |   2 +-
 hw/pci/pcie_aer.c             |   2 +-
 hw/ppc/spapr_iommu.c          |   2 +-
 hw/ppc/spapr_pci.c            |   7 +-
 hw/ppc/spapr_vio.c            |   4 +-
 hw/s390x/css.c                |  15 +-
 hw/usb/hcd-uhci.c             |   2 +-
 include/migration/cpr.h       |   2 +-
 include/migration/vmstate.h   |  73 ++--
 migration/cpr.c               |  22 +-
 migration/savevm.c            | 108 +++---
 migration/trace-events        |  30 +-
 migration/vmstate-types.c     | 642 +++++++++++++++++-----------------
 migration/vmstate.c           | 354 +++++++++++--------
 target/arm/machine.c          |   4 +-
 target/ppc/machine.c          |   6 +-
 29 files changed, 691 insertions(+), 612 deletions(-)
Signature · 1 line
2.52.0
#1 lore raw

PATCH v3 1/18 migration: vmstate_save_state_v: fix double error_setg

We may call error_setg twice on same errp if inner vmstate_save_state_v() or vmstate_save_state() call fails. Next we will crash on assertion in error_setv().

Fixes 848a0503422d043 "migration: Update error description outside migration.c"
---
 migration/vmstate.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
migration/vmstate.c +5 -2
index 4d28364f7ba..fccd030dfd1 100644
@@ -539,6 +539,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                 } else {
                     ret = inner_field->info->put(f, curr_elem, size,
                                                  inner_field, vmdesc_loop);
+                    if (ret < 0) {
+                        error_setg(errp, "put failed");
+                    }
                 }
 
                 written_bytes = qemu_file_transferred(f) - old_offset;
@@ -551,8 +554,8 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                 }
 
                 if (ret) {
-                    error_setg(errp, "Save of field %s/%s failed",
-                                vmsd->name, field->name);
+                    error_prepend(errp, "Save of field %s/%s failed: ",
+                                  vmsd->name, field->name);
                     if (vmsd->post_save) {
                         vmsd->post_save(opaque);
                     }
Signature · 1 line
2.52.0
#2 lore raw

Re: [PATCH v3 01/18] migration: vmstate_save_state_v: fix double error_setg

Quoted 8 lines · Vladimir Sementsov-Ogievskiy

We may call error_setg twice on same errp if inner vmstate_save_state_v() or vmstate_save_state() call fails. Next we will crash on assertion in error_setv().

Fixes 848a0503422d043 "migration: Update error description outside migration.c"

This feels like a qemu-stable material.

I'm picking it up for the current active stable series. Please let me know if I shouldn't.

Thanks,

/mjt

#3 lore raw

PATCH v3 2/18 migration: make vmstate_save_state_v() static

In reply to Vladimir Sementsov-Ogievskiy

It's used only in vmstate.c.

---
 include/migration/vmstate.h |  3 ---
 migration/vmstate.c         | 19 ++++++++++---------
 2 files changed, 10 insertions(+), 12 deletions(-)
include/migration/vmstate.h +0 -3
index 89f9f49d20a..3695afd483f 100644
@@ -1234,9 +1234,6 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id, Error **errp);
 int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, JSONWriter *vmdesc, Error **errp);
-int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
-                         void *opaque, JSONWriter *vmdesc,
-                         int version_id, Error **errp);
 
 bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque);
 
migration/vmstate.c +10 -9
index fccd030dfd1..651c3fe0115 100644
@@ -420,15 +420,9 @@ bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque)
     return true;
 }
 
-
-int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
-                       void *opaque, JSONWriter *vmdesc_id, Error **errp)
-{
-    return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id, errp);
-}
-
-int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
-                         void *opaque, JSONWriter *vmdesc, int version_id, Error **errp)
+static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
+                                void *opaque, JSONWriter *vmdesc,
+                                int version_id, Error **errp)
 {
     ERRP_GUARD();
     int ret = 0;
@@ -608,6 +602,13 @@ vmstate_get_subsection(const VMStateDescription * const *sub,
     return NULL;
 }
 
+int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
+                       void *opaque, JSONWriter *vmdesc_id, Error **errp)
+{
+    return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id,
+                                errp);
+}
+
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque, Error **errp)
 {
Signature · 1 line
2.52.0
#4 lore raw

PATCH v3 3/18 migration: make .post_save() a void function

In reply to Vladimir Sementsov-Ogievskiy

All other handlers now have _errp() variants. Should we go this way for .post_save()? Actually it's rather strange, when the vmstate do successful preparations in .pre_save(), then successfully save all sections and subsections, end then fail when all the state is successfully transferred to the target.

Happily, we have only three .post_save() realizations, all always successful. Let's make this a rule.

Also note, that we call .post_save() in two places, and handle its (theoretical) failure inconsistently. Fix that too.

---
 docs/devel/migration/main.rst |  2 +-
 hw/ppc/spapr_pci.c            |  3 +--
 include/migration/vmstate.h   |  8 +++++++-
 migration/savevm.c            |  3 +--
 migration/vmstate.c           | 12 +++---------
 target/arm/machine.c          |  4 +---
 6 files changed, 14 insertions(+), 18 deletions(-)
docs/devel/migration/main.rst +1 -1
index 234d280249a..2de70507640 100644
@@ -439,7 +439,7 @@ The functions to do that are inside a vmstate definition, and are called:
 
   This function is called before we save the state of one device.
 
-- ``int (*post_save)(void *opaque);``
+- ``void (*post_save)(void *opaque);``
 
   This function is called after we save the state of one device
   (even upon failure, unless the call to pre_save returned an error).
hw/ppc/spapr_pci.c +1 -2
index ea998bdff15..1dc3b02659f 100644
@@ -2093,14 +2093,13 @@ static int spapr_pci_pre_save(void *opaque)
     return 0;
 }
 
-static int spapr_pci_post_save(void *opaque)
+static void spapr_pci_post_save(void *opaque)
 {
     SpaprPhbState *sphb = opaque;
 
     g_free(sphb->msi_devs);
     sphb->msi_devs = NULL;
     sphb->msi_devs_num = 0;
-    return 0;
 }
 
 static int spapr_pci_post_load(void *opaque, int version_id)
include/migration/vmstate.h +7 -1
index 3695afd483f..85838a49aee 100644
@@ -223,7 +223,13 @@ struct VMStateDescription {
     bool (*post_load_errp)(void *opaque, int version_id, Error **errp);
     int (*pre_save)(void *opaque);
     bool (*pre_save_errp)(void *opaque, Error **errp);
-    int (*post_save)(void *opaque);
+
+    /*
+     * Unless .pre_save() fails, .post_save() is called after saving
+     * fields and subsections. It should not fail because at this
+     * point the state has potentially already been transferred.
+     */
+    void (*post_save)(void *opaque);
     bool (*needed)(void *opaque);
     bool (*dev_unplug_pending)(void *opaque);
 
migration/savevm.c +1 -2
index 3a16c467b25..c5236e71ba1 100644
@@ -321,14 +321,13 @@ static int configuration_pre_save(void *opaque)
     return 0;
 }
 
-static int configuration_post_save(void *opaque)
+static void configuration_post_save(void *opaque)
 {
     SaveState *state = opaque;
 
     g_free(state->capabilities);
     state->capabilities = NULL;
     state->caps_count = 0;
-    return 0;
 }
 
 static int configuration_pre_load(void *opaque)
migration/vmstate.c +3 -9
index 651c3fe0115..5111e7a141f 100644
@@ -550,10 +550,7 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                 if (ret) {
                     error_prepend(errp, "Save of field %s/%s failed: ",
                                   vmsd->name, field->name);
-                    if (vmsd->post_save) {
-                        vmsd->post_save(opaque);
-                    }
-                    return ret;
+                    goto out;
                 }
 
                 /* Compressed arrays only care about the first element */
@@ -578,12 +575,9 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
 
     ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
 
+out:
     if (vmsd->post_save) {
-        int ps_ret = vmsd->post_save(opaque);
-        if (!ret && ps_ret) {
-            ret = ps_ret;
-            error_setg(errp, "post-save failed: %s", vmsd->name);
-        }
+        vmsd->post_save(opaque);
     }
     return ret;
 }
target/arm/machine.c +1 -3
index bbaae344492..de810220e25 100644
@@ -993,15 +993,13 @@ static int cpu_pre_save(void *opaque)
     return 0;
 }
 
-static int cpu_post_save(void *opaque)
+static void cpu_post_save(void *opaque)
 {
     ARMCPU *cpu = opaque;
 
     if (!kvm_enabled()) {
         pmu_op_finish(&cpu->env);
     }
-
-    return 0;
 }
 
 static int cpu_pre_load(void *opaque)
Signature · 1 line
2.52.0
#5 lore raw

Re: [PATCH v3 03/18] migration: make .post_save() a void function

Quoted 62 lines · Vladimir Sementsov-Ogievskiy

All other handlers now have _errp() variants. Should we go this way for .post_save()? Actually it's rather strange, when the vmstate do successful preparations in .pre_save(), then successfully save all sections and subsections, end then fail when all the state is successfully transferred to the target.

Happily, we have only three .post_save() realizations, all always successful. Let's make this a rule.

Also note, that we call .post_save() in two places, and handle its (theoretical) failure inconsistently. Fix that too.

---
 docs/devel/migration/main.rst |  2 +-
 hw/ppc/spapr_pci.c            |  3 +--
 include/migration/vmstate.h   |  8 +++++++-
 migration/savevm.c            |  3 +--
 migration/vmstate.c           | 12 +++---------
 target/arm/machine.c          |  4 +---
 6 files changed, 14 insertions(+), 18 deletions(-)
docs/devel/migration/main.rst +1 -1
index 234d280249a..2de70507640 100644
@@ -439,7 +439,7 @@ The functions to do that are inside a vmstate definition, and are called:
 
   This function is called before we save the state of one device.
 
-- ``int (*post_save)(void *opaque);``
+- ``void (*post_save)(void *opaque);``
 
   This function is called after we save the state of one device
   (even upon failure, unless the call to pre_save returned an error).
hw/ppc/spapr_pci.c +1 -2
index ea998bdff15..1dc3b02659f 100644
@@ -2093,14 +2093,13 @@ static int spapr_pci_pre_save(void *opaque)
     return 0;
 }
 
-static int spapr_pci_post_save(void *opaque)
+static void spapr_pci_post_save(void *opaque)
 {
     SpaprPhbState *sphb = opaque;
 
     g_free(sphb->msi_devs);
     sphb->msi_devs = NULL;
     sphb->msi_devs_num = 0;
-    return 0;
 }
 
 static int spapr_pci_post_load(void *opaque, int version_id)
include/migration/vmstate.h +0 -0
index 3695afd483f..85838a49aee 100644
@@ -223,7 +223,13 @@ struct VMStateDescription {
     bool (*post_load_errp)(void *opaque, int version_id, Error **errp);

include/migration/vmstate.h struct VMStateDescription { · line 232

Quoted 10 lines
include/migration/vmstate.h +7 -1
     int (*pre_save)(void *opaque);
     bool (*pre_save_errp)(void *opaque, Error **errp);
-    int (*post_save)(void *opaque);
+
+    /*
+     * Unless .pre_save() fails, .post_save() is called after saving
+     * fields and subsections. It should not fail because at this
+     * point the state has potentially already been transferred.
+     */
+    void (*post_save)(void *opaque);

+CC Paolo, Zhao

This change breaks the rust build:
https://gitlab.com/farosas/qemu/-/jobs/13393119755

error[E0308]: mismatched types

     --> ../rust/migration/src/vmstate.rs:605:18
      |
  605 |             Some(vmstate_no_version_cb::<T, F>)
      |             ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
      |             |
      |             arguments to this enum variant are incorrect
      |
      = note: expected fn pointer `unsafe extern "C" fn(_) -> ()`
                    found fn item `unsafe extern "C" fn(_) -> i32 {vmstate_no_version_cb::<T, F, impl Into<Errno>>}`

Is something like the following diff the right fix? It'd be nice to get an ack so this series doesn't miss the freeze. (CI run:
https://gitlab.com/farosas/qemu/-/pipelines/2369688593)

-- >8 --
From 90a7b53d2b0d53692df1ef2cfe7607b13e4961d8 Mon Sep 17 00:00:00 2001
From: Fabiano Rosas <[email protected]>
Date: Fri, 6 Mar 2026 18:53:14 -0300
Subject: [PATCH] fixup! migration: make .post_save() a void function

---
 rust/migration/src/migratable.rs |  3 +--
 rust/migration/src/vmstate.rs    | 12 +++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)
rust/migration/src/migratable.rs +1 -2
index 7748aac2f2..7088c1b156 100644
@@ -406,10 +406,9 @@ fn pre_save(&self) -> Result<(), InvalidError> {
         Ok(())
     }
 
-    fn post_save(&self) -> Result<(), InvalidError> {
+    fn post_save(&self) {
         let state = unsafe { Box::from_raw(self.migration_state.replace(ptr::null_mut())) };
         drop(state);
-        Ok(())
     }
 
     fn pre_load(&self) -> Result<(), InvalidError> {
rust/migration/src/vmstate.rs +7 -5
index edc7c70265..f34a36f680 100644
@@ -492,6 +492,11 @@ fn from(_value: InvalidError) -> Errno {
     into_neg_errno(result)
 }
 
+unsafe extern "C" fn vmstate_post_save_cb<T, F: for<'a> FnCall<(&'a T,), ()>>(opaque: *mut c_void) {
+    // SAFETY: the function is used in T's implementation of VMState.
+    F::call((unsafe { &*(opaque.cast::<T>()) },));
+}
+
 unsafe extern "C" fn vmstate_post_load_cb<
     T,
     F: for<'a> FnCall<(&'a T, u8), Result<(), impl Into<Errno>>>,
@@ -597,12 +602,9 @@ pub const fn pre_save<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>
     }
 
     #[must_use]
-    pub const fn post_save<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
-        mut self,
-        _f: &F,
-    ) -> Self {
+    pub const fn post_save<F: for<'a> FnCall<(&'a T,), ()>>(mut self, _f: &F) -> Self {
         self.0.post_save = if F::IS_SOME {
-            Some(vmstate_no_version_cb::<T, F>)
+            Some(vmstate_post_save_cb::<T, F>)
         } else {
             None
         };
Signature · 1 line
2.51.0
#6 lore raw

Re: [PATCH v3 03/18] migration: make .post_save() a void function

Hola,

Espero que este mensaje se encuentre bien.

Con respecto a la propuesta de Vladimir de cambiar .post_save() a una función void en el proceso de migración, Fabiano ha señalado que este cambio afecta actualmente la compilación en Rust.

Para evitar inconvenientes con el próximo "freeze", Fabiano ya ha sugerido una solución técnica para corregir este error de compatibilidad. Quedamos a la espera de sus comentarios o de su aprobación para proceder con los ajustes necesarios y asegurar que la serie de parches pueda integrarse sin problemas.

Saludos cordiales,

Jhosenad

[email protected]

Quoted 170 lines · Fabiano Rosas
Quoted 23 lines · Vladimir Sementsov-Ogievskiy

All other handlers now have _errp() variants. Should we go this way for .post_save()? Actually it's rather strange, when the vmstate do successful preparations in .pre_save(), then successfully save all sections and subsections, end then fail when all the state is successfully transferred to the target.

Happily, we have only three .post_save() realizations, all always successful. Let's make this a rule.

Also note, that we call .post_save() in two places, and handle its (theoretical) failure inconsistently. Fix that too.

---
 docs/devel/migration/main.rst |  2 +-
 hw/ppc/spapr_pci.c            |  3 +--
 include/migration/vmstate.h   |  8 +++++++-
 migration/savevm.c            |  3 +--
 migration/vmstate.c           | 12 +++---------
 target/arm/machine.c          |  4 +---
 6 files changed, 14 insertions(+), 18 deletions(-)

Review

Quoted 1 line
(quoted diff) +0 -0

b/docs/devel/migration/main.rst

docs/devel/migration/main.rst The functions to do that are inside a vmstate

Quoted 4 lines

index 234d280249a..2de70507640 100644

docs/devel/migration/main.rst +0 -0
@@ -439,7 +439,7 @@ The functions to do that are inside a vmstate

definition, and are called:

Quoted 34 lines
docs/devel/migration/main.rst +1 -1
   This function is called before we save the state of one device.
 
-- ``int (*post_save)(void *opaque);``
+- ``void (*post_save)(void *opaque);``
 
   This function is called after we save the state of one device
   (even upon failure, unless the call to pre_save returned an error).
hw/ppc/spapr_pci.c +1 -2
index ea998bdff15..1dc3b02659f 100644
@@ -2093,14 +2093,13 @@ static int spapr_pci_pre_save(void *opaque)
     return 0;
 }
 
-static int spapr_pci_post_save(void *opaque)
+static void spapr_pci_post_save(void *opaque)
 {
     SpaprPhbState *sphb = opaque;
 
     g_free(sphb->msi_devs);
     sphb->msi_devs = NULL;
     sphb->msi_devs_num = 0;
-    return 0;
 }
 
 static int spapr_pci_post_load(void *opaque, int version_id)
include/migration/vmstate.h +0 -0
index 3695afd483f..85838a49aee 100644
@@ -223,7 +223,13 @@ struct VMStateDescription {
     bool (*post_load_errp)(void *opaque, int version_id, Error **errp);

include/migration/vmstate.h struct VMStateDescription { · line 232

Quoted 10 lines
include/migration/vmstate.h +7 -1
     int (*pre_save)(void *opaque);
     bool (*pre_save_errp)(void *opaque, Error **errp);
-    int (*post_save)(void *opaque);
+
+    /*
+     * Unless .pre_save() fails, .post_save() is called after saving
+     * fields and subsections. It should not fail because at this
+     * point the state has potentially already been transferred.
+     */
+    void (*post_save)(void *opaque);

+CC Paolo, Zhao

This change breaks the rust build:
https://gitlab.com/farosas/qemu/-/jobs/13393119755

error[E0308]: mismatched types

     --> ../rust/migration/src/vmstate.rs:605:18
      |
  605 |             Some(vmstate_no_version_cb::<T, F>)
      |             ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn
pointer, found fn item
      |             |
      |             arguments to this enum variant are incorrect
      |
      = note: expected fn pointer `unsafe extern "C" fn(_) -> ()`
                    found fn item `unsafe extern "C" fn(_) -> i32

{vmstate_no_version_cb::<T, F, impl Into<Errno>>}`

Is something like the following diff the right fix? It'd be nice to get an ack so this series doesn't miss the freeze. (CI run:
https://gitlab.com/farosas/qemu/-/pipelines/2369688593)

-- >8 --
From 90a7b53d2b0d53692df1ef2cfe7607b13e4961d8 Mon Sep 17 00:00:00 2001
From: Fabiano Rosas <[email protected]>
Date: Fri, 6 Mar 2026 18:53:14 -0300
Subject: [PATCH] fixup! migration: make .post_save() a void function

---
 rust/migration/src/migratable.rs |  3 +--
 rust/migration/src/vmstate.rs    | 12 +++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)
rust/migration/src/migratable.rs +1 -2
index 7748aac2f2..7088c1b156 100644
@@ -406,10 +406,9 @@ fn pre_save(&self) -> Result<(), InvalidError> {
         Ok(())
     }
 
-    fn post_save(&self) -> Result<(), InvalidError> {
+    fn post_save(&self) {
         let state = unsafe {
 Box::from_raw(self.migration_state.replace(ptr::null_mut())) };
         drop(state);
-        Ok(())
     }
 
     fn pre_load(&self) -> Result<(), InvalidError> {
rust/migration/src/vmstate.rs +5 -0
index edc7c70265..f34a36f680 100644
@@ -492,6 +492,11 @@ fn from(_value: InvalidError) -> Errno {
     into_neg_errno(result)
 }
 
+unsafe extern "C" fn vmstate_post_save_cb<T, F: for<'a> FnCall<(&'a T,),
 ()>>(opaque: *mut c_void) {
+    // SAFETY: the function is used in T's implementation of VMState.
+    F::call((unsafe { &*(opaque.cast::<T>()) },));
+}
+
 unsafe extern "C" fn vmstate_post_load_cb<
     T,
     F: for<'a> FnCall<(&'a T, u8), Result<(), impl Into<Errno>>>,
rust/migration/src/vmstate.rs +0 -0
@@ -597,12 +602,9 @@ pub const fn pre_save<F: for<'a> FnCall<(&'a T,),

Result<(), impl Into<Errno>>>>

     }

     #[must_use]

- pub const fn post_save<F: for<'a> FnCall<(&'a T,), Result<(), impl
Into<Errno>>>>(

rust/migration/src/vmstate.rs +1 -3
-        mut self,
-        _f: &F,
-    ) -> Self {
+    pub const fn post_save<F: for<'a> FnCall<(&'a T,), ()>>(mut self, _f:
&F) -> Self {
         self.0.post_save = if F::IS_SOME {
rust/migration/src/vmstate.rs +1 -2
-            Some(vmstate_no_version_cb::<T, F>)
+            Some(vmstate_post_save_cb::<T, F>)
         } else {
             None
         };
--

2.51.0

Reviewed-by
#7 lore raw

Re: [PATCH v3 03/18] migration: make .post_save() a void function

In reply to Fabiano Rosas
Quoted 38 lines

+CC Paolo, Zhao

This change breaks the rust build:
https://gitlab.com/farosas/qemu/-/jobs/13393119755

error[E0308]: mismatched types

     --> ../rust/migration/src/vmstate.rs:605:18
      |
  605 |             Some(vmstate_no_version_cb::<T, F>)
      |             ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
      |             |
      |             arguments to this enum variant are incorrect
      |
      = note: expected fn pointer `unsafe extern "C" fn(_) -> ()`
                    found fn item `unsafe extern "C" fn(_) -> i32 {vmstate_no_version_cb::<T, F, impl Into<Errno>>}`

Is something like the following diff the right fix? It'd be nice to get an ack so this series doesn't miss the freeze. (CI run:
https://gitlab.com/farosas/qemu/-/pipelines/2369688593)

-- >8 --
From 90a7b53d2b0d53692df1ef2cfe7607b13e4961d8 Mon Sep 17 00:00:00 2001
From: Fabiano Rosas <[email protected]>
Date: Fri, 6 Mar 2026 18:53:14 -0300
Subject: [PATCH] fixup! migration: make .post_save() a void function

---
 rust/migration/src/migratable.rs |  3 +--
 rust/migration/src/vmstate.rs    | 12 +++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)
rust/migration/src/migratable.rs +0 -0
index 7748aac2f2..7088c1b156 100644

rust/migration/src/migratable.rs fn pre_save(&self) -> Result<(), InvalidError> { · line 412

Quoted 9 lines
rust/migration/src/migratable.rs +1 -2
@@ -406,10 +406,9 @@ fn pre_save(&self) -> Result<(), InvalidError> {
         Ok(())
     }
 
-    fn post_save(&self) -> Result<(), InvalidError> {
+    fn post_save(&self) {
         let state = unsafe { Box::from_raw(self.migration_state.replace(ptr::null_mut())) };
         drop(state);
-        Ok(())
     }

What about this?

fn post_save(&self) {
    let _ = unsafe { Box::from_raw(self.migration_state.replace(ptr::null_mut())) };
}

Others LGTM,

#8 lore raw

PATCH v3 4/18 migration: vmstate_load_state(): add some newlines

In reply to Vladimir Sementsov-Ogievskiy

Split logical blocks by newlines, that simplify reading the code.

---
 migration/vmstate.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
migration/vmstate.c +12 -0
index 5111e7a141f..dd7cd279937 100644
@@ -139,6 +139,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
     int ret = 0;
 
     trace_vmstate_load_state(vmsd->name, version_id);
+
     if (version_id > vmsd->version_id) {
         error_setg(errp, "%s: incoming version_id %d is too new "
                    "for local version_id %d",
@@ -146,6 +147,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
         return -EINVAL;
     }
+
     if  (version_id < vmsd->minimum_version_id) {
         error_setg(errp, "%s: incoming version_id %d is too old "
                    "for local minimum version_id %d",
@@ -153,6 +155,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
         return -EINVAL;
     }
+
     if (vmsd->pre_load_errp) {
         if (!vmsd->pre_load_errp(opaque, errp)) {
             error_prepend(errp, "pre load hook failed for: '%s', "
@@ -171,9 +174,12 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             return ret;
         }
     }
+
     while (field->name) {
         bool exists = vmstate_field_exists(vmsd, field, opaque, version_id);
+
         trace_vmstate_load_state_field(vmsd->name, field->name, exists);
+
         if (exists) {
             void *first_elem = opaque + field->offset;
             int i, n_elems = vmstate_n_elems(opaque, field);
@@ -184,6 +190,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                 first_elem = *(void **)first_elem;
                 assert(first_elem || !n_elems || !size);
             }
+
             for (i = 0; i < n_elems; i++) {
                 void *curr_elem = first_elem + size * i;
                 const VMStateField *inner_field;
@@ -235,6 +242,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                                    vmsd->name, ret);
                     }
                 }
+
                 if (ret < 0) {
                     qemu_file_set_error(f, ret);
                     trace_vmstate_load_field_error(field->name, ret);
@@ -249,11 +257,13 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         field++;
     }
     assert(field->flags == VMS_END);
+
     ret = vmstate_subsection_load(f, vmsd, opaque, errp);
     if (ret != 0) {
         qemu_file_set_error(f, ret);
         return ret;
     }
+
     if (vmsd->post_load_errp) {
         if (!vmsd->post_load_errp(opaque, version_id, errp)) {
             error_prepend(errp, "post load hook failed for: %s, version_id: "
@@ -271,7 +281,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        ret);
         }
     }
+
     trace_vmstate_load_state_end(vmsd->name, "end", ret);
+
     return ret;
 }
 
Signature · 1 line
2.52.0
#9 lore raw

PATCH v3 5/18 migration: vmstate_save/load_state(): refactor tracing errors

In reply to Vladimir Sementsov-Ogievskiy

To simplify further changes (convertion to bool+errp APIs), let's rework some error paths:

- get rid of int ret in traces, as we are moving to bool+errp APIs
- split traces to _fail / _success (seems better than add boolean result to the message).
- prefer short error paths (return immediately on error)
- around trace_vmstate_load_field_error(), do not call qemu_file_set_error(), if the erroc comes from qemu_file_get_error()

---
 migration/trace-events |  6 ++++--
 migration/vmstate.c    | 25 +++++++++++++++----------
 2 files changed, 19 insertions(+), 12 deletions(-)
migration/trace-events +4 -2
index 90629f828f8..0b3f759ccc4 100644
@@ -57,13 +57,15 @@ postcopy_page_req_sync(void *host_addr) "sync page req %p"
 # vmstate.c
 vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
 vmstate_load_state(const char *name, int version_id) "%s v%d"
-vmstate_load_state_end(const char *name, const char *reason, int val) "%s %s/%d"
+vmstate_load_state_fail(const char *name, const char *reason) "%s %s"
+vmstate_load_state_success(const char *name) "%s"
 vmstate_load_state_field(const char *name, const char *field, bool exists) "%s:%s exists=%d"
 vmstate_n_elems(const char *name, int n_elems) "%s: %d"
 vmstate_subsection_load(const char *parent) "%s"
 vmstate_subsection_load_bad(const char *parent,  const char *sub, const char *sub2) "%s: %s/%s"
 vmstate_subsection_load_good(const char *parent) "%s"
-vmstate_save_state_pre_save_res(const char *name, int res) "%s/%d"
+vmstate_save_state_pre_save_fail(const char *name) "%s"
+vmstate_save_state_pre_save_success(const char *name) "%s"
 vmstate_save_state_loop(const char *name, const char *field, int n_elems) "%s/%s[%d]"
 vmstate_save_state_top(const char *idstr) "%s"
 vmstate_subsection_save_loop(const char *name, const char *sub) "%s/%s"
migration/vmstate.c +15 -10
index dd7cd279937..87e1f049592 100644
@@ -144,7 +144,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         error_setg(errp, "%s: incoming version_id %d is too new "
                    "for local version_id %d",
                    vmsd->name, version_id, vmsd->version_id);
-        trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
+        trace_vmstate_load_state_fail(vmsd->name, "too new");
         return -EINVAL;
     }
 
@@ -152,7 +152,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         error_setg(errp, "%s: incoming version_id %d is too old "
                    "for local minimum version_id %d",
                    vmsd->name, version_id, vmsd->minimum_version_id);
-        trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
+        trace_vmstate_load_state_fail(vmsd->name, "too old");
         return -EINVAL;
     }
 
@@ -240,10 +240,10 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                         error_setg(errp,
                                    "Failed to load %s state: stream error: %d",
                                    vmsd->name, ret);
+                        trace_vmstate_load_field_error(field->name, ret);
+                        return ret;
                     }
-                }
-
-                if (ret < 0) {
+                } else {
                     qemu_file_set_error(f, ret);
                     trace_vmstate_load_field_error(field->name, ret);
                     return ret;
@@ -269,7 +269,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             error_prepend(errp, "post load hook failed for: %s, version_id: "
                           "%d, minimum_version: %d: ", vmsd->name,
                           vmsd->version_id, vmsd->minimum_version_id);
-            ret = -EINVAL;
+            trace_vmstate_load_state_fail(vmsd->name, "post-load");
+            return -EINVAL;
         }
     } else if (vmsd->post_load) {
         ret = vmsd->post_load(opaque, version_id);
@@ -279,12 +280,14 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        "minimum_version: %d, ret: %d",
                        vmsd->name, vmsd->version_id, vmsd->minimum_version_id,
                        ret);
+            trace_vmstate_load_state_fail(vmsd->name, "post-load");
+            return ret;
         }
     }
 
-    trace_vmstate_load_state_end(vmsd->name, "end", ret);
+    trace_vmstate_load_state_success(vmsd->name);
 
-    return ret;
+    return 0;
 }
 
 static int vmfield_name_num(const VMStateField *start,
@@ -444,20 +447,22 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
 
     if (vmsd->pre_save_errp) {
         ret = vmsd->pre_save_errp(opaque, errp) ? 0 : -EINVAL;
-        trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
         if (ret < 0) {
             error_prepend(errp, "pre-save for %s failed: ", vmsd->name);
+            trace_vmstate_save_state_pre_save_fail(vmsd->name);
             return ret;
         }
     } else if (vmsd->pre_save) {
         ret = vmsd->pre_save(opaque);
-        trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
         if (ret) {
             error_setg(errp, "pre-save failed: %s", vmsd->name);
+            trace_vmstate_save_state_pre_save_fail(vmsd->name);
             return ret;
         }
     }
 
+    trace_vmstate_save_state_pre_save_success(vmsd->name);
+
     if (vmdesc) {
         json_writer_str(vmdesc, "vmsd_name", vmsd->name);
         json_writer_int64(vmdesc, "version", version_id);
Signature · 1 line
2.52.0
Reviewed-by on v3 5/18
#10 lore raw

Re: [PATCH v3 05/18] migration: vmstate_save/load_state(): refactor tracing errors

Quoted 11 lines · Vladimir Sementsov-Ogievskiy

To simplify further changes (convertion to bool+errp APIs), let's rework some error paths:

- get rid of int ret in traces, as we are moving to bool+errp APIs
- split traces to _fail / _success (seems better than add boolean result to the message).
- prefer short error paths (return immediately on error)
- around trace_vmstate_load_field_error(), do not call qemu_file_set_error(), if the erroc comes from qemu_file_get_error()

Signature · 1 line
Peter Xu
#11 lore raw

PATCH v3 6/18 migration: factor out vmstate_pre_save() from vmstate_save_state()

In reply to Vladimir Sementsov-Ogievskiy

Simplify vmstate_save_state() which is rather big, and simplify further refactoring.

---
 migration/vmstate.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)
migration/vmstate.c +23 -14
index 87e1f049592..605b5c59c76 100644
@@ -435,6 +435,26 @@ bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque)
     return true;
 }
 
+static bool vmstate_pre_save(const VMStateDescription *vmsd, void *opaque,
+                             Error **errp)
+{
+    ERRP_GUARD();
+
+    if (vmsd->pre_save_errp) {
+        if (!vmsd->pre_save_errp(opaque, errp)) {
+            error_prepend(errp, "pre-save for %s failed: ", vmsd->name);
+            return false;
+        }
+    } else if (vmsd->pre_save) {
+        if (vmsd->pre_save(opaque) < 0) {
+            error_setg(errp, "pre-save failed: %s", vmsd->name);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                                 void *opaque, JSONWriter *vmdesc,
                                 int version_id, Error **errp)
@@ -445,20 +465,9 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
 
     trace_vmstate_save_state_top(vmsd->name);
 
-    if (vmsd->pre_save_errp) {
-        ret = vmsd->pre_save_errp(opaque, errp) ? 0 : -EINVAL;
-        if (ret < 0) {
-            error_prepend(errp, "pre-save for %s failed: ", vmsd->name);
-            trace_vmstate_save_state_pre_save_fail(vmsd->name);
-            return ret;
-        }
-    } else if (vmsd->pre_save) {
-        ret = vmsd->pre_save(opaque);
-        if (ret) {
-            error_setg(errp, "pre-save failed: %s", vmsd->name);
-            trace_vmstate_save_state_pre_save_fail(vmsd->name);
-            return ret;
-        }
+    if (!vmstate_pre_save(vmsd, opaque, errp)) {
+        trace_vmstate_save_state_pre_save_fail(vmsd->name);
+        return -EINVAL;
     }
 
     trace_vmstate_save_state_pre_save_success(vmsd->name);
Signature · 1 line
2.52.0
#12 lore raw

PATCH v3 7/18 migration: factor out vmstate_save_field() from vmstate_save_state()

In reply to Vladimir Sementsov-Ogievskiy

Simplify vmstate_save_state() which is rather big, and simplify further refactoring.

---
 migration/vmstate.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)
migration/vmstate.c +25 -15
index 605b5c59c76..2ce26527124 100644
@@ -26,6 +26,9 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
                                    Error **errp);
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque, Error **errp);
+static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
+                                void *opaque, JSONWriter *vmdesc,
+                                int version_id, Error **errp);
 
 /* Whether this field should exist for either save or load the VM? */
 static bool
@@ -455,6 +458,26 @@ static bool vmstate_pre_save(const VMStateDescription *vmsd, void *opaque,
     return true;
 }
 
+static bool vmstate_save_field(QEMUFile *f, void *pv, size_t size,
+                               const VMStateField *field,
+                               JSONWriter *vmdesc, Error **errp)
+{
+    if (field->flags & VMS_STRUCT) {
+        return vmstate_save_state(f, field->vmsd, pv, vmdesc, errp) >= 0;
+    } else if (field->flags & VMS_VSTRUCT) {
+        return vmstate_save_state_v(f, field->vmsd, pv, vmdesc,
+                                    field->struct_version_id,
+                                    errp) >= 0;
+    }
+
+    if (field->info->put(f, pv, size, field, vmdesc) < 0) {
+        error_setg(errp, "put failed");
+        return false;
+    }
+
+    return true;
+}
+
 static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                                 void *opaque, JSONWriter *vmdesc,
                                 int version_id, Error **errp)
@@ -548,21 +571,8 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                 vmsd_desc_field_start(vmsd, vmdesc_loop, inner_field,
                                       i, max_elems);
 
-                if (inner_field->flags & VMS_STRUCT) {
-                    ret = vmstate_save_state(f, inner_field->vmsd,
-                                             curr_elem, vmdesc_loop, errp);
-                } else if (inner_field->flags & VMS_VSTRUCT) {
-                    ret = vmstate_save_state_v(f, inner_field->vmsd,
-                                               curr_elem, vmdesc_loop,
-                                               inner_field->struct_version_id,
-                                               errp);
-                } else {
-                    ret = inner_field->info->put(f, curr_elem, size,
-                                                 inner_field, vmdesc_loop);
-                    if (ret < 0) {
-                        error_setg(errp, "put failed");
-                    }
-                }
+                ret = vmstate_save_field(f, curr_elem, size, inner_field,
+                                         vmdesc_loop, errp) ? 0 : -EINVAL;
 
                 written_bytes = qemu_file_transferred(f) - old_offset;
                 vmsd_desc_field_end(vmsd, vmdesc_loop, inner_field,
Signature · 1 line
2.52.0
#13 lore raw

PATCH v3 8/18 migration: factor out vmstate_pre_load() from vmstate_load_state()

In reply to Vladimir Sementsov-Ogievskiy

Simplify vmstate_load_state() which is rather big, and simplify further refactoring.

---
 migration/vmstate.c | 46 ++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)
migration/vmstate.c +29 -17
index 2ce26527124..35dbe1550e9 100644
@@ -134,6 +134,33 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
     }
 }
 
+static bool vmstate_pre_load(const VMStateDescription *vmsd, void *opaque,
+                             Error **errp)
+{
+    ERRP_GUARD();
+
+    if (vmsd->pre_load_errp) {
+        if (!vmsd->pre_load_errp(opaque, errp)) {
+            error_prepend(errp, "pre load hook failed for: '%s', "
+                          "version_id: %d, minimum version_id: %d: ",
+                          vmsd->name, vmsd->version_id,
+                          vmsd->minimum_version_id);
+            return false;
+        }
+    } else if (vmsd->pre_load) {
+        int ret = vmsd->pre_load(opaque);
+        if (ret) {
+            error_setg(errp, "pre load hook failed for: '%s', "
+                       "version_id: %d, minimum version_id: %d, ret: %d",
+                       vmsd->name, vmsd->version_id, vmsd->minimum_version_id,
+                       ret);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id, Error **errp)
 {
@@ -159,23 +186,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         return -EINVAL;
     }
 
-    if (vmsd->pre_load_errp) {
-        if (!vmsd->pre_load_errp(opaque, errp)) {
-            error_prepend(errp, "pre load hook failed for: '%s', "
-                          "version_id: %d, minimum version_id: %d: ",
-                          vmsd->name, vmsd->version_id,
-                          vmsd->minimum_version_id);
-            return -EINVAL;
-        }
-    } else if (vmsd->pre_load) {
-        ret = vmsd->pre_load(opaque);
-        if (ret) {
-            error_setg(errp, "pre load hook failed for: '%s', "
-                       "version_id: %d, minimum version_id: %d, ret: %d",
-                       vmsd->name, vmsd->version_id, vmsd->minimum_version_id,
-                       ret);
-            return ret;
-        }
+    if (!vmstate_pre_load(vmsd, opaque, errp)) {
+        return -EINVAL;
     }
 
     while (field->name) {
Signature · 1 line
2.52.0
#14 lore raw

PATCH v3 9/18 migration: factor out vmstate_load_field() from vmstate_load_state()

In reply to Vladimir Sementsov-Ogievskiy

Simplify vmstate_load_state() which is rather big, and simplify further refactoring.

---
 migration/vmstate.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)
migration/vmstate.c +23 -18
index 35dbe1550e9..32944c04443 100644
@@ -161,6 +161,27 @@ static bool vmstate_pre_load(const VMStateDescription *vmsd, void *opaque,
     return true;
 }
 
+static bool vmstate_load_field(QEMUFile *f, void *pv, size_t size,
+                               const VMStateField *field, Error **errp)
+{
+    if (field->flags & VMS_STRUCT) {
+        return vmstate_load_state(f, field->vmsd, pv, field->vmsd->version_id,
+                                  errp) >= 0;
+    } else if (field->flags & VMS_VSTRUCT) {
+        return vmstate_load_state(f, field->vmsd, pv, field->struct_version_id,
+                                  errp) >= 0;
+    }
+
+    if (field->info->get(f, pv, size, field) < 0) {
+        error_setg(errp,
+                   "Failed to load element of type %s for %s",
+                   field->info->name, field->name);
+        return false;
+    }
+
+    return true;
+}
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id, Error **errp)
 {
@@ -225,24 +246,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                     inner_field = field;
                 }
 
-                if (inner_field->flags & VMS_STRUCT) {
-                    ret = vmstate_load_state(f, inner_field->vmsd, curr_elem,
-                                             inner_field->vmsd->version_id,
-                                             errp);
-                } else if (inner_field->flags & VMS_VSTRUCT) {
-                    ret = vmstate_load_state(f, inner_field->vmsd, curr_elem,
-                                             inner_field->struct_version_id,
-                                             errp);
-                } else {
-                    ret = inner_field->info->get(f, curr_elem, size,
-                                                 inner_field);
-                    if (ret < 0) {
-                        error_setg(errp,
-                                   "Failed to load element of type %s for %s: "
-                                   "%d", inner_field->info->name,
-                                   inner_field->name, ret);
-                    }
-                }
+                ret = vmstate_load_field(f, curr_elem, size, inner_field,
+                                         errp) ? 0 : -EINVAL;
 
                 /* If we used a fake temp field.. free it now */
                 if (inner_field != field) {
Signature · 1 line
2.52.0
#15 lore raw

PATCH v3 10/18 migration: factor out vmstate_post_load() from vmstate_load_state()

In reply to Vladimir Sementsov-Ogievskiy

Simplify vmstate_load_state() which is rather big, and simplify further refactoring.

---
 migration/vmstate.c | 49 +++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 19 deletions(-)
migration/vmstate.c +30 -19
index 32944c04443..53b57c09d82 100644
@@ -182,6 +182,33 @@ static bool vmstate_load_field(QEMUFile *f, void *pv, size_t size,
     return true;
 }
 
+static bool vmstate_post_load(const VMStateDescription *vmsd,
+                              void *opaque, int version_id, Error **errp)
+{
+    ERRP_GUARD();
+
+    if (vmsd->post_load_errp) {
+        if (!vmsd->post_load_errp(opaque, version_id, errp)) {
+            error_prepend(errp, "post load hook failed for: %s, version_id: "
+                          "%d, minimum_version: %d: ", vmsd->name,
+                          vmsd->version_id, vmsd->minimum_version_id);
+            return false;
+        }
+    } else if (vmsd->post_load) {
+        int ret = vmsd->post_load(opaque, version_id);
+        if (ret < 0) {
+            error_setg(errp,
+                       "post load hook failed for: %s, version_id: %d, "
+                       "minimum_version: %d, ret: %d",
+                       vmsd->name, vmsd->version_id, vmsd->minimum_version_id,
+                       ret);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id, Error **errp)
 {
@@ -284,25 +311,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         return ret;
     }
 
-    if (vmsd->post_load_errp) {
-        if (!vmsd->post_load_errp(opaque, version_id, errp)) {
-            error_prepend(errp, "post load hook failed for: %s, version_id: "
-                          "%d, minimum_version: %d: ", vmsd->name,
-                          vmsd->version_id, vmsd->minimum_version_id);
-            trace_vmstate_load_state_fail(vmsd->name, "post-load");
-            return -EINVAL;
-        }
-    } else if (vmsd->post_load) {
-        ret = vmsd->post_load(opaque, version_id);
-        if (ret < 0) {
-            error_setg(errp,
-                       "post load hook failed for: %s, version_id: %d, "
-                       "minimum_version: %d, ret: %d",
-                       vmsd->name, vmsd->version_id, vmsd->minimum_version_id,
-                       ret);
-            trace_vmstate_load_state_fail(vmsd->name, "post-load");
-            return ret;
-        }
+    if (!vmstate_post_load(vmsd, opaque, version_id, errp)) {
+        trace_vmstate_load_state_fail(vmsd->name, "post-load");
+        return -EINVAL;
     }
 
     trace_vmstate_load_state_success(vmsd->name);
Signature · 1 line
2.52.0
#16 lore raw

PATCH v3 11/18 migration: convert vmstate_subsection_save/load functions to bool

In reply to Vladimir Sementsov-Ogievskiy

Convert them to bool return value, as preparation to further convertion of vmstate_save/load_state().

---
 migration/vmstate.c | 57 +++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 31 deletions(-)
migration/vmstate.c +26 -31
index 53b57c09d82..8825aa3b1f1 100644
@@ -21,11 +21,11 @@
 #include "qemu/error-report.h"
 #include "trace.h"
 
-static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
-                                   void *opaque, JSONWriter *vmdesc,
-                                   Error **errp);
-static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
-                                   void *opaque, Error **errp);
+static bool vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
+                                    void *opaque, JSONWriter *vmdesc,
+                                    Error **errp);
+static bool vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
+                                    void *opaque, Error **errp);
 static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
                                 void *opaque, JSONWriter *vmdesc,
                                 int version_id, Error **errp);
@@ -305,10 +305,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
     }
     assert(field->flags == VMS_END);
 
-    ret = vmstate_subsection_load(f, vmsd, opaque, errp);
-    if (ret != 0) {
-        qemu_file_set_error(f, ret);
-        return ret;
+    if (!vmstate_subsection_load(f, vmsd, opaque, errp)) {
+        qemu_file_set_error(f, -EINVAL);
+        return -EINVAL;
     }
 
     if (!vmstate_post_load(vmsd, opaque, version_id, errp)) {
@@ -637,7 +636,7 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
         json_writer_end_array(vmdesc);
     }
 
-    ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
+    ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp) ? 0 : -EINVAL;
 
 out:
     if (vmsd->post_save) {
@@ -667,15 +666,14 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                                 errp);
 }
 
-static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
-                                   void *opaque, Error **errp)
+static bool vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
+                                    void *opaque, Error **errp)
 {
     ERRP_GUARD();
     trace_vmstate_subsection_load(vmsd->name);
 
     while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
         char idstr[256], *idstr_ret;
-        int ret;
         uint8_t version_id, len, size;
         const VMStateDescription *sub_vmsd;
 
@@ -683,12 +681,12 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
         if (len < strlen(vmsd->name) + 1) {
             /* subsection name has to be "section_name/a" */
             trace_vmstate_subsection_load_bad(vmsd->name, "(short)", "");
-            return 0;
+            return true;
         }
         size = qemu_peek_buffer(f, (uint8_t **)&idstr_ret, len, 2);
         if (size != len) {
             trace_vmstate_subsection_load_bad(vmsd->name, "(peek fail)", "");
-            return 0;
+            return true;
         }
         memcpy(idstr, idstr_ret, size);
         idstr[size] = 0;
@@ -696,41 +694,39 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
         if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
             trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(prefix)");
             /* it doesn't have a valid subsection name */
-            return 0;
+            return true;
         }
         sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
         if (sub_vmsd == NULL) {
             trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(lookup)");
             error_setg(errp, "VM subsection '%s' in '%s' does not exist",
                        idstr, vmsd->name);
-            return -ENOENT;
+            return false;
         }
         qemu_file_skip(f, 1); /* subsection */
         qemu_file_skip(f, 1); /* len */
         qemu_file_skip(f, len); /* idstr */
         version_id = qemu_get_be32(f);
 
-        ret = vmstate_load_state(f, sub_vmsd, opaque, version_id, errp);
-        if (ret) {
+        if (vmstate_load_state(f, sub_vmsd, opaque, version_id, errp) < 0) {
             trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(child)");
             error_prepend(errp,
-                          "Loading VM subsection '%s' in '%s' failed: %d: ",
-                          idstr, vmsd->name, ret);
-            return ret;
+                          "Loading VM subsection '%s' in '%s' failed: ",
+                          idstr, vmsd->name);
+            return false;
         }
     }
 
     trace_vmstate_subsection_load_good(vmsd->name);
-    return 0;
+    return true;
 }
 
-static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
-                                   void *opaque, JSONWriter *vmdesc,
-                                   Error **errp)
+static bool vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
+                                    void *opaque, JSONWriter *vmdesc,
+                                    Error **errp)
 {
     const VMStateDescription * const *sub = vmsd->subsections;
     bool vmdesc_has_subsections = false;
-    int ret = 0;
 
     trace_vmstate_subsection_save_top(vmsd->name);
     while (sub && *sub) {
@@ -754,9 +750,8 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
             qemu_put_byte(f, len);
             qemu_put_buffer(f, (uint8_t *)vmsdsub->name, len);
             qemu_put_be32(f, vmsdsub->version_id);
-            ret = vmstate_save_state(f, vmsdsub, opaque, vmdesc, errp);
-            if (ret) {
-                return ret;
+            if (vmstate_save_state(f, vmsdsub, opaque, vmdesc, errp) < 0) {
+                return false;
             }
 
             if (vmdesc) {
@@ -770,5 +765,5 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
         json_writer_end_array(vmdesc);
     }
 
-    return ret;
+    return true;
 }
Signature · 1 line
2.52.0
#17 lore raw

PATCH v3 12/18 migration: VMStateInfo: introduce new handlers with errp

In reply to Vladimir Sementsov-Ogievskiy

Add new APIs with errp, to allow handlers report good error messages. We'll convert existing handlers soon.

---
 include/migration/vmstate.h | 17 ++++++++++++++---
 migration/vmstate.c         |  4 ++++
 2 files changed, 18 insertions(+), 3 deletions(-)
include/migration/vmstate.h +14 -3
index 85838a49aee..77d58d27d41 100644
@@ -32,12 +32,16 @@
 typedef struct VMStateInfo VMStateInfo;
 typedef struct VMStateField VMStateField;
 
-/* VMStateInfo allows customized migration of objects that don't fit in
+/*
+ * VMStateInfo allows customized migration of objects that don't fit in
  * any category in VMStateFlags. Additional information is always passed
- * into get and put in terms of field and vmdesc parameters. However
+ * into load and save in terms of field and vmdesc parameters. However
  * these two parameters should only be used in cases when customized
  * handling is needed, such as QTAILQ. For primitive data types such as
- * integer, field and vmdesc parameters should be ignored inside get/put.
+ * integer, field and vmdesc parameters should be ignored inside load/save.
+ *
+ * @get and @put are deprecated copies of @load and @save. For new interfaces
+ * use @load and @save.
  */
 struct VMStateInfo {
     const char *name;
@@ -46,6 +50,13 @@ struct VMStateInfo {
     int coroutine_mixed_fn (*put)(QEMUFile *f, void *pv, size_t size,
                                   const VMStateField *field,
                                   JSONWriter *vmdesc);
+    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size,
+                                    const VMStateField *field,
+                                    Error **errp);
+    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size,
+                                    const VMStateField *field,
+                                    JSONWriter *vmdesc,
+                                    Error **errp);
 };
 
 enum VMStateFlags {
migration/vmstate.c +4 -0
index 8825aa3b1f1..f16626d7d11 100644
@@ -170,6 +170,8 @@ static bool vmstate_load_field(QEMUFile *f, void *pv, size_t size,
     } else if (field->flags & VMS_VSTRUCT) {
         return vmstate_load_state(f, field->vmsd, pv, field->struct_version_id,
                                   errp) >= 0;
+    } else if (field->info->load) {
+        return field->info->load(f, pv, size, field, errp);
     }
 
     if (field->info->get(f, pv, size, field) < 0) {
@@ -495,6 +497,8 @@ static bool vmstate_save_field(QEMUFile *f, void *pv, size_t size,
         return vmstate_save_state_v(f, field->vmsd, pv, vmdesc,
                                     field->struct_version_id,
                                     errp) >= 0;
+    } else if (field->info->save) {
+        return field->info->save(f, pv, size, field, vmdesc, errp);
     }
 
     if (field->info->put(f, pv, size, field, vmdesc) < 0) {
Signature · 1 line
2.52.0
#18 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

include/migration/vmstate.h struct VMStateInfo { · line 59

Quoted 7 lines · Vladimir Sementsov-Ogievskiy
include/migration/vmstate.h +7 -0
+    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size,
+                                    const VMStateField *field,
+                                    Error **errp);
+    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size,
+                                    const VMStateField *field,
+                                    JSONWriter *vmdesc,
+                                    Error **errp);

Use void functions for callbacks, with a wrapper adding the bool type. While bool/errp is preferred in general, for callbacks it provides N opportunities to get it wrong instead of just one.

Paolo

#19 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

Quoted 10 lines · Paolo Bonzini
Quoted 7 lines · Vladimir Sementsov-Ogievskiy

+    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size, +                                    const VMStateField *field, +                                    Error **errp);
+    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size, +                                    const VMStateField *field, +                                    JSONWriter *vmdesc,
+                                    Error **errp);

Use void functions for callbacks, with a wrapper adding the bool type. While bool/errp is preferred in general, for callbacks it provides N opportunities to get it wrong instead of just one.

Hmm, for now, the series is already merged.

Next, I'm unsure, how callbacks are different from other functions around errors?

I'm not sure that making wrappers, which will have to use ERRP_GUARD and check *errp for the error is a good practice. If it is, it would be good to start by modifying the documentation in include/qapi/error.h to mention the exclusion for callbacks.

Signature · 2 lines
Best regards,
Vladimir
#20 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

Il lun 27 apr 2026, 15:19 Vladimir Sementsov-Ogievskiy < [email protected]> ha scritto:

Quoted 10 lines · Paolo Bonzini
Quoted 9 lines · Vladimir Sementsov-Ogievskiy

include/migration/vmstate.h struct VMStateInfo { · line 59

Quoted 7 lines
include/migration/vmstate.h +7 -0
+    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size,
+                                    const VMStateField *field,
+                                    Error **errp);
+    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size,
+                                    const VMStateField *field,
+                                    JSONWriter *vmdesc,
+                                    Error **errp);

Use void functions for callbacks, with a wrapper adding the bool type.

Quoted 9 lines

While bool/errp is preferred in general, for callbacks it provides N opportunities to get it wrong instead of just one.

Quoted 1 line

Hmm, for now, the series is already merged.

Next, I'm unsure, how callbacks are different from other functions around errors?

As I wrote above, confusing the *error == NULL and bool return value is avoided without much extra effort for callbacks (or other kinds of virtual functions) l, because there is a single caller. So the balance between "avoiding the possibility of getting the return value wrong" and "convenience of having a return value" tilts more towards the former.

I will post a follow up.

Paolo

I'm not sure that making wrappers, which will have to use ERRP_GUARD and

Quoted 13 lines

check
*errp for the error is a good practice. If it is, it would be good to start by
modifying the documentation in include/qapi/error.h to mention the exclusion
for callbacks.

--
Best regards,
Vladimir

#21 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

[add Markus]

Quoted 16 lines · Paolo Bonzini

Il lun 27 apr 2026, 15:19 Vladimir Sementsov-Ogievskiy <[email protected] <mailto:[email protected]>> ha scritto:

On 26.04.26 11:33, Paolo Bonzini wrote:
> On 3/4/26 21:22, Vladimir Sementsov-Ogievskiy wrote:
>> +    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size,
>> +                                    const VMStateField *field,
>> +                                    Error **errp);
>> +    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size,
>> +                                    const VMStateField *field,
>> +                                    JSONWriter *vmdesc,
>> +                                    Error **errp);
> Use void functions for callbacks, with a wrapper adding the bool type. While bool/errp is preferred in general, for callbacks it provides N opportunities to get it wrong instead of just one. >

Quoted 7 lines

Hmm, for now, the series is already merged.

Next, I'm unsure, how callbacks are different from other functions around errors?

As I wrote above, confusing the *error == NULL and bool return value is avoided without much extra effort for callbacks (or other kinds of virtual functions) l, because there is a single caller. So the balance between "avoiding the possibility of getting the return value wrong" and "convenience of having a return value" tilts more towards the former.

Understand. Hm.

An alternative for this single-caller case, is just add an assertion into this caller:

bool wrapper(..., errp) {

   ERRP_GUARD();

   bool ok = x->foo(..., errp);

   assert(ok == !(*errp));

return ok;
}

with this, we have:

- same overhead (using ERRP_GUARD)

- same guarantee

- keep common recommended bool+errp interface everywhere, which is rather good benefit IMHO

Markus, what do you think about this case of errp-functions?

Signature · 2 lines
Best regards,
Vladimir
#22 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

Quoted 28 lines · Vladimir Sementsov-Ogievskiy

[add Markus]

Quoted 16 lines · Paolo Bonzini

Il lun 27 apr 2026, 15:19 Vladimir Sementsov-Ogievskiy <[email protected] <mailto:[email protected]>> ha scritto:

    On 26.04.26 11:33, Paolo Bonzini wrote:
     > On 3/4/26 21:22, Vladimir Sementsov-Ogievskiy wrote:
     >> +    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size,
     >> +                                    const VMStateField *field,
     >> +                                    Error **errp);
     >> +    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size,
     >> +                                    const VMStateField *field,
     >> +                                    JSONWriter *vmdesc,
     >> +                                    Error **errp);
     > Use void functions for callbacks, with a wrapper adding the bool type. While bool/errp is preferred in general, for callbacks it provides N opportunities to get it wrong instead of just one. >

Quoted 6 lines

    Hmm, for now, the series is already merged.

    Next, I'm unsure, how callbacks are different from other functions around errors?

As I wrote above, confusing the *error == NULL and bool return value is avoided without much extra effort for callbacks (or other kinds of virtual functions) l, because there is a single caller. So the balance between "avoiding the possibility of getting the return value wrong" and "convenience of having a return value" tilts more towards the former.

Understand. Hm.

Quoted 8 lines

An alternative for this single-caller case, is just add an assertion into this caller:

bool wrapper(..., errp) {
   ERRP_GUARD();
   bool ok = x->foo(..., errp);
   assert(ok == !(*errp));
   return ok;
}

I don't think this makes sense, the assertion moves a compile-time guarantee to a run-time guarantee which is strictly worse.

There are many more examples of void+errp callbacks than bool+errp, the most important one being .realize for devices. A quick grep, which is certainly not fully accurate, shows 63 vs. 27:

$ git grep -w 'void.*(\*.*errp' | wc -l
63
$ git grep -w 'bool.*(\*.*errp' | wc -l
27

In many cases there are just two or three implementation so I wouldn't go to great lengths to change it even if I disagree. But for vmstate (e.g. pre_load_errp and friends) this is not going to be the case in the long run, and it's more similar to the device case.

Paolo

#23 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

Quoted 16 lines · Paolo Bonzini
Quoted 15 lines · Vladimir Sementsov-Ogievskiy

[add Markus]

Quoted 13 lines · Paolo Bonzini

Il lun 27 apr 2026, 15:19 Vladimir Sementsov-Ogievskiy <[email protected] <mailto:[email protected]>> ha scritto:

Quoted 10 lines · Paolo Bonzini
Quoted 8 lines · Vladimir Sementsov-Ogievskiy
Quoted 7 lines

+    bool coroutine_mixed_fn (*load)(QEMUFile *f, void *pv, size_t size, +                                    const VMStateField *field, +                                    Error **errp);
+    bool coroutine_mixed_fn (*save)(QEMUFile *f, void *pv, size_t size, +                                    const VMStateField *field, +                                    JSONWriter *vmdesc,
+                                    Error **errp);

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Use void functions for callbacks, with a wrapper adding the bool type. While bool/errp is preferred in general, for callbacks it provides N opportunities to get it wrong instead of just one.

Hmm, for now, the series is already merged.

Next, I'm unsure, how callbacks are different from other functions around errors?

As I wrote above, confusing the *error == NULL and bool return value is avoided without much extra effort for callbacks (or other kinds of virtual functions) l, because there is a single caller. So the balance between "avoiding the possibility of getting the return value wrong" and "convenience of having a return value" tilts more towards the former.

Bug patterns:

1. Set an error, immediately return success. I can't remember seeing this.

2. Set an error, forget to return, somehow survive all the way to the normal success return. I've seen and fixed this. Returning void does not help at all here.

3. Set an error, goto error exit without flipping the return value variable from success to failure. I've seen and fixed this, too. Best to initialize the return value variable to failure, and flip it to success only when success is ensured. Of course, that flip can be forgotten as well if it's needed in multiple places. That's rare. Anyway, returning eliminates this bug pattern.

The last one does some tilting.

Tilting the other way: keeping the rules simple and consistent with GError's.

Quoted 11 lines
Quoted 10 lines

Understand. Hm.

An alternative for this single-caller case, is just add an assertion into this caller:

bool wrapper(..., errp) {
   ERRP_GUARD();
   bool ok = x->foo(..., errp);
   assert(ok == !(*errp));
   return ok;
}

Quoted 8 lines

I don't think this makes sense, the assertion moves a compile-time guarantee to a run-time guarantee which is strictly worse.

There are many more examples of void+errp callbacks than bool+errp, the most important one being .realize for devices. A quick grep, which is certainly not fully accurate, shows 63 vs. 27:

$ git grep -w 'void.*(\*.*errp' | wc -l
63
$ git grep -w 'bool.*(\*.*errp' | wc -l
27

Yes. At some point, I briefly considered updating them to return bool for rules conformance, but decided it wasn't worth it.

Quoted 1 line

In many cases there are just two or three implementation so I wouldn't go to great lengths to change it even if I disagree. But for vmstate (e.g. pre_load_errp and friends) this is not going to be the case in the long run, and it's more similar to the device case.

We should always be willing to accept reasoned exceptions to rules. However...

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

I'm not sure that making wrappers, which will have to use ERRP_GUARD and check *errp for the error is a good practice. If it is, it would be good to start by modifying the documentation in include/qapi/error.h to mention the exclusion for callbacks.

... explaining them in documentation is desirable.

#24 lore raw

Re: [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp

Il mar 28 apr 2026, 13:39 Markus Armbruster <[email protected]> ha scritto:

Quoted 7 lines

3. Set an error, goto error exit without flipping the return value variable from success to failure. I've seen and fixed this, too. Best to initialize the return value variable to failure, and flip it to success only when success is ensured. Of course, that flip can be forgotten as well if it's needed in multiple places. That's rare. Anyway, returning eliminates this bug pattern.

This can also be fixed with autofree etc.

Another one is returning directly the value from another function without flipping from 0/-errno to bool. I think this one is more common.

Paolo

The last one does some tilting.

Quoted 49 lines

Tilting the other way: keeping the rules simple and consistent with GError's.

Quoted 3 lines
Quoted 3 lines

Understand. Hm.

An alternative for this single-caller case, is just add an assertion

into this caller:

Quoted 9 lines
Quoted 7 lines
bool wrapper(..., errp) {
    ERRP_GUARD();
    bool ok = x->foo(..., errp);
    assert(ok == !(*errp));
    return ok;
}

I don't think this makes sense, the assertion moves a compile-time

guarantee to a run-time guarantee which is strictly worse.

Quoted 2 lines

There are many more examples of void+errp callbacks than bool+errp, the

most important one being .realize for devices. A quick grep, which is certainly not fully accurate, shows 63 vs. 27:

Quoted 5 lines
$ git grep -w 'void.*(\*.*errp' | wc -l
63
$ git grep -w 'bool.*(\*.*errp' | wc -l
27

Yes. At some point, I briefly considered updating them to return bool for rules conformance, but decided it wasn't worth it.

Quoted 1 line

In many cases there are just two or three implementation so I wouldn't

go to great lengths to change it even if I disagree. But for vmstate (e.g. pre_load_errp and friends) this is not going to be the case in the long run, and it's more similar to the device case.

We should always be willing to accept reasoned exceptions to rules. However...

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

I'm not sure that making wrappers, which will have to use ERRP_GUARD

and check

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

*errp for the error is a good practice. If it is, it would be good to

start by

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

modifying the documentation in include/qapi/error.h to mention the

exclusion

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

for callbacks.

... explaining them in documentation is desirable.

#25 lore raw

PATCH v3 13/18 migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd()

In reply to Vladimir Sementsov-Ogievskiy

Introduce new APIs, returning bool.
The analysis
https://lore.kernel.org/qemu-devel/[email protected]/
shows, that vmstate_load_state() return value actually only used to check for success, specific errno values doesn't make sense.

With this commit we introduce new functions with modern bool interface, and in following commits we'll update the code base to use them, starting from migration/ code, and finally we will remove old vmstate_load_state() and vmstate_save_state().

This patch reworks existing functions to new one, so that old interfaces are simple wrappers, which will be easy to remove later.

---
 include/migration/vmstate.h |  9 ++++
 migration/vmstate.c         | 89 ++++++++++++++++++++-----------------
 2 files changed, 58 insertions(+), 40 deletions(-)
include/migration/vmstate.h +9 -0
index 77d58d27d41..3c1d84f2d67 100644
@@ -1247,10 +1247,19 @@ extern const VMStateInfo vmstate_info_qlist;
         .flags = VMS_END, \
     }
 
+/*
+ * vmstate_load_state() and vmstate_save_state() are
+ * depreacated, use vmstate_load_vmsd() and vmstate_save_vmsd()
+ * instead.
+ */
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id, Error **errp);
 int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, JSONWriter *vmdesc, Error **errp);
+bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
+                       void *opaque, int version_id, Error **errp);
+bool vmstate_save_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
+                       void *opaque, JSONWriter *vmdesc, Error **errp);
 
 bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque);
 
migration/vmstate.c +49 -40

239 lines not on this page · raw message

Signature · 1 line
2.52.0
Reviewed-by on v3 13/18
#26 lore raw

Re: [PATCH v3 13/18] migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd()

Quoted 13 lines · Vladimir Sementsov-Ogievskiy

Introduce new APIs, returning bool.
The analysis
https://lore.kernel.org/qemu-devel/[email protected]/
shows, that vmstate_load_state() return value actually only used to check for success, specific errno values doesn't make sense.

With this commit we introduce new functions with modern bool interface, and in following commits we'll update the code base to use them, starting from migration/ code, and finally we will remove old vmstate_load_state() and vmstate_save_state().

Quoted 5 lines

This patch reworks existing functions to new one, so that old interfaces are simple wrappers, which will be easy to remove later.

Signature · 1 line
Peter Xu
#27 lore raw

PATCH v3 14/18 migration/cpr: move to new migration APIs

In reply to Vladimir Sementsov-Ogievskiy
---
 include/migration/cpr.h |  2 +-
 migration/cpr.c         | 22 +++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)
include/migration/cpr.h +1 -1
index 5850fd1788b..96ce26e7117 100644
@@ -43,7 +43,7 @@ void cpr_set_incoming_mode(MigMode mode);
 bool cpr_is_incoming(void);
 
 bool cpr_state_save(MigrationChannel *channel, Error **errp);
-int cpr_state_load(MigrationChannel *channel, Error **errp);
+bool cpr_state_load(MigrationChannel *channel, Error **errp);
 void cpr_state_close(void);
 struct QIOChannel *cpr_state_ioc(void);
 
migration/cpr.c +9 -13
index a0b37007f55..05266dfcfd3 100644
@@ -178,7 +178,6 @@ bool cpr_is_incoming(void)
 
 bool cpr_state_save(MigrationChannel *channel, Error **errp)
 {
-    int ret;
     QEMUFile *f;
     MigMode mode = migrate_mode();
 
@@ -199,8 +198,7 @@ bool cpr_state_save(MigrationChannel *channel, Error **errp)
     qemu_put_be32(f, QEMU_CPR_FILE_MAGIC);
     qemu_put_be32(f, QEMU_CPR_FILE_VERSION);
 
-    ret = vmstate_save_state(f, &vmstate_cpr_state, &cpr_state, 0, errp);
-    if (ret) {
+    if (!vmstate_save_vmsd(f, &vmstate_cpr_state, &cpr_state, 0, errp)) {
         qemu_fclose(f);
         return false;
     }
@@ -223,9 +221,8 @@ bool cpr_state_save(MigrationChannel *channel, Error **errp)
     return true;
 }
 
-int cpr_state_load(MigrationChannel *channel, Error **errp)
+bool cpr_state_load(MigrationChannel *channel, Error **errp)
 {
-    int ret;
     uint32_t v;
     QEMUFile *f;
     MigMode mode = 0;
@@ -241,10 +238,10 @@ int cpr_state_load(MigrationChannel *channel, Error **errp)
         cpr_set_incoming_mode(mode);
         f = cpr_transfer_input(channel, errp);
     } else {
-        return 0;
+        return true;
     }
     if (!f) {
-        return -1;
+        return false;
     }
 
     trace_cpr_state_load(MigMode_str(mode));
@@ -254,19 +251,18 @@ int cpr_state_load(MigrationChannel *channel, Error **errp)
     if (v != QEMU_CPR_FILE_MAGIC) {
         error_setg(errp, "Not a migration stream (bad magic %x)", v);
         qemu_fclose(f);
-        return -EINVAL;
+        return false;
     }
     v = qemu_get_be32(f);
     if (v != QEMU_CPR_FILE_VERSION) {
         error_setg(errp, "Unsupported migration stream version %d", v);
         qemu_fclose(f);
-        return -ENOTSUP;
+        return false;
     }
 
-    ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1, errp);
-    if (ret) {
+    if (!vmstate_load_vmsd(f, &vmstate_cpr_state, &cpr_state, 1, errp)) {
         qemu_fclose(f);
-        return ret;
+        return false;
     }
 
     if (migrate_mode() == MIG_MODE_CPR_EXEC) {
@@ -280,7 +276,7 @@ int cpr_state_load(MigrationChannel *channel, Error **errp)
      */
     cpr_state_file = f;
 
-    return ret;
+    return true;
 }
 
 void cpr_state_close(void)
Signature · 1 line
2.52.0
#28 lore raw

PATCH v3 15/18 migration/savevm: move to new migration APIs

In reply to Vladimir Sementsov-Ogievskiy
---
 migration/savevm.c | 105 ++++++++++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 50 deletions(-)
migration/savevm.c +55 -50
index c5236e71ba1..a4c64dda0a2 100644
@@ -205,27 +205,28 @@ void timer_get(QEMUFile *f, QEMUTimer *ts)
  * Not in vmstate.c to not add qemu-timer.c as dependency to vmstate.c
  */
 
-static int get_timer(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field)
+static bool load_timer(QEMUFile *f, void *pv, size_t size,
+                       const VMStateField *field, Error **errp)
 {
     QEMUTimer *v = pv;
     timer_get(f, v);
-    return 0;
+    return true;
 }
 
-static int put_timer(QEMUFile *f, void *pv, size_t size,
-                     const VMStateField *field, JSONWriter *vmdesc)
+static bool save_timer(QEMUFile *f, void *pv, size_t size,
+                       const VMStateField *field, JSONWriter *vmdesc,
+                       Error **errp)
 {
     QEMUTimer *v = pv;
     timer_put(f, v);
 
-    return 0;
+    return true;
 }
 
 const VMStateInfo vmstate_info_timer = {
     .name = "timer",
-    .get  = get_timer,
-    .put  = put_timer,
+    .load  = load_timer,
+    .save  = save_timer,
 };
 
 
@@ -297,7 +298,7 @@ static uint32_t get_validatable_capabilities_count(void)
     return result;
 }
 
-static int configuration_pre_save(void *opaque)
+static bool configuration_pre_save(void *opaque, Error **errp)
 {
     SaveState *state = opaque;
     const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
@@ -318,7 +319,7 @@ static int configuration_pre_save(void *opaque)
     }
     state->uuid = qemu_uuid;
 
-    return 0;
+    return true;
 }
 
 static void configuration_post_save(void *opaque)
@@ -330,7 +331,7 @@ static void configuration_post_save(void *opaque)
     state->caps_count = 0;
 }
 
-static int configuration_pre_load(void *opaque)
+static bool configuration_pre_load(void *opaque, Error **errp)
 {
     SaveState *state = opaque;
 
@@ -339,7 +340,7 @@ static int configuration_pre_load(void *opaque)
      * minimum possible value for this CPU.
      */
     state->target_page_bits = migration_legacy_page_bits();
-    return 0;
+    return true;
 }
 
 static bool configuration_validate_capabilities(SaveState *state)
@@ -376,28 +377,31 @@ static bool configuration_validate_capabilities(SaveState *state)
     return ret;
 }
 
-static int configuration_post_load(void *opaque, int version_id)
+static bool configuration_post_load(void *opaque, int version_id, Error **errp)
 {
     SaveState *state = opaque;
     const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
-    int ret = 0;
+    bool ok = true;
 
     if (strncmp(state->name, current_name, state->len) != 0) {
-        error_report("Machine type received is '%.*s' and local is '%s'",
-                     (int) state->len, state->name, current_name);
-        ret = -EINVAL;
+        error_setg(errp,
+                   "Machine type received is '%.*s' and local is '%s'",
+                   (int) state->len, state->name, current_name);
+        ok = false;
         goto out;
     }
 
     if (state->target_page_bits != qemu_target_page_bits()) {
-        error_report("Received TARGET_PAGE_BITS is %d but local is %d",
-                     state->target_page_bits, qemu_target_page_bits());
-        ret = -EINVAL;
+        error_setg(errp,
+                   "Received TARGET_PAGE_BITS is %d but local is %d",
+                   state->target_page_bits, qemu_target_page_bits());
+        ok = false;
         goto out;
     }
 
     if (!configuration_validate_capabilities(state)) {
-        ret = -EINVAL;
+        error_setg(errp, "Failed to validate capabilities");
+        ok = false;
         goto out;
     }
 
@@ -409,11 +413,12 @@ out:
     state->capabilities = NULL;
     state->caps_count = 0;
 
-    return ret;
+    return ok;
 }
 
-static int get_capability(QEMUFile *f, void *pv, size_t size,
-                          const VMStateField *field)
+static bool load_capability(QEMUFile *f, void *pv, size_t size,
+                            const VMStateField *field,
+                            Error **errp)
 {
     MigrationCapability *capability = pv;
     char capability_str[UINT8_MAX + 1];
@@ -426,15 +431,16 @@ static int get_capability(QEMUFile *f, void *pv, size_t size,
     for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
         if (!strcmp(MigrationCapability_str(i), capability_str)) {
             *capability = i;
-            return 0;
+            return true;
         }
     }
-    error_report("Received unknown capability %s", capability_str);
-    return -EINVAL;
+    error_setg(errp, "Received unknown capability %s", capability_str);
+    return false;
 }
 
-static int put_capability(QEMUFile *f, void *pv, size_t size,
-                          const VMStateField *field, JSONWriter *vmdesc)
+static bool save_capability(QEMUFile *f, void *pv, size_t size,
+                            const VMStateField *field, JSONWriter *vmdesc,
+                            Error **errp)
 {
     MigrationCapability *capability = pv;
     const char *capability_str = MigrationCapability_str(*capability);
@@ -443,13 +449,13 @@ static int put_capability(QEMUFile *f, void *pv, size_t size,
 
     qemu_put_byte(f, len);
     qemu_put_buffer(f, (uint8_t *)capability_str, len);
-    return 0;
+    return true;
 }
 
 static const VMStateInfo vmstate_info_capability = {
     .name = "capability",
-    .get  = get_capability,
-    .put  = put_capability,
+    .load = load_capability,
+    .save = save_capability,
 };
 
 /* The target-page-bits subsection is present only if the
@@ -539,9 +545,9 @@ static const VMStateDescription vmstate_uuid = {
 static const VMStateDescription vmstate_configuration = {
     .name = "configuration",
     .version_id = 1,
-    .pre_load = configuration_pre_load,
-    .post_load = configuration_post_load,
-    .pre_save = configuration_pre_save,
+    .pre_load_errp = configuration_pre_load,
+    .post_load_errp = configuration_post_load,
+    .pre_save_errp = configuration_pre_save,
     .post_save = configuration_post_save,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32(len, SaveState),
@@ -969,8 +975,13 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se, Error **errp)
         }
         return ret;
     }
-    return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id,
-                              errp);
+
+    if (!vmstate_load_vmsd(f, se->vmsd, se->opaque, se->load_version_id,
+                           errp)) {
+        return -EINVAL;
+    }
+
+    return 0;
 }
 
 static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
@@ -1028,8 +1039,6 @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se)
 static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
                         Error **errp)
 {
-    int ret;
-
     if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
         return 0;
     }
@@ -1050,10 +1059,8 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
     if (!se->vmsd) {
         vmstate_save_old_style(f, se, vmdesc);
     } else {
-        ret = vmstate_save_state(f, se->vmsd, se->opaque, vmdesc,
-                                 errp);
-        if (ret) {
-            return ret;
+        if (!vmstate_save_vmsd(f, se->vmsd, se->opaque, vmdesc, errp)) {
+            return -EINVAL;
         }
     }
 
@@ -1317,8 +1324,8 @@ static void qemu_savevm_send_configuration(MigrationState *s, QEMUFile *f)
         json_writer_start_object(vmdesc, "configuration");
     }
 
-    vmstate_save_state(f, &vmstate_configuration, &savevm_state,
-                       vmdesc, &local_err);
+    vmstate_save_vmsd(f, &vmstate_configuration, &savevm_state,
+                      vmdesc, &local_err);
     if (local_err) {
         error_report_err(local_err);
     }
@@ -2748,7 +2755,6 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type, Error **errp)
 static int qemu_loadvm_state_header(QEMUFile *f, Error **errp)
 {
     unsigned int v;
-    int ret;
 
     v = qemu_get_be32(f);
     if (v != QEMU_VM_FILE_MAGIC) {
@@ -2779,10 +2785,9 @@ static int qemu_loadvm_state_header(QEMUFile *f, Error **errp)
             return -EINVAL;
         }
 
-        ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0,
-                                 errp);
-        if (ret) {
-            return ret;
+        if (!vmstate_load_vmsd(f, &vmstate_configuration, &savevm_state, 0,
+                               errp)) {
+            return -EINVAL;
         }
     }
     return 0;
Signature · 1 line
2.52.0
#29 lore raw

PATCH v3 16/18 hw/s390x/css: drop use of .err_hint for vmstate

In reply to Vladimir Sementsov-Ogievskiy

That's the only usage through the whole base. Doesn't worth keeping the whole complexity. And 2.7 machines were long ago.

---
 hw/s390x/css.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
hw/s390x/css.c +8 -7
index 04ce3178a0b..df5f0bc27f1 100644
@@ -192,10 +192,6 @@ static const VMStateDescription vmstate_schdev_orb = {
 static int subch_dev_post_load(void *opaque, int version_id);
 static int subch_dev_pre_save(void *opaque);
 
-const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
-    " Likely reason: some sequences of plug and unplug  can break"
-    " migration for machine versions prior to  2.7 (known design flaw).";
-
 const VMStateDescription vmstate_subch_dev = {
     .name = "s390_subch_dev",
     .version_id = 1,
@@ -203,10 +199,15 @@ const VMStateDescription vmstate_subch_dev = {
     .post_load = subch_dev_post_load,
     .pre_save = subch_dev_pre_save,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT8_EQUAL(cssid, SubchDev, "Bug!"),
-        VMSTATE_UINT8_EQUAL(ssid, SubchDev, "Bug!"),
+        VMSTATE_UINT8_EQUAL(cssid, SubchDev, NULL),
+        VMSTATE_UINT8_EQUAL(ssid, SubchDev, NULL),
         VMSTATE_UINT16(migrated_schid, SubchDev),
-        VMSTATE_UINT16_EQUAL(devno, SubchDev, err_hint_devno),
+        /*
+         * If devno mismatch on target, it may be due to some
+         * sequences of plug and unplug breaks migration for
+         * machine versions prior to 2.7 (known design flaw).
+         */
+        VMSTATE_UINT16_EQUAL(devno, SubchDev, NULL),
         VMSTATE_BOOL(thinint_active, SubchDev),
         VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB),
         VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32),
Signature · 1 line
2.52.0
Reviewed-by on v3 16/18
#30 lore raw

Re: [PATCH v3 16/18] hw/s390x/css: drop use of .err_hint for vmstate

Quoted 8 lines · Vladimir Sementsov-Ogievskiy

That's the only usage through the whole base. Doesn't worth keeping the whole complexity. And 2.7 machines were long ago.

---
 hw/s390x/css.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Agreed that the two "Bug!" hints aren't helpful, and the devno hint is probably offering more confusion than anything else.

Quoted 35 lines
hw/s390x/css.c +8 -7
index 04ce3178a0b..df5f0bc27f1 100644
@@ -192,10 +192,6 @@ static const VMStateDescription vmstate_schdev_orb = {
 static int subch_dev_post_load(void *opaque, int version_id);
 static int subch_dev_pre_save(void *opaque);
 
-const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
-    " Likely reason: some sequences of plug and unplug  can break"
-    " migration for machine versions prior to  2.7 (known design flaw).";
-
 const VMStateDescription vmstate_subch_dev = {
     .name = "s390_subch_dev",
     .version_id = 1,
@@ -203,10 +199,15 @@ const VMStateDescription vmstate_subch_dev = {
     .post_load = subch_dev_post_load,
     .pre_save = subch_dev_pre_save,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT8_EQUAL(cssid, SubchDev, "Bug!"),
-        VMSTATE_UINT8_EQUAL(ssid, SubchDev, "Bug!"),
+        VMSTATE_UINT8_EQUAL(cssid, SubchDev, NULL),
+        VMSTATE_UINT8_EQUAL(ssid, SubchDev, NULL),
         VMSTATE_UINT16(migrated_schid, SubchDev),
-        VMSTATE_UINT16_EQUAL(devno, SubchDev, err_hint_devno),
+        /*
+         * If devno mismatch on target, it may be due to some
+         * sequences of plug and unplug breaks migration for
+         * machine versions prior to 2.7 (known design flaw).
+         */
+        VMSTATE_UINT16_EQUAL(devno, SubchDev, NULL),
         VMSTATE_BOOL(thinint_active, SubchDev),
         VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB),
         VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32),
#32 lore raw

PATCH v3 17/18 migration: drop VMStateField.err_hint

In reply to Vladimir Sementsov-Ogievskiy

The field is unused, all users of VMSTATE_*_EQUAL pass _err_hint=NULL.

---
 hw/block/fdc.c              |  2 +-
 hw/display/qxl.c            |  4 ++--
 hw/display/vga.c            |  2 +-
 hw/display/virtio-gpu.c     |  2 +-
 hw/display/vmware_vga.c     |  2 +-
 hw/i386/vmmouse.c           |  2 +-
 hw/ide/ahci.c               |  2 +-
 hw/intc/openpic.c           |  2 +-
 hw/intc/spapr_xive.c        |  2 +-
 hw/intc/xics.c              |  2 +-
 hw/intc/xive.c              |  2 +-
 hw/nvram/eeprom93xx.c       |  2 +-
 hw/pci/pci.c                |  2 +-
 hw/pci/pcie_aer.c           |  2 +-
 hw/ppc/spapr_iommu.c        |  2 +-
 hw/ppc/spapr_pci.c          |  4 ++--
 hw/ppc/spapr_vio.c          |  4 ++--
 hw/s390x/css.c              |  6 +++---
 hw/usb/hcd-uhci.c           |  2 +-
 include/migration/vmstate.h | 36 +++++++++++++++++-------------------
 migration/vmstate-types.c   | 15 ---------------
 target/ppc/machine.c        |  6 +++---
 22 files changed, 44 insertions(+), 61 deletions(-)
hw/block/fdc.c +1 -1
index 4585640af92..2c1681b7d0a 100644
@@ -1051,7 +1051,7 @@ const VMStateDescription vmstate_fdc = {
         VMSTATE_UINT8(config, FDCtrl),
         VMSTATE_UINT8(lock, FDCtrl),
         VMSTATE_UINT8(pwrd, FDCtrl),
-        VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl, NULL),
+        VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl),
         VMSTATE_STRUCT_ARRAY(drives, FDCtrl, MAX_FD, 1,
                              vmstate_fdrive, FDrive),
         VMSTATE_END_OF_LIST()
hw/display/qxl.c +2 -2
index 0551b38230b..a63b70f4ebc 100644
@@ -2442,12 +2442,12 @@ static const VMStateDescription qxl_vmstate = {
         VMSTATE_UINT32(last_release_offset, PCIQXLDevice),
         VMSTATE_UINT32(mode, PCIQXLDevice),
         VMSTATE_UINT32(ssd.unique, PCIQXLDevice),
-        VMSTATE_INT32_EQUAL(num_memslots, PCIQXLDevice, NULL),
+        VMSTATE_INT32_EQUAL(num_memslots, PCIQXLDevice),
         VMSTATE_STRUCT_ARRAY(guest_slots, PCIQXLDevice, NUM_MEMSLOTS, 0,
                              qxl_memslot, struct guest_slots),
         VMSTATE_STRUCT(guest_primary.surface, PCIQXLDevice, 0,
                        qxl_surface, QXLSurfaceCreate),
-        VMSTATE_INT32_EQUAL(ssd.num_surfaces, PCIQXLDevice, NULL),
+        VMSTATE_INT32_EQUAL(ssd.num_surfaces, PCIQXLDevice),
         VMSTATE_VARRAY_INT32(guest_surfaces.cmds, PCIQXLDevice,
                              ssd.num_surfaces, 0,
                              vmstate_info_uint64, uint64_t),
hw/display/vga.c +1 -1
index 59a65cbbff5..f936f47ad22 100644
@@ -2160,7 +2160,7 @@ const VMStateDescription vmstate_vga_common = {
         VMSTATE_BUFFER(palette, VGACommonState),
 
         VMSTATE_INT32(bank_offset, VGACommonState),
-        VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState, NULL),
+        VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState),
         VMSTATE_UINT16(vbe_index, VGACommonState),
         VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB),
         VMSTATE_UINT32(vbe_start_addr, VGACommonState),
hw/display/virtio-gpu.c +1 -1
index 643e91ca2a7..9aeeef16410 100644
@@ -1211,7 +1211,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
     .fields = (const VMStateField[]) {
         VMSTATE_INT32(parent_obj.enable, struct VirtIOGPU),
         VMSTATE_UINT32_EQUAL(parent_obj.conf.max_outputs,
-                             struct VirtIOGPU, NULL),
+                             struct VirtIOGPU),
         VMSTATE_STRUCT_VARRAY_UINT32(parent_obj.scanout, struct VirtIOGPU,
                                      parent_obj.conf.max_outputs, 1,
                                      vmstate_virtio_gpu_scanout,
hw/display/vmware_vga.c +1 -1
index ea7a9fca04e..d97a60cdf75 100644
@@ -1209,7 +1209,7 @@ static const VMStateDescription vmstate_vmware_vga_internal = {
     .minimum_version_id = 0,
     .post_load = vmsvga_post_load,
     .fields = (const VMStateField[]) {
-        VMSTATE_INT32_EQUAL(new_depth, struct vmsvga_state_s, NULL),
+        VMSTATE_INT32_EQUAL(new_depth, struct vmsvga_state_s),
         VMSTATE_INT32(enable, struct vmsvga_state_s),
         VMSTATE_INT32(config, struct vmsvga_state_s),
         VMSTATE_INT32(cursor.id, struct vmsvga_state_s),
hw/i386/vmmouse.c +1 -1
index c1aeeca0c9a..417c3aecb92 100644
@@ -278,7 +278,7 @@ static const VMStateDescription vmstate_vmmouse = {
     .minimum_version_id = 0,
     .post_load = vmmouse_post_load,
     .fields = (const VMStateField[]) {
-        VMSTATE_INT32_EQUAL(queue_size, VMMouseState, NULL),
+        VMSTATE_INT32_EQUAL(queue_size, VMMouseState),
         VMSTATE_UINT32_ARRAY(queue, VMMouseState, VMMOUSE_QUEUE_SIZE),
         VMSTATE_UINT16(nb_queue, VMMouseState),
         VMSTATE_UINT16(status, VMMouseState),
hw/ide/ahci.c +1 -1
index 08f51c8e361..c2b4432b947 100644
@@ -1795,7 +1795,7 @@ const VMStateDescription vmstate_ahci = {
         VMSTATE_UINT32(control_regs.impl, AHCIState),
         VMSTATE_UINT32(control_regs.version, AHCIState),
         VMSTATE_UINT32(idp_index, AHCIState),
-        VMSTATE_UINT32_EQUAL(ports, AHCIState, NULL),
+        VMSTATE_UINT32_EQUAL(ports, AHCIState),
         VMSTATE_END_OF_LIST()
     },
 };
hw/intc/openpic.c +1 -1
index cd353a04f53..99d2a1e65ec 100644
@@ -1477,7 +1477,7 @@ static const VMStateDescription vmstate_openpic = {
         VMSTATE_UINT32(max_irq, OpenPICState),
         VMSTATE_STRUCT_VARRAY_UINT32(src, OpenPICState, max_irq, 0,
                                      vmstate_openpic_irqsource, IRQSource),
-        VMSTATE_UINT32_EQUAL(nb_cpus, OpenPICState, NULL),
+        VMSTATE_UINT32_EQUAL(nb_cpus, OpenPICState),
         VMSTATE_STRUCT_VARRAY_UINT32(dst, OpenPICState, nb_cpus, 0,
                                      vmstate_openpic_irqdest, IRQDest),
         VMSTATE_STRUCT_ARRAY(timers, OpenPICState, OPENPIC_MAX_TMR, 0,
hw/intc/spapr_xive.c +1 -1
index 76ab476f59a..c30dace4e25 100644
@@ -578,7 +578,7 @@ static const VMStateDescription vmstate_spapr_xive = {
     .pre_save = vmstate_spapr_xive_pre_save,
     .post_load = NULL, /* handled at the machine level */
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT32_EQUAL(nr_irqs, SpaprXive, NULL),
+        VMSTATE_UINT32_EQUAL(nr_irqs, SpaprXive),
         VMSTATE_STRUCT_VARRAY_POINTER_UINT32(eat, SpaprXive, nr_irqs,
                                      vmstate_spapr_xive_eas, XiveEAS),
         VMSTATE_STRUCT_VARRAY_POINTER_UINT32(endt, SpaprXive, nr_ends,
hw/intc/xics.c +1 -1
index 1d40c4386d1..c0a252d0519 100644
@@ -668,7 +668,7 @@ static const VMStateDescription vmstate_ics = {
     .post_load = ics_post_load,
     .fields = (const VMStateField[]) {
         /* Sanity check */
-        VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL),
+        VMSTATE_UINT32_EQUAL(nr_irqs, ICSState),
 
         VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs,
                                              vmstate_ics_irq,
hw/intc/xive.c +1 -1
index d702b58bd0a..f473e6ac77e 100644
@@ -1565,7 +1565,7 @@ static const VMStateDescription vmstate_xive_source = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT32_EQUAL(nr_irqs, XiveSource, NULL),
+        VMSTATE_UINT32_EQUAL(nr_irqs, XiveSource),
         VMSTATE_VBUFFER_UINT32(status, XiveSource, 1, NULL, nr_irqs),
         VMSTATE_END_OF_LIST()
     },
hw/nvram/eeprom93xx.c +1 -1
index a8fd60a8fb5..73b3d248d44 100644
@@ -144,7 +144,7 @@ static const VMStateDescription vmstate_eeprom = {
         VMSTATE_UINT8(addrbits, eeprom_t),
         VMSTATE_UINT16_HACK_TEST(size, eeprom_t, is_old_eeprom_version),
         VMSTATE_UNUSED_TEST(is_old_eeprom_version, 1),
-        VMSTATE_UINT16_EQUAL_V(size, eeprom_t, EEPROM_VERSION, NULL),
+        VMSTATE_UINT16_EQUAL_V(size, eeprom_t, EEPROM_VERSION),
         VMSTATE_UINT16(data, eeprom_t),
         VMSTATE_VARRAY_UINT16_UNSAFE(contents, eeprom_t, size, 0,
                                      vmstate_info_uint16, uint16_t),
hw/pci/pci.c +1 -1
index 90d6d71efdc..8868d6ad387 100644
@@ -106,7 +106,7 @@ static const VMStateDescription vmstate_pcibus = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (const VMStateField[]) {
-        VMSTATE_INT32_EQUAL(nirq, PCIBus, NULL),
+        VMSTATE_INT32_EQUAL(nirq, PCIBus),
         VMSTATE_VARRAY_INT32(irq_count, PCIBus,
                              nirq, 0, vmstate_info_int32,
                              int32_t),
hw/pci/pcie_aer.c +1 -1
index 2c85a78fcde..22497b1fd65 100644
@@ -820,7 +820,7 @@ const VMStateDescription vmstate_pcie_aer_log = {
     .minimum_version_id = 1,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT16(log_num, PCIEAERLog),
-        VMSTATE_UINT16_EQUAL(log_max, PCIEAERLog, NULL),
+        VMSTATE_UINT16_EQUAL(log_max, PCIEAERLog),
         VMSTATE_VALIDATE("log_num <= log_max", pcie_aer_state_log_num_valid),
         VMSTATE_STRUCT_VARRAY_POINTER_UINT16(log, PCIEAERLog, log_num,
                               vmstate_pcie_aer_err, PCIEAERErr),
hw/ppc/spapr_iommu.c +1 -1
index c2432a0c00c..e6264b07854 100644
@@ -285,7 +285,7 @@ static const VMStateDescription vmstate_spapr_tce_table = {
     .post_load = spapr_tce_table_post_load,
     .fields = (const VMStateField []) {
         /* Sanity check */
-        VMSTATE_UINT32_EQUAL(liobn, SpaprTceTable, NULL),
+        VMSTATE_UINT32_EQUAL(liobn, SpaprTceTable),
 
         /* IOMMU state */
         VMSTATE_UINT32(mig_nb_table, SpaprTceTable),
hw/ppc/spapr_pci.c +2 -2
index 1dc3b02659f..c1d4b7806e4 100644
@@ -2051,7 +2051,7 @@ static const VMStateDescription vmstate_spapr_pci_lsi = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT32_EQUAL(irq, SpaprPciLsi, NULL),
+        VMSTATE_UINT32_EQUAL(irq, SpaprPciLsi),
 
         VMSTATE_END_OF_LIST()
     },
@@ -2129,7 +2129,7 @@ static const VMStateDescription vmstate_spapr_pci = {
     .post_save = spapr_pci_post_save,
     .post_load = spapr_pci_post_load,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT64_EQUAL(buid, SpaprPhbState, NULL),
+        VMSTATE_UINT64_EQUAL(buid, SpaprPhbState),
         VMSTATE_STRUCT_ARRAY(lsi_table, SpaprPhbState, PCI_NUM_PINS, 0,
                              vmstate_spapr_pci_lsi, SpaprPciLsi),
         VMSTATE_INT32(msi_devs_num, SpaprPhbState),
hw/ppc/spapr_vio.c +2 -2
index 501e82a7665..3f05081bada 100644
@@ -609,8 +609,8 @@ const VMStateDescription vmstate_spapr_vio = {
     .minimum_version_id = 1,
     .fields = (const VMStateField[]) {
         /* Sanity check */
-        VMSTATE_UINT32_EQUAL(reg, SpaprVioDevice, NULL),
-        VMSTATE_UINT32_EQUAL(irq, SpaprVioDevice, NULL),
+        VMSTATE_UINT32_EQUAL(reg, SpaprVioDevice),
+        VMSTATE_UINT32_EQUAL(irq, SpaprVioDevice),
 
         /* General VIO device state */
         VMSTATE_UINT64(signal_state, SpaprVioDevice),
hw/s390x/css.c +3 -3
index df5f0bc27f1..ecd28fed5c3 100644
@@ -199,15 +199,15 @@ const VMStateDescription vmstate_subch_dev = {
     .post_load = subch_dev_post_load,
     .pre_save = subch_dev_pre_save,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT8_EQUAL(cssid, SubchDev, NULL),
-        VMSTATE_UINT8_EQUAL(ssid, SubchDev, NULL),
+        VMSTATE_UINT8_EQUAL(cssid, SubchDev),
+        VMSTATE_UINT8_EQUAL(ssid, SubchDev),
         VMSTATE_UINT16(migrated_schid, SubchDev),
         /*
          * If devno mismatch on target, it may be due to some
          * sequences of plug and unplug breaks migration for
          * machine versions prior to 2.7 (known design flaw).
          */
-        VMSTATE_UINT16_EQUAL(devno, SubchDev, NULL),
+        VMSTATE_UINT16_EQUAL(devno, SubchDev),
         VMSTATE_BOOL(thinint_active, SubchDev),
         VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB),
         VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32),
hw/usb/hcd-uhci.c +1 -1
index b2224c7f766..a7b9fe13175 100644
@@ -364,7 +364,7 @@ static const VMStateDescription vmstate_uhci = {
     .post_load = uhci_post_load,
     .fields = (const VMStateField[]) {
         VMSTATE_PCI_DEVICE(dev, UHCIState),
-        VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState, NULL),
+        VMSTATE_UINT8_EQUAL(num_ports_vmstate, UHCIState),
         VMSTATE_STRUCT_ARRAY(ports, UHCIState, UHCI_PORTS, 1,
                              vmstate_uhci_port, UHCIPort),
         VMSTATE_UINT16(cmd, UHCIState),
include/migration/vmstate.h +17 -19
index 3c1d84f2d67..ca702765020 100644
@@ -182,7 +182,6 @@ typedef enum {
 
 struct VMStateField {
     const char *name;
-    const char *err_hint;
     size_t offset;
     size_t size;
     size_t start;
@@ -358,9 +357,8 @@ extern const VMStateInfo vmstate_info_qlist;
 }
 
 #define VMSTATE_SINGLE_FULL(_field, _state, _test, _version, _info,  \
-                            _type, _err_hint) {                      \
+                            _type) {                      \
     .name         = (stringify(_field)),                             \
-    .err_hint     = (_err_hint),                                     \
     .version_id   = (_version),                                      \
     .field_exists = (_test),                                         \
     .size         = sizeof(_type),                                   \
@@ -1012,35 +1010,35 @@ extern const VMStateInfo vmstate_info_qlist;
 
 #endif
 
-#define VMSTATE_UINT8_EQUAL(_f, _s, _err_hint)                        \
+#define VMSTATE_UINT8_EQUAL(_f, _s)                                   \
     VMSTATE_SINGLE_FULL(_f, _s, 0, 0,                                 \
-                        vmstate_info_uint8_equal, uint8_t, _err_hint)
+                        vmstate_info_uint8_equal, uint8_t)
 
-#define VMSTATE_UINT16_EQUAL(_f, _s, _err_hint)                       \
+#define VMSTATE_UINT16_EQUAL(_f, _s)                                  \
     VMSTATE_SINGLE_FULL(_f, _s, 0, 0,                                 \
-                        vmstate_info_uint16_equal, uint16_t, _err_hint)
+                        vmstate_info_uint16_equal, uint16_t)
 
-#define VMSTATE_UINT16_EQUAL_V(_f, _s, _v, _err_hint)                 \
+#define VMSTATE_UINT16_EQUAL_V(_f, _s, _v)                            \
     VMSTATE_SINGLE_FULL(_f, _s, 0,  _v,                               \
-                        vmstate_info_uint16_equal, uint16_t, _err_hint)
+                        vmstate_info_uint16_equal, uint16_t)
 
-#define VMSTATE_INT32_EQUAL(_f, _s, _err_hint)                        \
+#define VMSTATE_INT32_EQUAL(_f, _s)                                   \
     VMSTATE_SINGLE_FULL(_f, _s, 0, 0,                                 \
-                        vmstate_info_int32_equal, int32_t, _err_hint)
+                        vmstate_info_int32_equal, int32_t)
 
-#define VMSTATE_UINT32_EQUAL_V(_f, _s, _v, _err_hint)                 \
+#define VMSTATE_UINT32_EQUAL_V(_f, _s, _v)                            \
     VMSTATE_SINGLE_FULL(_f, _s, 0,  _v,                               \
-                        vmstate_info_uint32_equal, uint32_t, _err_hint)
+                        vmstate_info_uint32_equal, uint32_t)
 
-#define VMSTATE_UINT32_EQUAL(_f, _s, _err_hint)                       \
-    VMSTATE_UINT32_EQUAL_V(_f, _s, 0, _err_hint)
+#define VMSTATE_UINT32_EQUAL(_f, _s)                                  \
+    VMSTATE_UINT32_EQUAL_V(_f, _s, 0)
 
-#define VMSTATE_UINT64_EQUAL_V(_f, _s, _v, _err_hint)                 \
+#define VMSTATE_UINT64_EQUAL_V(_f, _s, _v)                            \
     VMSTATE_SINGLE_FULL(_f, _s, 0,  _v,                               \
-                        vmstate_info_uint64_equal, uint64_t, _err_hint)
+                        vmstate_info_uint64_equal, uint64_t)
 
-#define VMSTATE_UINT64_EQUAL(_f, _s, _err_hint)                       \
-    VMSTATE_UINT64_EQUAL_V(_f, _s, 0, _err_hint)
+#define VMSTATE_UINT64_EQUAL(_f, _s)                                  \
+    VMSTATE_UINT64_EQUAL_V(_f, _s, 0)
 
 #define VMSTATE_INT32_POSITIVE_LE(_f, _s)                             \
     VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_le, int32_t)
migration/vmstate-types.c +0 -15
index 89cb2114721..033a2685c8f 100644
@@ -131,9 +131,6 @@ static int get_int32_equal(QEMUFile *f, void *pv, size_t size,
         return 0;
     }
     error_report("%" PRIx32 " != %" PRIx32, *v, v2);
-    if (field->err_hint) {
-        error_printf("%s\n", field->err_hint);
-    }
     return -EINVAL;
 }
 
@@ -280,9 +277,6 @@ static int get_uint32_equal(QEMUFile *f, void *pv, size_t size,
         return 0;
     }
     error_report("%" PRIx32 " != %" PRIx32, *v, v2);
-    if (field->err_hint) {
-        error_printf("%s\n", field->err_hint);
-    }
     return -EINVAL;
 }
 
@@ -391,9 +385,6 @@ static int get_uint64_equal(QEMUFile *f, void *pv, size_t size,
         return 0;
     }
     error_report("%" PRIx64 " != %" PRIx64, *v, v2);
-    if (field->err_hint) {
-        error_printf("%s\n", field->err_hint);
-    }
     return -EINVAL;
 }
 
@@ -417,9 +408,6 @@ static int get_uint8_equal(QEMUFile *f, void *pv, size_t size,
         return 0;
     }
     error_report("%x != %x", *v, v2);
-    if (field->err_hint) {
-        error_printf("%s\n", field->err_hint);
-    }
     return -EINVAL;
 }
 
@@ -443,9 +431,6 @@ static int get_uint16_equal(QEMUFile *f, void *pv, size_t size,
         return 0;
     }
     error_report("%x != %x", *v, v2);
-    if (field->err_hint) {
-        error_printf("%s\n", field->err_hint);
-    }
     return -EINVAL;
 }
 
target/ppc/machine.c +3 -3
index 49cfdc6d674..9eae0ff647d 100644
@@ -564,7 +564,7 @@ static const VMStateDescription vmstate_tlb6xx = {
     .minimum_version_id = 1,
     .needed = tlb6xx_needed,
     .fields = (const VMStateField[]) {
-        VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL),
+        VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
         VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlb6, PowerPCCPU,
                                             env.nb_tlb,
                                             vmstate_tlb6xx_entry,
@@ -603,7 +603,7 @@ static const VMStateDescription vmstate_tlbemb = {
     .minimum_version_id = 1,
     .needed = tlbemb_needed,
     .fields = (const VMStateField[]) {
-        VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL),
+        VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
         VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbe, PowerPCCPU,
                                             env.nb_tlb,
                                             vmstate_tlbemb_entry,
@@ -639,7 +639,7 @@ static const VMStateDescription vmstate_tlbmas = {
     .minimum_version_id = 1,
     .needed = tlbmas_needed,
     .fields = (const VMStateField[]) {
-        VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU, NULL),
+        VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
         VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbm, PowerPCCPU,
                                             env.nb_tlb,
                                             vmstate_tlbmas_entry,
Signature · 1 line
2.52.0
Acked-by on v3 17/18
#33 lore raw

Re: [PATCH v3 17/18] migration: drop VMStateField.err_hint

Quoted 19 lines · Vladimir Sementsov-Ogievskiy

The field is unused, all users of VMSTATE_*_EQUAL pass _err_hint=NULL.

---
 hw/block/fdc.c              |  2 +-
 hw/display/qxl.c            |  4 ++--
 hw/display/vga.c            |  2 +-
 hw/display/virtio-gpu.c     |  2 +-
 hw/display/vmware_vga.c     |  2 +-
 hw/i386/vmmouse.c           |  2 +-
 hw/ide/ahci.c               |  2 +-
 hw/intc/openpic.c           |  2 +-
 hw/intc/spapr_xive.c        |  2 +-
 hw/intc/xics.c              |  2 +-
 hw/intc/xive.c              |  2 +-
 hw/nvram/eeprom93xx.c       |  2 +-
 hw/pci/pci.c                |  2 +-
 hw/pci/pcie_aer.c           |  2 +-
 hw/ppc/spapr_iommu.c        |  2 +-
Quoted 8 lines
 hw/ppc/spapr_pci.c          |  4 ++--
 hw/ppc/spapr_vio.c          |  4 ++--
 hw/s390x/css.c              |  6 +++---
 hw/usb/hcd-uhci.c           |  2 +-
 include/migration/vmstate.h | 36 +++++++++++++++++-------------------
 migration/vmstate-types.c   | 15 ---------------
 target/ppc/machine.c        |  6 +++---
 22 files changed, 44 insertions(+), 61 deletions(-)
#36 lore raw

PATCH v3 18/18 migration/vmstate-types: move to new migration APIs

In reply to Vladimir Sementsov-Ogievskiy
---
 migration/trace-events    |  24 +-
 migration/vmstate-types.c | 627 +++++++++++++++++++-------------------
 2 files changed, 327 insertions(+), 324 deletions(-)
migration/trace-events +12 -12
index 0b3f759ccc4..5fe3cc59c01 100644
@@ -73,20 +73,20 @@ vmstate_subsection_save_top(const char *idstr) "%s"
 vmstate_field_exists(const char *vmsd, const char *name, int field_version, int version, int result) "%s:%s field_version %d version %d result %d"
 
 # vmstate-types.c
-get_qtailq(const char *name, int version_id) "%s v%d"
-get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d"
-put_qtailq(const char *name, int version_id) "%s v%d"
-put_qtailq_end(const char *name, const char *reason) "%s %s"
+load_qtailq(const char *name, int version_id) "%s v%d"
+load_qtailq_end(const char *name) "%s"
+save_qtailq(const char *name, int version_id) "%s v%d"
+save_qtailq_end(const char *name) "%s"
 
-get_gtree(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d"
-get_gtree_end(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, int ret) "%s(%s/%s) %d"
-put_gtree(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d"
-put_gtree_end(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, int ret) "%s(%s/%s) %d"
+load_gtree(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d"
+load_gtree_end(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name) "%s(%s/%s)"
+save_gtree(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d"
+save_gtree_end(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name) "%s(%s/%s)"
 
-get_qlist(const char *field_name, const char *vmsd_name, int version_id) "%s(%s v%d)"
-get_qlist_end(const char *field_name, const char *vmsd_name) "%s(%s)"
-put_qlist(const char *field_name, const char *vmsd_name, int version_id) "%s(%s v%d)"
-put_qlist_end(const char *field_name, const char *vmsd_name) "%s(%s)"
+load_qlist(const char *field_name, const char *vmsd_name, int version_id) "%s(%s v%d)"
+load_qlist_end(const char *field_name, const char *vmsd_name) "%s(%s)"
+save_qlist(const char *field_name, const char *vmsd_name, int version_id) "%s(%s v%d)"
+save_qlist_end(const char *field_name, const char *vmsd_name) "%s(%s)"
 
 # qemu-file.c
 qemu_file_fclose(void) ""
migration/vmstate-types.c +315 -312

1,193 lines not on this page · raw message

Signature · 1 line
2.52.0
#38 lore raw
In reply to Vladimir Sementsov-Ogievskiy
Quoted 57 lines · Vladimir Sementsov-Ogievskiy

v3:
01,02: add r-bs by Fabiano and Peter
03: improve comment, add r-b by Peter
04: add r-bs by Fabiano and Peter
05: new, replacement for dropped "[PATCH v2 05/16] migration: vmstate_save/load_state(): stop tracing errors" 06: rebase on kept trace-points, keep r-b by Peter
07-09: add r-b by Peter
10: rebase on kept trace-points, keep r-b by Peter
11-12: add r-b by Peter
13: rebase ion kept traces and logic around trace_vmstate_load_field_error() already refactored, fix indent, drop r-b 14: add r-b by Peter
15: add r-b by Peter, use "else { if () {} }"
16-17: new
18: rebased on dropped err_hint, drop r-b

Vladimir Sementsov-Ogievskiy (18):
migration: vmstate_save_state_v: fix double error_setg
migration: make vmstate_save_state_v() static
migration: make .post_save() a void function
migration: vmstate_load_state(): add some newlines
migration: vmstate_save/load_state(): refactor tracing errors
migration: factor out vmstate_pre_save() from vmstate_save_state()
migration: factor out vmstate_save_field() from vmstate_save_state()
migration: factor out vmstate_pre_load() from vmstate_load_state()
migration: factor out vmstate_load_field() from vmstate_load_state()
migration: factor out vmstate_post_load() from vmstate_load_state()
migration: convert vmstate_subsection_save/load functions to bool
migration: VMStateInfo: introduce new handlers with errp
migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd() migration/cpr: move to new migration APIs migration/savevm: move to new migration APIs hw/s390x/css: drop use of .err_hint for vmstate
migration: drop VMStateField.err_hint migration/vmstate-types: move to new migration APIs

 docs/devel/migration/main.rst |   2 +-
 hw/block/fdc.c                |   2 +-
 hw/display/qxl.c              |   4 +-
 hw/display/vga.c              |   2 +-
 hw/display/virtio-gpu.c       |   2 +-
 hw/display/vmware_vga.c       |   2 +-
 hw/i386/vmmouse.c             |   2 +-
 hw/ide/ahci.c                 |   2 +-
 hw/intc/openpic.c             |   2 +-
 hw/intc/spapr_xive.c          |   2 +-
 hw/intc/xics.c                |   2 +-
 hw/intc/xive.c                |   2 +-
 hw/nvram/eeprom93xx.c         |   2 +-
 hw/pci/pci.c                  |   2 +-
 hw/pci/pcie_aer.c             |   2 +-
 hw/ppc/spapr_iommu.c          |   2 +-
 hw/ppc/spapr_pci.c            |   7 +-
 hw/ppc/spapr_vio.c            |   4 +-
 hw/s390x/css.c                |  15 +-
 hw/usb/hcd-uhci.c             |   2 +-
 include/migration/cpr.h       |   2 +-
 include/migration/vmstate.h   |  73 ++--
Quoted 8 lines
 migration/cpr.c               |  22 +-
 migration/savevm.c            | 108 +++---
 migration/trace-events        |  30 +-
 migration/vmstate-types.c     | 642 +++++++++++++++++-----------------
 migration/vmstate.c           | 354 +++++++++++--------
 target/arm/machine.c          |   4 +-
 target/ppc/machine.c          |   6 +-
 29 files changed, 691 insertions(+), 612 deletions(-)

Queued, thanks!

#39 lore raw
Quoted 68 lines · Fabiano Rosas
Quoted 57 lines · Vladimir Sementsov-Ogievskiy

v3:
01,02: add r-bs by Fabiano and Peter
03: improve comment, add r-b by Peter
04: add r-bs by Fabiano and Peter
05: new, replacement for dropped "[PATCH v2 05/16] migration: vmstate_save/load_state(): stop tracing errors" 06: rebase on kept trace-points, keep r-b by Peter
07-09: add r-b by Peter
10: rebase on kept trace-points, keep r-b by Peter
11-12: add r-b by Peter
13: rebase ion kept traces and logic around trace_vmstate_load_field_error() already refactored, fix indent, drop r-b 14: add r-b by Peter
15: add r-b by Peter, use "else { if () {} }"
16-17: new
18: rebased on dropped err_hint, drop r-b

Vladimir Sementsov-Ogievskiy (18):
migration: vmstate_save_state_v: fix double error_setg
migration: make vmstate_save_state_v() static
migration: make .post_save() a void function
migration: vmstate_load_state(): add some newlines
migration: vmstate_save/load_state(): refactor tracing errors
migration: factor out vmstate_pre_save() from vmstate_save_state()
migration: factor out vmstate_save_field() from vmstate_save_state()
migration: factor out vmstate_pre_load() from vmstate_load_state()
migration: factor out vmstate_load_field() from vmstate_load_state()
migration: factor out vmstate_post_load() from vmstate_load_state()
migration: convert vmstate_subsection_save/load functions to bool
migration: VMStateInfo: introduce new handlers with errp
migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd() migration/cpr: move to new migration APIs migration/savevm: move to new migration APIs hw/s390x/css: drop use of .err_hint for vmstate
migration: drop VMStateField.err_hint migration/vmstate-types: move to new migration APIs

  docs/devel/migration/main.rst |   2 +-
  hw/block/fdc.c                |   2 +-
  hw/display/qxl.c              |   4 +-
  hw/display/vga.c              |   2 +-
  hw/display/virtio-gpu.c       |   2 +-
  hw/display/vmware_vga.c       |   2 +-
  hw/i386/vmmouse.c             |   2 +-
  hw/ide/ahci.c                 |   2 +-
  hw/intc/openpic.c             |   2 +-
  hw/intc/spapr_xive.c          |   2 +-
  hw/intc/xics.c                |   2 +-
  hw/intc/xive.c                |   2 +-
  hw/nvram/eeprom93xx.c         |   2 +-
  hw/pci/pci.c                  |   2 +-
  hw/pci/pcie_aer.c             |   2 +-
  hw/ppc/spapr_iommu.c          |   2 +-
  hw/ppc/spapr_pci.c            |   7 +-
  hw/ppc/spapr_vio.c            |   4 +-
  hw/s390x/css.c                |  15 +-
  hw/usb/hcd-uhci.c             |   2 +-
  include/migration/cpr.h       |   2 +-
  include/migration/vmstate.h   |  73 ++--
Quoted 8 lines
  migration/cpr.c               |  22 +-
  migration/savevm.c            | 108 +++---
  migration/trace-events        |  30 +-
  migration/vmstate-types.c     | 642 +++++++++++++++++-----------------
  migration/vmstate.c           | 354 +++++++++++--------
  target/arm/machine.c          |   4 +-
  target/ppc/machine.c          |   6 +-
  29 files changed, 691 insertions(+), 612 deletions(-)
Quoted 1 line

Queued, thanks!

Hi! Aren't these patches lost?

Signature · 2 lines
Best regards,
Vladimir
#40 lore raw
Quoted 69 lines · Vladimir Sementsov-Ogievskiy
Quoted 68 lines · Fabiano Rosas
Quoted 65 lines · Vladimir Sementsov-Ogievskiy

v3:
01,02: add r-bs by Fabiano and Peter
03: improve comment, add r-b by Peter
04: add r-bs by Fabiano and Peter
05: new, replacement for dropped "[PATCH v2 05/16] migration: vmstate_save/load_state(): stop tracing errors" 06: rebase on kept trace-points, keep r-b by Peter
07-09: add r-b by Peter
10: rebase on kept trace-points, keep r-b by Peter
11-12: add r-b by Peter
13: rebase ion kept traces and logic around trace_vmstate_load_field_error() already refactored, fix indent, drop r-b 14: add r-b by Peter
15: add r-b by Peter, use "else { if () {} }"
16-17: new
18: rebased on dropped err_hint, drop r-b

Vladimir Sementsov-Ogievskiy (18):
migration: vmstate_save_state_v: fix double error_setg
migration: make vmstate_save_state_v() static
migration: make .post_save() a void function
migration: vmstate_load_state(): add some newlines
migration: vmstate_save/load_state(): refactor tracing errors
migration: factor out vmstate_pre_save() from vmstate_save_state()
migration: factor out vmstate_save_field() from vmstate_save_state()
migration: factor out vmstate_pre_load() from vmstate_load_state()
migration: factor out vmstate_load_field() from vmstate_load_state()
migration: factor out vmstate_post_load() from vmstate_load_state()
migration: convert vmstate_subsection_save/load functions to bool
migration: VMStateInfo: introduce new handlers with errp
migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd() migration/cpr: move to new migration APIs migration/savevm: move to new migration APIs hw/s390x/css: drop use of .err_hint for vmstate
migration: drop VMStateField.err_hint migration/vmstate-types: move to new migration APIs

  docs/devel/migration/main.rst |   2 +-
  hw/block/fdc.c                |   2 +-
  hw/display/qxl.c              |   4 +-
  hw/display/vga.c              |   2 +-
  hw/display/virtio-gpu.c       |   2 +-
  hw/display/vmware_vga.c       |   2 +-
  hw/i386/vmmouse.c             |   2 +-
  hw/ide/ahci.c                 |   2 +-
  hw/intc/openpic.c             |   2 +-
  hw/intc/spapr_xive.c          |   2 +-
  hw/intc/xics.c                |   2 +-
  hw/intc/xive.c                |   2 +-
  hw/nvram/eeprom93xx.c         |   2 +-
  hw/pci/pci.c                  |   2 +-
  hw/pci/pcie_aer.c             |   2 +-
  hw/ppc/spapr_iommu.c          |   2 +-
  hw/ppc/spapr_pci.c            |   7 +-
  hw/ppc/spapr_vio.c            |   4 +-
  hw/s390x/css.c                |  15 +-
  hw/usb/hcd-uhci.c             |   2 +-
  include/migration/cpr.h       |   2 +-
  include/migration/vmstate.h   |  73 ++--
  migration/cpr.c               |  22 +-
  migration/savevm.c            | 108 +++---
  migration/trace-events        |  30 +-
  migration/vmstate-types.c     | 642 +++++++++++++++++-----------------
  migration/vmstate.c           | 354 +++++++++++--------
  target/arm/machine.c          |   4 +-
  target/ppc/machine.c          |   6 +-
  29 files changed, 691 insertions(+), 612 deletions(-)
Quoted 4 lines
Quoted 1 line

Queued, thanks!

Hi! Aren't these patches lost?

Hi, no, they are in my staging branch, I'll post when the devel window reopens. Unfortunately the rust build issue took up the amount of time I had slotted for this and I had to put it on hold to work on other things.