public inbox for qemu-devel@nongnu.org
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
To: peterx@redhat.com
Cc: farosas@suse.de, vsementsov@yandex-team.ru,
	qemu-devel@nongnu.org,
	Pierrick Bouvier <pierrick.bouvier@linaro.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Harsh Prateek Bora <harshpb@linux.ibm.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-ppc@nongnu.org (open list:sPAPR (pseries)),
	qemu-arm@nongnu.org (open list:ARM TCG CPUs)
Subject: [PATCH v3 03/18] migration: make .post_save() a void function
Date: Thu,  5 Mar 2026 00:22:47 +0300	[thread overview]
Message-ID: <20260304212303.667141-4-vsementsov@yandex-team.ru> (raw)
In-Reply-To: <20260304212303.667141-1-vsementsov@yandex-team.ru>

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>
---
 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(-)

diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index 234d280249a..2de70507640 100644
--- a/docs/devel/migration/main.rst
+++ b/docs/devel/migration/main.rst
@@ -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).
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index ea998bdff15..1dc3b02659f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -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)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 3695afd483f..85838a49aee 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -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);
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 3a16c467b25..c5236e71ba1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -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)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 651c3fe0115..5111e7a141f 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -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;
 }
diff --git a/target/arm/machine.c b/target/arm/machine.c
index bbaae344492..de810220e25 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -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)
-- 
2.52.0



  parent reply	other threads:[~2026-03-04 21:24 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-04 21:22 [PATCH v3 00/18] migration: more bool+errp APIs Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 01/18] migration: vmstate_save_state_v: fix double error_setg Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 02/18] migration: make vmstate_save_state_v() static Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` Vladimir Sementsov-Ogievskiy [this message]
2026-03-06 23:20   ` [PATCH v3 03/18] migration: make .post_save() a void function Fabiano Rosas
2026-03-06 23:34     ` noreply77-demartz@thinocorp.com
2026-03-09 14:41     ` Zhao Liu
2026-03-04 21:22 ` [PATCH v3 04/18] migration: vmstate_load_state(): add some newlines Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 05/18] migration: vmstate_save/load_state(): refactor tracing errors Vladimir Sementsov-Ogievskiy
2026-03-05 16:59   ` Peter Xu
2026-03-04 21:22 ` [PATCH v3 06/18] migration: factor out vmstate_pre_save() from vmstate_save_state() Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 07/18] migration: factor out vmstate_save_field() " Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 08/18] migration: factor out vmstate_pre_load() from vmstate_load_state() Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 09/18] migration: factor out vmstate_load_field() " Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 10/18] migration: factor out vmstate_post_load() " Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 11/18] migration: convert vmstate_subsection_save/load functions to bool Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 12/18] migration: VMStateInfo: introduce new handlers with errp Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 13/18] migration: introduce vmstate_load_vmsd() and vmstate_save_vmsd() Vladimir Sementsov-Ogievskiy
2026-03-05 17:01   ` Peter Xu
2026-03-04 21:22 ` [PATCH v3 14/18] migration/cpr: move to new migration APIs Vladimir Sementsov-Ogievskiy
2026-03-04 21:22 ` [PATCH v3 15/18] migration/savevm: " Vladimir Sementsov-Ogievskiy
2026-03-04 21:23 ` [PATCH v3 16/18] hw/s390x/css: drop use of .err_hint for vmstate Vladimir Sementsov-Ogievskiy
2026-03-05  2:29   ` Eric Farman
2026-03-05 17:14   ` Peter Xu
2026-03-04 21:23 ` [PATCH v3 17/18] migration: drop VMStateField.err_hint Vladimir Sementsov-Ogievskiy
2026-03-05  2:39   ` Eric Farman
2026-03-05  5:18   ` Akihiko Odaki
2026-03-05 17:14   ` Peter Xu
2026-03-04 21:23 ` [PATCH v3 18/18] migration/vmstate-types: move to new migration APIs Vladimir Sementsov-Ogievskiy
2026-03-05 17:15   ` Peter Xu
2026-03-06 20:53 ` [PATCH v3 00/18] migration: more bool+errp APIs Fabiano Rosas
2026-03-24  7:22   ` Vladimir Sementsov-Ogievskiy
2026-03-24 12:42     ` Fabiano Rosas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260304212303.667141-4-vsementsov@yandex-team.ru \
    --to=vsementsov@yandex-team.ru \
    --cc=farosas@suse.de \
    --cc=harshpb@linux.ibm.com \
    --cc=npiggin@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=peterx@redhat.com \
    --cc=pierrick.bouvier@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox