On 03.03.26 18:22, Peter Xu wrote:
On Sat, Feb 21, 2026 at 12:02:04AM +0300, Vladimir Sementsov-Ogievskiy wrote:
Simplify vmstate_save_state() which is rather big, and simplify further
refactoring.
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Reviewed-by: Peter Xu <[email protected]>
One trivial thing..
---
migration/vmstate.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index b07bbdd366..810b131f18 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -430,6 +430,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();
I can come up with no reason a caller should pass in NULL for errp.. I
don't know why we assert(errp) so rarely in qemu for similar cases, but
iiuc that's what we really want here..
assert(errp) will not handle error_fatal case correctly. ERRP_GUARD guarantees,
that in case of errp == &error_fatal, the error message will contain what we
prepend by error_prepend().
So, current practice is add ERRP_GUARD in any function which want to use
error_prepend(errp, ..).
May be, we may have some additional macro
#define ERRP_GOOD(); which will assert that errp is not in (NULL, &error_fatal),
and then, use it here, but then we should check, that for the whole callers *
caller either pass errp and has ERRP_GOOD() macro, or pass local error object.
+
+ 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)
@@ -440,18 +460,8 @@ 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);
- return ret;
- }
- } else if (vmsd->pre_save) {
- ret = vmsd->pre_save(opaque);
- if (ret) {
- error_setg(errp, "pre-save failed: %s", vmsd->name);
- return ret;
- }
+ if (!vmstate_pre_save(vmsd, opaque, errp)) {
+ return -EINVAL;
}
trace_vmstate_save_state_pre_save_done(vmsd->name);
--
2.52.0
--
Best regards,
Vladimir
