migration: make .post_save() a void function
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. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Zhao Liu <zhao1.liu@intel.com> #rust Link: https://lore.kernel.org/qemu-devel/20260304212303.667141-4-vsementsov@yandex-team.ru Signed-off-by: Fabiano Rosas <farosas@suse.de>
Vladimir Sementsov-Ogievskiy committed
Mar 5, 2026 at 00:22 UTC
ef2045832ea414faa719d75a64bb20583c88353f
8 files changed
+23
-27
docs/devel/migration/main.rst
+1
-1
@@ -439,7 +439,7 @@ The functions to do that are inside a vmstate definition, and are called:
439
440
This function is called before we save the state of one device.
441
442
-- ``int (*post_save)(void *opaque);``
442
+- ``void (*post_save)(void *opaque);``
443
444
This function is called after we save the state of one device
445
(even upon failure, unless the call to pre_save returned an error).
hw/ppc/spapr_pci.c
+1
-2
@@ -2093,14 +2093,13 @@ static int spapr_pci_pre_save(void *opaque)
2093
return 0;
2094
}
2095
2096
-static int spapr_pci_post_save(void *opaque)
2096
+static void spapr_pci_post_save(void *opaque)
2097
{
2098
SpaprPhbState *sphb = opaque;
2099
2100
g_free(sphb->msi_devs);
2101
sphb->msi_devs = NULL;
2102
sphb->msi_devs_num = 0;
2103
- return 0;
2103
}
2104
2105
static int spapr_pci_post_load(void *opaque, int version_id)
include/migration/vmstate.h
+7
-1
@@ -223,7 +223,13 @@ struct VMStateDescription {
223
bool (*post_load_errp)(void *opaque, int version_id, Error **errp);
224
int (*pre_save)(void *opaque);
225
bool (*pre_save_errp)(void *opaque, Error **errp);
226
- int (*post_save)(void *opaque);
226
+
227
+ /*
228
+ * Unless .pre_save() fails, .post_save() is called after saving
229
+ * fields and subsections. It should not fail because at this
230
+ * point the state has potentially already been transferred.
231
+ */
232
+ void (*post_save)(void *opaque);
233
bool (*needed)(void *opaque);
234
bool (*dev_unplug_pending)(void *opaque);
235
migration/savevm.c
+1
-2
@@ -321,14 +321,13 @@ static int configuration_pre_save(void *opaque)
321
return 0;
322
}
323
324
-static int configuration_post_save(void *opaque)
324
+static void configuration_post_save(void *opaque)
325
{
326
SaveState *state = opaque;
327
328
g_free(state->capabilities);
329
state->capabilities = NULL;
330
state->caps_count = 0;
331
- return 0;
331
}
332
333
static int configuration_pre_load(void *opaque)
migration/vmstate.c
+3
-9
@@ -550,10 +550,7 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
550
if (ret) {
551
error_prepend(errp, "Save of field %s/%s failed: ",
552
vmsd->name, field->name);
553
- if (vmsd->post_save) {
554
- vmsd->post_save(opaque);
555
- }
556
- return ret;
553
+ goto out;
554
}
555
556
/* Compressed arrays only care about the first element */
@@ -578,12 +575,9 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
575
576
ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
577
578
+out:
579
if (vmsd->post_save) {
582
- int ps_ret = vmsd->post_save(opaque);
583
- if (!ret && ps_ret) {
584
- ret = ps_ret;
585
- error_setg(errp, "post-save failed: %s", vmsd->name);
586
- }
580
+ vmsd->post_save(opaque);
581
}
582
return ret;
583
}
rust/migration/src/migratable.rs
+2
-4
@@ -406,10 +406,8 @@ impl<T: ToMigrationStateShared> Migratable<T> {
406
Ok(())
407
}
408
409
- fn post_save(&self) -> Result<(), InvalidError> {
410
- let state = unsafe { Box::from_raw(self.migration_state.replace(ptr::null_mut())) };
411
- drop(state);
412
- Ok(())
409
+ fn post_save(&self) {
410
+ let _ = unsafe { Box::from_raw(self.migration_state.replace(ptr::null_mut())) };
411
}
412
413
fn pre_load(&self) -> Result<(), InvalidError> {
rust/migration/src/vmstate.rs
+7
-5
@@ -492,6 +492,11 @@ unsafe extern "C" fn vmstate_no_version_cb<
492
into_neg_errno(result)
493
}
494
495
+unsafe extern "C" fn vmstate_post_save_cb<T, F: for<'a> FnCall<(&'a T,), ()>>(opaque: *mut c_void) {
496
+ // SAFETY: the function is used in T's implementation of VMState.
497
+ F::call((unsafe { &*(opaque.cast::<T>()) },));
498
+}
499
+
500
unsafe extern "C" fn vmstate_post_load_cb<
501
T,
502
F: for<'a> FnCall<(&'a T, u8), Result<(), impl Into<Errno>>>,
@@ -597,12 +602,9 @@ impl<T> VMStateDescriptionBuilder<T> {
602
}
603
604
#[must_use]
600
- pub const fn post_save<F: for<'a> FnCall<(&'a T,), Result<(), impl Into<Errno>>>>(
601
- mut self,
602
- _f: &F,
603
- ) -> Self {
605
+ pub const fn post_save<F: for<'a> FnCall<(&'a T,), ()>>(mut self, _f: &F) -> Self {
606
self.0.post_save = if F::IS_SOME {
605
- Some(vmstate_no_version_cb::<T, F>)
607
+ Some(vmstate_post_save_cb::<T, F>)
608
} else {
609
None
610
};
target/arm/machine.c
+1
-3
@@ -998,7 +998,7 @@ static int cpu_pre_save(void *opaque)
998
return 0;
999
}
1000
1001
-static int cpu_post_save(void *opaque)
1001
+static void cpu_post_save(void *opaque)
1002
{
1003
ARMCPU *cpu = opaque;
1004
@@ -1008,8 +1008,6 @@ static int cpu_post_save(void *opaque)
1008
1009
cpu->cpreg_vmstate_indexes = NULL;
1010
cpu->cpreg_vmstate_values = NULL;
1011
-
1012
- return 0;
1011
}
1012
1013
static int cpu_pre_load(void *opaque)