* [PATCH v13 00/27] migration: propagate vTPM errors using Error objects
@ 2025-08-29 20:01 Arun Menon
2025-08-29 20:01 ` [PATCH v13 01/27] migration: push Error **errp into vmstate_subsection_load() Arun Menon
` (26 more replies)
0 siblings, 27 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé,
Stefan Berger
Hello,
Currently, when a migration of a VM with an encrypted vTPM
fails on the destination host (e.g., due to a mismatch in secret values),
the error message displayed on the source host is generic and unhelpful.
For example, a typical error looks like this:
"operation failed: job 'migration out' failed: Sibling indicated error 1.
operation failed: job 'migration in' failed: load of migration failed:
Input/output error"
This message does not provide any specific indication of a vTPM failure.
Such generic errors are logged using error_report(), which prints to
the console/monitor but does not make the detailed error accessible via
the QMP query-migrate command.
This series addresses the issue, by ensuring that specific TPM error
messages are propagated via the QEMU Error object.
To make this possible,
- A set of functions in the call stack is changed
to incorporate an Error object as an additional parameter.
- Also, the TPM backend makes use of a new hook called post_load_errp()
that explicitly passes an Error object.
It is organized as follows,
- Patches 1-23 focuses on pushing Error object into the functions
that are important in the call stack where TPM errors are observed.
We still need to make changes in rest of the functions in savevm.c
such that they also incorporate the errp object for propagating errors.
- Patches 12, 13, 20, are minor refactoring changes.
- Patch 24 removes error variant of vmstate_save_state() function.
- Patch 25 renames post_save() to cleanup_save()
- Patch 26 introduces the new variants of the hooks in VMStateDescription
structure. These hooks should be used in future implementations.
- Patch 27 focuses on changing the TPM backend such that the errors are
set in the Error object.
While this series focuses specifically on TPM error reporting during
live migration, it lays the groundwork for broader improvements.
A lot of methods in savevm.c that previously returned an integer now capture
errors in the Error object, enabling other modules to adopt the
post_load_errp hook in the future.
One such change previously attempted:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01727.html
Resolves: https://issues.redhat.com/browse/RHEL-82826
Signed-off-by: Arun Menon <armenon@redhat.com>
---
Changes in v13:
- Akihiko suggested to use error_report_err() instead of warn_report_err().
We must not prefix error messages with "warning:" unless the error is a non-critical
failure that can be logged while the program continues to function.
- Removed error_prepend() with ERRP_GUARD() where it is not required within
the series.
- Link to v12: https://lore.kernel.org/qemu-devel/20250821-propagate_tpm_error-v12-0-72b803e707dc@redhat.com
Changes in v12:
- Remove error_prepend() calls where no additional information is appended to
the error string. This also allows us to remove unnecessary ERRP_GUARD().
- Avoid ambiguity by propagating clear messages in errp.
- Add clarity to commit messages throughout the series.
- Link to v11: https://lore.kernel.org/qemu-devel/20250813-propagate_tpm_error-v11-0-b470a374b42d@redhat.com
Changes in v11:
- Remove unnecessary NULL check in postcopy_ram_listen_thread.
- Change error_warn to error_fatal or pass local_err wherever appropriate, because,
https://lore.kernel.org/qemu-devel/20250808080823.2638861-13-armbru@redhat.com/
Most changes are in patches 2,24.
- Link to v10: https://lore.kernel.org/qemu-devel/20250808-propagate_tpm_error-v10-0-3e81a1d419b2@redhat.com
Changes in v10:
- Remove the patch to propagate most recent error and the patch of refactoring
vmstate_save_state_v(): 23,24. They are not required because we intend to keep
the design as is.
- Added 2 new patches
- patch 25: Rename post_save() to cleanup_save() and make it void
- patch 20: Return -1 on memory allocation failure in ram.c
- Pass &error_warn or &error_fatal to capture error or exit on error.
- Link to v9: https://lore.kernel.org/qemu-devel/20250805-propagate_tpm_error-v9-0-123450810db7@redhat.com
Changes in v9:
- Re ordering patches such that error is reported in each one of them.
- format specifier enclosed in '' changed i.e. '%d' changed to %d
- Reporting errors where they were missed before. Setting errp to NULL
in case of retry.
- Link to v8: https://lore.kernel.org/qemu-devel/20250731-propagate_tpm_error-v8-0-28fd82fdfdb2@redhat.com
Changes in v8:
- 3 new patches added:
- patch 23:
- Changes the error propagation by returning the most recent error
to the caller when both save device state and post_save fails.
- patch 24:
- Refactors the vmstate_save_state_v() function by adding wrapper
functions to separate concerns.
- patch 25:
- Removes the error variant of the vmstate_save_state()
function introduced in commit 969298f9d7.
- Use ERRP_GUARD() where there is an errp dereference or an error_prepend call.
- Pass &error_warn in place of NULL, in vmstate_load_state() calls so
that the caller knows about the error.
- Remove unnecessary null check before setting errp. Dereferencing it is not required.
- Documentation for the new variants of post/pre save/load hooks added.
- Some patches, although they received a 'Reviewed-by' tag, have undergone few minor changes,
Patch 1 : removed extra space
Patch 2 : Commit message changed, refactoring the function to
always set errp and return.
Patch 8 : Commit message changed.
Patch 9 : use error_setg_errno instead of error_setg.
Patch 27 : use error_setg_errno instead of error_setg.
- Link to v7: https://lore.kernel.org/qemu-devel/20250725-propagate_tpm_error-v7-0-d52704443975@redhat.com
Changes in v7:
- Fix propagating errors in post_save_errp. The latest error encountered is
propagated.
- user-strings in error_prepend() calls now end with a ': ' so that the print is pretty.
- Change the order of one of the patches.
- Link to v6: https://lore.kernel.org/qemu-devel/20250721-propagate_tpm_error-v6-0-fef740e15e17@redhat.com
Changes in v6:
- Incorporated review comments from Daniel and Akihiko, related to few
semantic errors and improve error logging.
- Add one more patch that removes NULL checks after calling
qemu_file_get_return_path() because it does not fail.
- Link to v5: https://lore.kernel.org/qemu-devel/20250717-propagate_tpm_error-v5-0-1f406f88ee65@redhat.com
Changes in v5:
- Solve a bug that set errp even though it was not NULL, pointed out by Fabiano in v4.
- Link to v4: https://lore.kernel.org/qemu-devel/20250716-propagate_tpm_error-v4-0-7141902077c0@redhat.com
Changes in v4:
- Split the patches into smaller ones based on functions. Pass NULL in the
caller until errp is made available. Every function that has an
Error **errp object passed to it, ensures that it sets the errp object
in case of failure.
- A few more functions within loadvm_process_command() now handle errors using
the errp object. I've converted these for consistency, taking Daniel's
patches (link above) as a reference.
- Along with the post_load_errp() hook, other duplicate hooks are also introduced.
This will enable us to migrate to the newer versions eventually.
- Fix some semantic errors, like using error_propagate_prepend() in places where
we need to preserve existing behaviour of accumulating the error in local_err
and then propagating it to errp. This can be refactored in a later commit.
- Add more information in commit messages explaining the changes.
- Link to v3: https://lore.kernel.org/qemu-devel/20250702-propagate_tpm_error-v3-0-986d94540528@redhat.com
Changes in v3:
- Split the 2nd patch into 2. Introducing post_load_with_error() hook
has been separated from using it in the backends TPM module. This is
so that it can be acknowledged.
- Link to v2: https://lore.kernel.org/qemu-devel/20250627-propagate_tpm_error-v2-0-85990c89da29@redhat.com
Changes in v2:
- Combine the first two changes into one, focusing on passing the
Error object (errp) consistently through functions involved in
loading the VM's state. Other functions are not yet changed.
- As suggested in the review comment, add null checks for errp
before adding error messages, preventing crashes.
We also now correctly set errors when post-copy migration fails.
- In process_incoming_migration_co(), switch to error_prepend
instead of error_setg. This means we now null-check local_err in
the "fail" section before using it, preventing dereferencing issues.
- Link to v1: https://lore.kernel.org/qemu-devel/20250624-propagate_tpm_error-v1-0-2171487a593d@redhat.com
---
Arun Menon (27):
migration: push Error **errp into vmstate_subsection_load()
migration: push Error **errp into vmstate_load_state()
migration: push Error **errp into qemu_loadvm_state_header()
migration: push Error **errp into vmstate_load()
migration: push Error **errp into loadvm_process_command()
migration: push Error **errp into loadvm_handle_cmd_packaged()
migration: push Error **errp into qemu_loadvm_state()
migration: push Error **errp into qemu_load_device_state()
migration: push Error **errp into qemu_loadvm_state_main()
migration: push Error **errp into qemu_loadvm_section_start_full()
migration: push Error **errp into qemu_loadvm_section_part_end()
migration: Update qemu_file_get_return_path() docs and remove dead checks
migration: make loadvm_postcopy_handle_resume() void
migration: push Error **errp into ram_postcopy_incoming_init()
migration: push Error **errp into loadvm_postcopy_handle_advise()
migration: push Error **errp into loadvm_postcopy_handle_listen()
migration: push Error **errp into loadvm_postcopy_handle_run()
migration: push Error **errp into loadvm_postcopy_ram_handle_discard()
migration: push Error **errp into loadvm_handle_recv_bitmap()
migration: Return -1 on memory allocation failure in ram.c
migration: push Error **errp into loadvm_process_enable_colo()
migration: push Error **errp into loadvm_postcopy_handle_switchover_start()
migration: Capture error in postcopy_ram_listen_thread()
migration: Remove error variant of vmstate_save_state() function
migration: Rename post_save() to cleanup_save() and make it void
migration: Add error-parameterized function variants in VMSD struct
backends/tpm: Propagate vTPM error on migration failure
backends/tpm/tpm_emulator.c | 40 ++---
docs/devel/migration/main.rst | 21 ++-
hw/display/virtio-gpu.c | 5 +-
hw/pci/pci.c | 5 +-
hw/ppc/spapr_pci.c | 5 +-
hw/s390x/virtio-ccw.c | 4 +-
hw/scsi/spapr_vscsi.c | 6 +-
hw/vfio/pci.c | 9 +-
hw/virtio/virtio-mmio.c | 5 +-
hw/virtio/virtio-pci.c | 4 +-
hw/virtio/virtio.c | 13 +-
include/migration/colo.h | 2 +-
include/migration/vmstate.h | 20 ++-
migration/colo.c | 10 +-
migration/cpr.c | 6 +-
migration/migration.c | 33 ++---
migration/postcopy-ram.c | 9 +-
migration/postcopy-ram.h | 2 +-
migration/qemu-file.c | 1 -
migration/ram.c | 16 +-
migration/ram.h | 4 +-
migration/savevm.c | 334 ++++++++++++++++++++++++------------------
migration/savevm.h | 7 +-
migration/vmstate-types.c | 53 ++++---
migration/vmstate.c | 115 ++++++++++-----
target/arm/machine.c | 6 +-
tests/unit/test-vmstate.c | 83 +++++++++--
ui/vdagent.c | 8 +-
28 files changed, 516 insertions(+), 310 deletions(-)
---
base-commit: 59f504bb43366594e97cf496c9a9ccf59be00b73
change-id: 20250624-propagate_tpm_error-bf4ae6c23d30
Best regards,
--
Arun Menon <armenon@redhat.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v13 01/27] migration: push Error **errp into vmstate_subsection_load()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 02/27] migration: push Error **errp into vmstate_load_state() Arun Menon
` (25 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that vmstate_subsection_load() must report an error
in errp, in case of failure.
The errors are temporarily reported using error_report_err().
This is removed in the subsequent patches in this series,
when we are actually able to propagate the error to the calling
function using errp.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/vmstate.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 5feaa3244d259874f03048326b2497e7db32e47c..08f2b562e3a5ec0e186336215ccfb378a7675f0c 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -25,7 +25,7 @@ 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);
+ void *opaque, Error **errp);
/* Whether this field should exist for either save or load the VM? */
static bool
@@ -136,6 +136,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
{
const VMStateField *field = vmsd->fields;
int ret = 0;
+ Error *local_err = NULL;
trace_vmstate_load_state(vmsd->name, version_id);
if (version_id > vmsd->version_id) {
@@ -225,9 +226,10 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
field++;
}
assert(field->flags == VMS_END);
- ret = vmstate_subsection_load(f, vmsd, opaque);
+ ret = vmstate_subsection_load(f, vmsd, opaque, &local_err);
if (ret != 0) {
qemu_file_set_error(f, ret);
+ error_report_err(local_err);
return ret;
}
if (vmsd->post_load) {
@@ -566,7 +568,7 @@ vmstate_get_subsection(const VMStateDescription * const *sub,
}
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque)
+ void *opaque, Error **errp)
{
trace_vmstate_subsection_load(vmsd->name);
@@ -598,6 +600,8 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
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;
}
qemu_file_skip(f, 1); /* subsection */
@@ -608,6 +612,9 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
if (ret) {
trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(child)");
+ error_setg(errp,
+ "Loading VM subsection '%s' in '%s' failed: %d",
+ idstr, vmsd->name, ret);
return ret;
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 02/27] migration: push Error **errp into vmstate_load_state()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
2025-08-29 20:01 ` [PATCH v13 01/27] migration: push Error **errp into vmstate_subsection_load() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 03/27] migration: push Error **errp into qemu_loadvm_state_header() Arun Menon
` (24 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that vmstate_load_state() must report an error
in errp, in case of failure.
The errors are temporarily reported using error_report_err().
This is removed in the subsequent patches in this series,
when we are actually able to propagate the error to the calling
function using errp. Whereas, if we want the function to exit on
error, then error_fatal is passed.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
hw/display/virtio-gpu.c | 2 +-
hw/pci/pci.c | 3 ++-
hw/s390x/virtio-ccw.c | 2 +-
hw/scsi/spapr_vscsi.c | 4 ++-
hw/vfio/pci.c | 5 +++-
hw/virtio/virtio-mmio.c | 3 ++-
hw/virtio/virtio-pci.c | 2 +-
hw/virtio/virtio.c | 7 +++--
include/migration/vmstate.h | 2 +-
migration/cpr.c | 3 +--
migration/savevm.c | 8 ++++--
migration/vmstate-types.c | 28 ++++++++++++--------
migration/vmstate.c | 61 +++++++++++++++++++++++++++++--------------
tests/unit/test-vmstate.c | 63 ++++++++++++++++++++++++++++++++++++++-------
ui/vdagent.c | 5 +++-
15 files changed, 143 insertions(+), 55 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 0a1a625b0ea6cf26cb0d799171a57ed3d3ab2442..5dc31bc6bfb0272e29a4364ab10de2595a4bedf7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1343,7 +1343,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
}
/* load & apply scanout state */
- vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1);
+ vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1, &error_fatal);
return 0;
}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index c70b5ceebaf1f2b10768bd030526cbb518da2b8d..6be932d3bb67ff0c4808707db2a7b6378a90e82b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -934,7 +934,8 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
int pci_device_load(PCIDevice *s, QEMUFile *f)
{
int ret;
- ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
+ ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id,
+ &error_fatal);
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
return ret;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d2f85b39f30f7fc82e0c600144c0a958e1269b2c..6a9641a03d5d3a38a4de7ceb9deffc0cc303bcff 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1136,7 +1136,7 @@ static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f)
static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
{
VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1);
+ return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1, &error_fatal);
}
static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 20f70fb2729de78b9636a6b8c869695dab4f8902..da173f48676395cc02bd00cb3efa248afed4581b 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -642,15 +642,17 @@ static void *vscsi_load_request(QEMUFile *f, SCSIRequest *sreq)
VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(bus->qbus.parent);
vscsi_req *req;
int rc;
+ Error *local_err = NULL;
assert(sreq->tag < VSCSI_REQ_LIMIT);
req = &s->reqs[sreq->tag];
assert(!req->active);
memset(req, 0, sizeof(*req));
- rc = vmstate_load_state(f, &vmstate_spapr_vscsi_req, req, 1);
+ rc = vmstate_load_state(f, &vmstate_spapr_vscsi_req, req, 1, &local_err);
if (rc) {
fprintf(stderr, "VSCSI: failed loading request tag#%u\n", sreq->tag);
+ error_report_err(local_err);
return NULL;
}
assert(req->active);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 07257d0fa049b09fc296ac2279a6fafbdf93d277..1ecf150ed8218e8815655d3665f14ebac4382cf9 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2799,13 +2799,16 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
PCIDevice *pdev = &vdev->pdev;
pcibus_t old_addr[PCI_NUM_REGIONS - 1];
int bar, ret;
+ Error *local_err = NULL;
for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
old_addr[bar] = pdev->io_regions[bar].addr;
}
- ret = vmstate_load_state(f, &vmstate_vfio_pci_config, vdev, 1);
+ ret = vmstate_load_state(f, &vmstate_vfio_pci_config, vdev, 1,
+ &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 532c67107ba1d2978a76cf49f9cdc1de1dea3e11..0a688909fc606a3c9fde933667ae8c309ab527d0 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -34,6 +34,7 @@
#include "qemu/error-report.h"
#include "qemu/log.h"
#include "trace.h"
+#include "qapi/error.h"
static bool virtio_mmio_ioeventfd_enabled(DeviceState *d)
{
@@ -619,7 +620,7 @@ static int virtio_mmio_load_extra_state(DeviceState *opaque, QEMUFile *f)
{
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
- return vmstate_load_state(f, &vmstate_virtio_mmio, proxy, 1);
+ return vmstate_load_state(f, &vmstate_virtio_mmio, proxy, 1, &error_fatal);
}
static bool virtio_mmio_has_extra_state(DeviceState *opaque)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 767216d795998708f5716a23ae16c79cd90ff489..b04faa1e5c91b5cef40e54ec41d92422d16bfc13 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -161,7 +161,7 @@ static int virtio_pci_load_extra_state(DeviceState *d, QEMUFile *f)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
- return vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1);
+ return vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1, &error_fatal);
}
static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9a81ad912e013fc254899c4e55cff1f76a6112a4..018803c80d13107eb4e5e63914f9c1f837ab1b19 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3235,6 +3235,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+ Error *local_err = NULL;
/*
* We poison the endianness to ensure it does not get used before
@@ -3327,15 +3328,17 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
}
if (vdc->vmsd) {
- ret = vmstate_load_state(f, vdc->vmsd, vdev, version_id);
+ ret = vmstate_load_state(f, vdc->vmsd, vdev, version_id, &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
}
/* Subsections */
- ret = vmstate_load_state(f, &vmstate_virtio, vdev, 1);
+ ret = vmstate_load_state(f, &vmstate_virtio, vdev, 1, &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 1ff7bd9ac425ba67cd5ca7ad97bcf570f9e19abe..056781b1c21e737583f081594d9f88b32adfd674 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1196,7 +1196,7 @@ extern const VMStateInfo vmstate_info_qlist;
}
int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, int version_id);
+ void *opaque, int version_id, Error **errp);
int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc);
int vmstate_save_state_with_err(QEMUFile *f, const VMStateDescription *vmsd,
diff --git a/migration/cpr.c b/migration/cpr.c
index 42ad0b0d500e5de57faf0c6517e216b2d1c0cacf..8abb6db76d2474157f804ece4c35ebfc8c22d21a 100644
--- a/migration/cpr.c
+++ b/migration/cpr.c
@@ -233,9 +233,8 @@ int cpr_state_load(MigrationChannel *channel, Error **errp)
return -ENOTSUP;
}
- ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1);
+ ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1, errp);
if (ret) {
- error_setg(errp, "vmstate_load_state error %d", ret);
qemu_fclose(f);
return ret;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index fabbeb296ae987d0c06ba6dafda63720205fecfd..f5a1ab91016dba51f9cd1dee19a1c7ba31417423 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -969,7 +969,8 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
if (!se->vmsd) { /* Old style */
return se->ops->load_state(f, se->opaque, se->load_version_id);
}
- return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id);
+ return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id,
+ &error_fatal);
}
static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
@@ -2817,6 +2818,7 @@ static int qemu_loadvm_state_header(QEMUFile *f)
{
unsigned int v;
int ret;
+ Error *local_err = NULL;
v = qemu_get_be32(f);
if (v != QEMU_VM_FILE_MAGIC) {
@@ -2839,9 +2841,11 @@ static int qemu_loadvm_state_header(QEMUFile *f)
error_report("Configuration section missing");
return -EINVAL;
}
- ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0);
+ ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0,
+ &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
}
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index 741a588b7e18c6d37724b08a0101edc8bc74a0a5..c5cfd861e3aa5391245ef46bec9c90106c467e6e 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -19,6 +19,7 @@
#include "qemu/error-report.h"
#include "qemu/queue.h"
#include "trace.h"
+#include "qapi/error.h"
/* bool */
@@ -543,13 +544,17 @@ static int get_tmp(QEMUFile *f, void *pv, size_t size,
const VMStateField *field)
{
int ret;
+ Error *local_err = NULL;
const VMStateDescription *vmsd = field->vmsd;
int version_id = field->version_id;
void *tmp = g_malloc(size);
/* Writes the parent field which is at the start of the tmp */
*(void **)tmp = pv;
- ret = vmstate_load_state(f, vmsd, tmp, version_id);
+ ret = vmstate_load_state(f, vmsd, tmp, version_id, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
g_free(tmp);
return ret;
}
@@ -626,6 +631,7 @@ static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
const VMStateField *field)
{
int ret = 0;
+ Error *local_err = NULL;
const VMStateDescription *vmsd = field->vmsd;
/* size of a QTAILQ element */
size_t size = field->size;
@@ -649,8 +655,9 @@ static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
while (qemu_get_byte(f)) {
elm = g_malloc(size);
- ret = vmstate_load_state(f, vmsd, elm, version_id);
+ ret = vmstate_load_state(f, vmsd, elm, version_id, &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
@@ -772,6 +779,7 @@ static int get_gtree(QEMUFile *f, void *pv, size_t unused_size,
GTree *tree = *pval;
void *key, *val;
int ret = 0;
+ Error *local_err = NULL;
/* in case of direct key, the key vmsd can be {}, ie. check fields */
if (!direct_key && version_id > key_vmsd->version_id) {
@@ -803,18 +811,16 @@ static int get_gtree(QEMUFile *f, void *pv, size_t unused_size,
key = (void *)(uintptr_t)qemu_get_be64(f);
} else {
key = g_malloc0(key_size);
- ret = vmstate_load_state(f, key_vmsd, key, version_id);
+ ret = vmstate_load_state(f, key_vmsd, key, version_id, &local_err);
if (ret) {
- error_report("%s : failed to load %s (%d)",
- field->name, key_vmsd->name, ret);
+ error_report_err(local_err);
goto key_error;
}
}
val = g_malloc0(val_size);
- ret = vmstate_load_state(f, val_vmsd, val, version_id);
+ ret = vmstate_load_state(f, val_vmsd, val, version_id, &local_err);
if (ret) {
- error_report("%s : failed to load %s (%d)",
- field->name, val_vmsd->name, ret);
+ error_report_err(local_err);
goto val_error;
}
g_tree_insert(tree, key, val);
@@ -872,6 +878,7 @@ static int get_qlist(QEMUFile *f, void *pv, size_t unused_size,
const VMStateField *field)
{
int ret = 0;
+ Error *local_err = NULL;
const VMStateDescription *vmsd = field->vmsd;
/* size of a QLIST element */
size_t size = field->size;
@@ -892,10 +899,9 @@ static int get_qlist(QEMUFile *f, void *pv, size_t unused_size,
while (qemu_get_byte(f)) {
elm = g_malloc(size);
- ret = vmstate_load_state(f, vmsd, elm, version_id);
+ ret = vmstate_load_state(f, vmsd, elm, version_id, &local_err);
if (ret) {
- error_report("%s: failed to load %s (%d)", field->name,
- vmsd->name, ret);
+ error_report_err(local_err);
g_free(elm);
return ret;
}
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 08f2b562e3a5ec0e186336215ccfb378a7675f0c..8d1e9eb62bb9a7506604016b0ed858855e685776 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -132,30 +132,33 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
}
int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, int version_id)
+ void *opaque, int version_id, Error **errp)
{
const VMStateField *field = vmsd->fields;
int ret = 0;
- Error *local_err = NULL;
trace_vmstate_load_state(vmsd->name, version_id);
if (version_id > vmsd->version_id) {
- error_report("%s: incoming version_id %d is too new "
- "for local version_id %d",
- vmsd->name, version_id, vmsd->version_id);
+ 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);
return -EINVAL;
}
if (version_id < vmsd->minimum_version_id) {
- error_report("%s: incoming version_id %d is too old "
- "for local minimum version_id %d",
- vmsd->name, version_id, vmsd->minimum_version_id);
+ 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);
return -EINVAL;
}
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;
}
}
@@ -193,13 +196,21 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
if (inner_field->flags & VMS_STRUCT) {
ret = vmstate_load_state(f, inner_field->vmsd, curr_elem,
- inner_field->vmsd->version_id);
+ 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);
+ 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);
+ }
}
/* If we used a fake temp field.. free it now */
@@ -209,31 +220,40 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
if (ret >= 0) {
ret = qemu_file_get_error(f);
+ if (ret < 0) {
+ error_setg(errp,
+ "Failed to load %s state: stream error: %d",
+ vmsd->name, ret);
+ }
}
if (ret < 0) {
qemu_file_set_error(f, ret);
- error_report("Failed to load %s:%s", vmsd->name,
- field->name);
trace_vmstate_load_field_error(field->name, ret);
return ret;
}
}
} else if (field->flags & VMS_MUST_EXIST) {
- error_report("Input validation failed: %s/%s",
- vmsd->name, field->name);
+ error_setg(errp, "Input validation failed: %s/%s version_id: %d",
+ vmsd->name, field->name, vmsd->version_id);
return -1;
}
field++;
}
assert(field->flags == VMS_END);
- ret = vmstate_subsection_load(f, vmsd, opaque, &local_err);
+ ret = vmstate_subsection_load(f, vmsd, opaque, errp);
if (ret != 0) {
qemu_file_set_error(f, ret);
- error_report_err(local_err);
return ret;
}
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_end(vmsd->name, "end", ret);
return ret;
@@ -570,6 +590,7 @@ vmstate_get_subsection(const VMStateDescription * const *sub,
static int 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) {
@@ -609,12 +630,12 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
qemu_file_skip(f, len); /* idstr */
version_id = qemu_get_be32(f);
- ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
+ ret = vmstate_load_state(f, sub_vmsd, opaque, version_id, errp);
if (ret) {
trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(child)");
- error_setg(errp,
- "Loading VM subsection '%s' in '%s' failed: %d",
- idstr, vmsd->name, ret);
+ error_prepend(errp,
+ "Loading VM subsection '%s' in '%s' failed: %d: ",
+ idstr, vmsd->name, ret);
return ret;
}
}
diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 63f28f26f45691a70936d33e7341d16477a3471f..4ff0ab632f7e08b922dfcf565f31b0e63c17f59e 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -30,6 +30,7 @@
#include "../migration/savevm.h"
#include "qemu/module.h"
#include "io/channel-file.h"
+#include "qapi/error.h"
static int temp_fd;
@@ -108,14 +109,16 @@ static int load_vmstate_one(const VMStateDescription *desc, void *obj,
{
QEMUFile *f;
int ret;
+ Error *local_err = NULL;
f = open_test_file(true);
qemu_put_buffer(f, wire, size);
qemu_fclose(f);
f = open_test_file(false);
- ret = vmstate_load_state(f, desc, obj, version);
+ ret = vmstate_load_state(f, desc, obj, version, &local_err);
if (ret) {
+ error_report_err(local_err);
g_assert(qemu_file_get_error(f));
} else{
g_assert(!qemu_file_get_error(f));
@@ -355,6 +358,8 @@ static const VMStateDescription vmstate_versioned = {
static void test_load_v1(void)
{
+ Error *local_err = NULL;
+ int ret;
uint8_t buf[] = {
0, 0, 0, 10, /* a */
0, 0, 0, 30, /* c */
@@ -365,7 +370,10 @@ static void test_load_v1(void)
QEMUFile *loading = open_test_file(false);
TestStruct obj = { .b = 200, .e = 500, .f = 600 };
- vmstate_load_state(loading, &vmstate_versioned, &obj, 1);
+ ret = vmstate_load_state(loading, &vmstate_versioned, &obj, 1, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
g_assert(!qemu_file_get_error(loading));
g_assert_cmpint(obj.a, ==, 10);
g_assert_cmpint(obj.b, ==, 200);
@@ -378,6 +386,8 @@ static void test_load_v1(void)
static void test_load_v2(void)
{
+ Error *local_err = NULL;
+ int ret;
uint8_t buf[] = {
0, 0, 0, 10, /* a */
0, 0, 0, 20, /* b */
@@ -391,7 +401,10 @@ static void test_load_v2(void)
QEMUFile *loading = open_test_file(false);
TestStruct obj;
- vmstate_load_state(loading, &vmstate_versioned, &obj, 2);
+ ret = vmstate_load_state(loading, &vmstate_versioned, &obj, 2, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
g_assert_cmpint(obj.a, ==, 10);
g_assert_cmpint(obj.b, ==, 20);
g_assert_cmpint(obj.c, ==, 30);
@@ -467,6 +480,8 @@ static void test_save_skip(void)
static void test_load_noskip(void)
{
+ Error *local_err = NULL;
+ int ret;
uint8_t buf[] = {
0, 0, 0, 10, /* a */
0, 0, 0, 20, /* b */
@@ -480,7 +495,10 @@ static void test_load_noskip(void)
QEMUFile *loading = open_test_file(false);
TestStruct obj = { .skip_c_e = false };
- vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
+ ret = vmstate_load_state(loading, &vmstate_skipping, &obj, 2, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
g_assert(!qemu_file_get_error(loading));
g_assert_cmpint(obj.a, ==, 10);
g_assert_cmpint(obj.b, ==, 20);
@@ -493,6 +511,8 @@ static void test_load_noskip(void)
static void test_load_skip(void)
{
+ Error *local_err = NULL;
+ int ret;
uint8_t buf[] = {
0, 0, 0, 10, /* a */
0, 0, 0, 20, /* b */
@@ -504,7 +524,10 @@ static void test_load_skip(void)
QEMUFile *loading = open_test_file(false);
TestStruct obj = { .skip_c_e = true, .c = 300, .e = 500 };
- vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
+ ret = vmstate_load_state(loading, &vmstate_skipping, &obj, 2, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
g_assert(!qemu_file_get_error(loading));
g_assert_cmpint(obj.a, ==, 10);
g_assert_cmpint(obj.b, ==, 20);
@@ -744,6 +767,8 @@ static void test_save_q(void)
static void test_load_q(void)
{
+ int ret;
+ Error *local_err = NULL;
TestQtailq obj_q = {
.i16 = -512,
.i32 = 70000,
@@ -773,7 +798,10 @@ static void test_load_q(void)
TestQtailq tgt;
QTAILQ_INIT(&tgt.q);
- vmstate_load_state(fload, &vmstate_q, &tgt, 1);
+ ret = vmstate_load_state(fload, &vmstate_q, &tgt, 1, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
char eof = qemu_get_byte(fload);
g_assert(!qemu_file_get_error(fload));
g_assert_cmpint(tgt.i16, ==, obj_q.i16);
@@ -1115,6 +1143,8 @@ static void diff_iommu(TestGTreeIOMMU *iommu1, TestGTreeIOMMU *iommu2)
static void test_gtree_load_domain(void)
{
+ Error *local_err = NULL;
+ int ret;
TestGTreeDomain *dest_domain = g_new0(TestGTreeDomain, 1);
TestGTreeDomain *orig_domain = create_first_domain();
QEMUFile *fload, *fsave;
@@ -1127,7 +1157,11 @@ static void test_gtree_load_domain(void)
fload = open_test_file(false);
- vmstate_load_state(fload, &vmstate_domain, dest_domain, 1);
+ ret = vmstate_load_state(fload, &vmstate_domain, dest_domain, 1,
+ &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
eof = qemu_get_byte(fload);
g_assert(!qemu_file_get_error(fload));
g_assert_cmpint(orig_domain->id, ==, dest_domain->id);
@@ -1230,6 +1264,8 @@ static void test_gtree_save_iommu(void)
static void test_gtree_load_iommu(void)
{
+ Error *local_err = NULL;
+ int ret;
TestGTreeIOMMU *dest_iommu = g_new0(TestGTreeIOMMU, 1);
TestGTreeIOMMU *orig_iommu = create_iommu();
QEMUFile *fsave, *fload;
@@ -1241,7 +1277,10 @@ static void test_gtree_load_iommu(void)
qemu_fclose(fsave);
fload = open_test_file(false);
- vmstate_load_state(fload, &vmstate_iommu, dest_iommu, 1);
+ ret = vmstate_load_state(fload, &vmstate_iommu, dest_iommu, 1, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
eof = qemu_get_byte(fload);
g_assert(!qemu_file_get_error(fload));
g_assert_cmpint(orig_iommu->id, ==, dest_iommu->id);
@@ -1363,6 +1402,8 @@ static void test_save_qlist(void)
static void test_load_qlist(void)
{
+ Error *local_err = NULL;
+ int ret;
QEMUFile *fsave, *fload;
TestQListContainer *orig_container = alloc_container();
TestQListContainer *dest_container = g_new0(TestQListContainer, 1);
@@ -1376,7 +1417,11 @@ static void test_load_qlist(void)
qemu_fclose(fsave);
fload = open_test_file(false);
- vmstate_load_state(fload, &vmstate_container, dest_container, 1);
+ ret = vmstate_load_state(fload, &vmstate_container, dest_container, 1,
+ &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
eof = qemu_get_byte(fload);
g_assert(!qemu_file_get_error(fload));
g_assert_cmpint(eof, ==, QEMU_VM_EOF);
diff --git a/ui/vdagent.c b/ui/vdagent.c
index c0746fe5b168fdc7aeb4866de2ba0c3387566649..bc3c77f01332a4b594cba00f3f9f9a5bca4d739a 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -1001,6 +1001,7 @@ static int get_cbinfo(QEMUFile *f, void *pv, size_t size,
VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(pv);
struct CBInfoArray cbinfo = {};
int i, ret;
+ Error *local_err = NULL;
if (!have_clipboard(vd)) {
return 0;
@@ -1008,8 +1009,10 @@ static int get_cbinfo(QEMUFile *f, void *pv, size_t size,
vdagent_clipboard_peer_register(vd);
- ret = vmstate_load_state(f, &vmstate_cbinfo_array, &cbinfo, 0);
+ ret = vmstate_load_state(f, &vmstate_cbinfo_array, &cbinfo, 0,
+ &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 03/27] migration: push Error **errp into qemu_loadvm_state_header()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
2025-08-29 20:01 ` [PATCH v13 01/27] migration: push Error **errp into vmstate_subsection_load() Arun Menon
2025-08-29 20:01 ` [PATCH v13 02/27] migration: push Error **errp into vmstate_load_state() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 04/27] migration: push Error **errp into vmstate_load() Arun Menon
` (23 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_state_header() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index f5a1ab91016dba51f9cd1dee19a1c7ba31417423..0c445a957fc99f826e6753ed3795bcdd51f1e3f5 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2814,38 +2814,43 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
return 0;
}
-static int qemu_loadvm_state_header(QEMUFile *f)
+static int qemu_loadvm_state_header(QEMUFile *f, Error **errp)
{
unsigned int v;
int ret;
- Error *local_err = NULL;
v = qemu_get_be32(f);
if (v != QEMU_VM_FILE_MAGIC) {
- error_report("Not a migration stream");
+ error_setg(errp, "Not a migration stream, magic: %x != %x",
+ v, QEMU_VM_FILE_MAGIC);
return -EINVAL;
}
v = qemu_get_be32(f);
if (v == QEMU_VM_FILE_VERSION_COMPAT) {
- error_report("SaveVM v2 format is obsolete and don't work anymore");
+ error_setg(errp,
+ "SaveVM v2 format is obsolete and no longer supported");
+
return -ENOTSUP;
}
if (v != QEMU_VM_FILE_VERSION) {
- error_report("Unsupported migration stream version");
+ error_setg(errp, "Unsupported migration stream version, "
+ "file version %x != %x",
+ v, QEMU_VM_FILE_VERSION);
return -ENOTSUP;
}
if (migrate_get_current()->send_configuration) {
- if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
- error_report("Configuration section missing");
+ v = qemu_get_byte(f);
+ if (v != QEMU_VM_CONFIGURATION) {
+ error_setg(errp, "Configuration section missing, %x != %x",
+ v, QEMU_VM_CONFIGURATION);
return -EINVAL;
}
- ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0,
- &local_err);
+ ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0,
+ errp);
if (ret) {
- error_report_err(local_err);
return ret;
}
}
@@ -3121,8 +3126,9 @@ int qemu_loadvm_state(QEMUFile *f)
qemu_loadvm_thread_pool_create(mis);
- ret = qemu_loadvm_state_header(f);
+ ret = qemu_loadvm_state_header(f, &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 04/27] migration: push Error **errp into vmstate_load()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (2 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 03/27] migration: push Error **errp into qemu_loadvm_state_header() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 05/27] migration: push Error **errp into loadvm_process_command() Arun Menon
` (22 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that vmstate_load() must report an error
in errp, in case of failure.
The errors are temporarily reported using error_report_err().
This is removed in the subsequent patches in this series
when we are actually able to propagate the error to the calling
function.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 0c445a957fc99f826e6753ed3795bcdd51f1e3f5..40dae406db22dc9ddbff2f5ada92a32626b58e3b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -963,14 +963,20 @@ void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
}
}
-static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
+static int vmstate_load(QEMUFile *f, SaveStateEntry *se, Error **errp)
{
+ int ret;
trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
if (!se->vmsd) { /* Old style */
- return se->ops->load_state(f, se->opaque, se->load_version_id);
+ ret = se->ops->load_state(f, se->opaque, se->load_version_id);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load vmstate version_id: %d, ret: %d",
+ se->load_version_id, ret);
+ }
+ return ret;
}
return vmstate_load_state(f, se->vmsd, se->opaque, se->load_version_id,
- &error_fatal);
+ errp);
}
static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
@@ -2692,6 +2698,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
SaveStateEntry *se;
char idstr[256];
int ret;
+ Error *local_err = NULL;
/* Read section start */
section_id = qemu_get_be32(f);
@@ -2741,10 +2748,11 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
}
- ret = vmstate_load(f, se);
+ ret = vmstate_load(f, se, &local_err);
if (ret < 0) {
error_report("error while loading state for instance 0x%"PRIx32" of"
" device '%s'", instance_id, idstr);
+ error_report_err(local_err);
return ret;
}
@@ -2769,6 +2777,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
uint32_t section_id;
SaveStateEntry *se;
int ret;
+ Error *local_err = NULL;
section_id = qemu_get_be32(f);
@@ -2794,10 +2803,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
}
- ret = vmstate_load(f, se);
+ ret = vmstate_load(f, se, &local_err);
if (ret < 0) {
- error_report("error while loading state section id %d(%s)",
- section_id, se->idstr);
+ error_report_err(local_err);
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 05/27] migration: push Error **errp into loadvm_process_command()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (3 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 04/27] migration: push Error **errp into vmstate_load() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 06/27] migration: push Error **errp into loadvm_handle_cmd_packaged() Arun Menon
` (21 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_process_command() must report an error
in errp, in case of failure.
The errors are temporarily reported using error_report_err().
This is removed in the subsequent patches in this series
when we are actually able to propagate the error to the calling
function.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 86 +++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 63 insertions(+), 23 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 40dae406db22dc9ddbff2f5ada92a32626b58e3b..0e9935aea3e1a214cfa0ddd0e30fa5cf925373e3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2546,32 +2546,37 @@ static int loadvm_postcopy_handle_switchover_start(void)
* LOADVM_QUIT All good, but exit the loop
* <0 Error
*/
-static int loadvm_process_command(QEMUFile *f)
+static int loadvm_process_command(QEMUFile *f, Error **errp)
{
MigrationIncomingState *mis = migration_incoming_get_current();
uint16_t cmd;
uint16_t len;
uint32_t tmp32;
+ int ret;
cmd = qemu_get_be16(f);
len = qemu_get_be16(f);
/* Check validity before continue processing of cmds */
- if (qemu_file_get_error(f)) {
- return qemu_file_get_error(f);
+ ret = qemu_file_get_error(f);
+ if (ret) {
+ error_setg(errp,
+ "Failed to load VM process command: stream error: %d",
+ ret);
+ return ret;
}
if (cmd >= MIG_CMD_MAX || cmd == MIG_CMD_INVALID) {
- error_report("MIG_CMD 0x%x unknown (len 0x%x)", cmd, len);
+ error_setg(errp, "MIG_CMD 0x%x unknown (len 0x%x)", cmd, len);
return -EINVAL;
}
trace_loadvm_process_command(mig_cmd_args[cmd].name, len);
if (mig_cmd_args[cmd].len != -1 && mig_cmd_args[cmd].len != len) {
- error_report("%s received with bad length - expecting %zu, got %d",
- mig_cmd_args[cmd].name,
- (size_t)mig_cmd_args[cmd].len, len);
+ error_setg(errp, "%s received with bad length - expecting %zu, got %d",
+ mig_cmd_args[cmd].name,
+ (size_t)mig_cmd_args[cmd].len, len);
return -ERANGE;
}
@@ -2584,7 +2589,7 @@ static int loadvm_process_command(QEMUFile *f)
}
mis->to_src_file = qemu_file_get_return_path(f);
if (!mis->to_src_file) {
- error_report("CMD_OPEN_RETURN_PATH failed");
+ error_setg(errp, "CMD_OPEN_RETURN_PATH failed");
return -1;
}
@@ -2594,11 +2599,10 @@ static int loadvm_process_command(QEMUFile *f)
* been created.
*/
if (migrate_switchover_ack() && !mis->switchover_ack_pending_num) {
- int ret = migrate_send_rp_switchover_ack(mis);
+ ret = migrate_send_rp_switchover_ack(mis);
if (ret) {
- error_report(
- "Could not send switchover ack RP MSG, err %d (%s)", ret,
- strerror(-ret));
+ error_setg_errno(errp, -ret,
+ "Could not send switchover ack RP MSG");
return ret;
}
}
@@ -2608,39 +2612,71 @@ static int loadvm_process_command(QEMUFile *f)
tmp32 = qemu_get_be32(f);
trace_loadvm_process_command_ping(tmp32);
if (!mis->to_src_file) {
- error_report("CMD_PING (0x%x) received with no return path",
- tmp32);
+ error_setg(errp, "CMD_PING (0x%x) received with no return path",
+ tmp32);
return -1;
}
migrate_send_rp_pong(mis, tmp32);
break;
case MIG_CMD_PACKAGED:
- return loadvm_handle_cmd_packaged(mis);
+ ret = loadvm_handle_cmd_packaged(mis);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_POSTCOPY_ADVISE:
- return loadvm_postcopy_handle_advise(mis, len);
+ ret = loadvm_postcopy_handle_advise(mis, len);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_POSTCOPY_LISTEN:
- return loadvm_postcopy_handle_listen(mis);
+ ret = loadvm_postcopy_handle_listen(mis);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_POSTCOPY_RUN:
- return loadvm_postcopy_handle_run(mis);
+ ret = loadvm_postcopy_handle_run(mis);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_POSTCOPY_RAM_DISCARD:
- return loadvm_postcopy_ram_handle_discard(mis, len);
+ ret = loadvm_postcopy_ram_handle_discard(mis, len);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_POSTCOPY_RESUME:
return loadvm_postcopy_handle_resume(mis);
case MIG_CMD_RECV_BITMAP:
- return loadvm_handle_recv_bitmap(mis, len);
+ ret = loadvm_handle_recv_bitmap(mis, len);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_ENABLE_COLO:
- return loadvm_process_enable_colo(mis);
+ ret = loadvm_process_enable_colo(mis);
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
case MIG_CMD_SWITCHOVER_START:
- return loadvm_postcopy_handle_switchover_start();
+ ret = loadvm_postcopy_handle_switchover_start();
+ if (ret < 0) {
+ error_setg(errp, "Failed to load device state command: %d", ret);
+ }
+ return ret;
}
return 0;
@@ -3049,6 +3085,7 @@ int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
{
uint8_t section_type;
int ret = 0;
+ Error *local_err = NULL;
retry:
while (true) {
@@ -3076,7 +3113,10 @@ retry:
}
break;
case QEMU_VM_COMMAND:
- ret = loadvm_process_command(f);
+ ret = loadvm_process_command(f, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
trace_qemu_loadvm_state_section_command(ret);
if ((ret < 0) || (ret == LOADVM_QUIT)) {
goto out;
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 06/27] migration: push Error **errp into loadvm_handle_cmd_packaged()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (4 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 05/27] migration: push Error **errp into loadvm_process_command() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state() Arun Menon
` (20 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_handle_cmd_packaged() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 0e9935aea3e1a214cfa0ddd0e30fa5cf925373e3..de5671ffd1cd06e728227a3056c3f895d3a6e6f3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2405,7 +2405,7 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
* Returns: Negative values on error
*
*/
-static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
+static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
{
int ret;
size_t length;
@@ -2415,7 +2415,7 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
trace_loadvm_handle_cmd_packaged(length);
if (length > MAX_VM_CMD_PACKAGED_SIZE) {
- error_report("Unreasonably large packaged state: %zu", length);
+ error_setg(errp, "Unreasonably large packaged state: %zu", length);
return -1;
}
@@ -2426,8 +2426,8 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
length);
if (ret != length) {
object_unref(OBJECT(bioc));
- error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%zu",
- ret, length);
+ error_setg(errp, "CMD_PACKAGED: Buffer receive fail ret=%d length=%zu",
+ ret, length);
return (ret < 0) ? ret : -EAGAIN;
}
bioc->usage += length;
@@ -2457,6 +2457,9 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
} while (1);
ret = qemu_loadvm_state_main(packf, mis);
+ if (ret < 0) {
+ error_setg(errp, "VM state load failed: %d", ret);
+ }
trace_loadvm_handle_cmd_packaged_main(ret);
qemu_fclose(packf);
object_unref(OBJECT(bioc));
@@ -2620,11 +2623,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
break;
case MIG_CMD_PACKAGED:
- ret = loadvm_handle_cmd_packaged(mis);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_handle_cmd_packaged(mis, errp);
case MIG_CMD_POSTCOPY_ADVISE:
ret = loadvm_postcopy_handle_advise(mis, len);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (5 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 06/27] migration: push Error **errp into loadvm_handle_cmd_packaged() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-30 5:58 ` Akihiko Odaki
2025-08-29 20:01 ` [PATCH v13 08/27] migration: push Error **errp into qemu_load_device_state() Arun Menon
` (19 subsequent siblings)
26 siblings, 1 reply; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_state() must report an error
in errp, in case of failure.
When postcopy live migration runs, the device states are loaded by
both the qemu coroutine process_incoming_migration_co() and the
postcopy_ram_listen_thread(). Therefore, it is important that the
coroutine also reports the error in case of failure, with
error_report_err(). Otherwise, the source qemu will not display
any errors before going into the postcopy pause state.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/migration.c | 9 +++++----
migration/savevm.c | 30 ++++++++++++++++++------------
migration/savevm.h | 2 +-
3 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
MIGRATION_STATUS_ACTIVE);
mis->loadvm_co = qemu_coroutine_self();
- ret = qemu_loadvm_state(mis->from_src_file);
+ ret = qemu_loadvm_state(mis->from_src_file, &local_err);
mis->loadvm_co = NULL;
trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
@@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
}
if (ret < 0) {
- error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
+ error_prepend(&local_err, "load of migration failed: %s: ",
+ strerror(-ret));
goto fail;
}
@@ -924,13 +925,13 @@ fail:
migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_FAILED);
migrate_set_error(s, local_err);
- error_free(local_err);
+ error_report_err(local_err);
migration_incoming_state_destroy();
if (mis->exit_on_error) {
WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
- error_report_err(s->error);
+ error_free(s->error);
s->error = NULL;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index de5671ffd1cd06e728227a3056c3f895d3a6e6f3..0087fca15ce108685667d3808350d80d37b807b1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3159,28 +3159,24 @@ out:
return ret;
}
-int qemu_loadvm_state(QEMUFile *f)
+int qemu_loadvm_state(QEMUFile *f, Error **errp)
{
MigrationState *s = migrate_get_current();
MigrationIncomingState *mis = migration_incoming_get_current();
- Error *local_err = NULL;
int ret;
- if (qemu_savevm_state_blocked(&local_err)) {
- error_report_err(local_err);
+ if (qemu_savevm_state_blocked(errp)) {
return -EINVAL;
}
qemu_loadvm_thread_pool_create(mis);
- ret = qemu_loadvm_state_header(f, &local_err);
+ ret = qemu_loadvm_state_header(f, errp);
if (ret) {
- error_report_err(local_err);
return ret;
}
- if (qemu_loadvm_state_setup(f, &local_err) != 0) {
- error_report_err(local_err);
+ if (qemu_loadvm_state_setup(f, errp) != 0) {
return -EINVAL;
}
@@ -3191,6 +3187,9 @@ int qemu_loadvm_state(QEMUFile *f)
cpu_synchronize_all_pre_loadvm();
ret = qemu_loadvm_state_main(f, mis);
+ if (ret < 0) {
+ error_setg(errp, "Load VM state failed: %d", ret);
+ }
qemu_event_set(&mis->main_thread_load_event);
trace_qemu_loadvm_state_post_main(ret);
@@ -3208,8 +3207,15 @@ int qemu_loadvm_state(QEMUFile *f)
if (migrate_has_error(migrate_get_current()) ||
!qemu_loadvm_thread_pool_wait(s, mis)) {
ret = -EINVAL;
+ error_setg(errp,
+ "Error while loading vmstate");
} else {
ret = qemu_file_get_error(f);
+ if (ret < 0) {
+ error_setg(errp,
+ "Error while loading vmstate: stream error: %d",
+ ret);
+ }
}
}
/*
@@ -3474,6 +3480,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
void qmp_xen_load_devices_state(const char *filename, Error **errp)
{
+ ERRP_GUARD();
QEMUFile *f;
QIOChannelFile *ioc;
int ret;
@@ -3495,10 +3502,10 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
f = qemu_file_new_input(QIO_CHANNEL(ioc));
object_unref(OBJECT(ioc));
- ret = qemu_loadvm_state(f);
+ ret = qemu_loadvm_state(f, errp);
qemu_fclose(f);
if (ret < 0) {
- error_setg(errp, "loading Xen device state failed");
+ error_prepend(errp, "loading Xen device state failed: ");
}
migration_incoming_state_destroy();
}
@@ -3569,13 +3576,12 @@ bool load_snapshot(const char *name, const char *vmstate,
ret = -EINVAL;
goto err_drain;
}
- ret = qemu_loadvm_state(f);
+ ret = qemu_loadvm_state(f, errp);
migration_incoming_state_destroy();
bdrv_drain_all_end();
if (ret < 0) {
- error_setg(errp, "Error %d while loading VM state", ret);
return false;
}
diff --git a/migration/savevm.h b/migration/savevm.h
index 2d5e9c716686f06720325e82fe90c75335ced1de..b80770b7461a60e2ad6ba5e24a7baeae73d90955 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -64,7 +64,7 @@ void qemu_savevm_send_colo_enable(QEMUFile *f);
void qemu_savevm_live_state(QEMUFile *f);
int qemu_save_device_state(QEMUFile *f);
-int qemu_loadvm_state(QEMUFile *f);
+int qemu_loadvm_state(QEMUFile *f, Error **errp);
void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
int qemu_load_device_state(QEMUFile *f);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 08/27] migration: push Error **errp into qemu_load_device_state()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (6 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 09/27] migration: push Error **errp into qemu_loadvm_state_main() Arun Menon
` (18 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_load_device_state() must report an error
in errp, in case of failure.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/colo.c | 3 +--
migration/savevm.c | 4 ++--
migration/savevm.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index e0f713c837f5da25d67afbd02ceb6c54024ca3af..2f524b19e6a79dcfe3861f1dfb768222f65a718a 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -729,9 +729,8 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
bql_lock();
vmstate_loading = true;
colo_flush_ram_cache();
- ret = qemu_load_device_state(fb);
+ ret = qemu_load_device_state(fb, errp);
if (ret < 0) {
- error_setg(errp, "COLO: load device state failed");
vmstate_loading = false;
bql_unlock();
return;
diff --git a/migration/savevm.c b/migration/savevm.c
index 0087fca15ce108685667d3808350d80d37b807b1..5462eaff0dcec1fa207c08da6124fe91656201ac 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3264,7 +3264,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
return ret;
}
-int qemu_load_device_state(QEMUFile *f)
+int qemu_load_device_state(QEMUFile *f, Error **errp)
{
MigrationIncomingState *mis = migration_incoming_get_current();
int ret;
@@ -3272,7 +3272,7 @@ int qemu_load_device_state(QEMUFile *f)
/* Load QEMU_VM_SECTION_FULL section */
ret = qemu_loadvm_state_main(f, mis);
if (ret < 0) {
- error_report("Failed to load device state: %d", ret);
+ error_setg(errp, "Failed to load device state: %d", ret);
return ret;
}
diff --git a/migration/savevm.h b/migration/savevm.h
index b80770b7461a60e2ad6ba5e24a7baeae73d90955..b12681839f0b1afa3255e45215d99c13a224b19f 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -67,7 +67,7 @@ int qemu_save_device_state(QEMUFile *f);
int qemu_loadvm_state(QEMUFile *f, Error **errp);
void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
-int qemu_load_device_state(QEMUFile *f);
+int qemu_load_device_state(QEMUFile *f, Error **errp);
int qemu_loadvm_approve_switchover(void);
int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
bool in_postcopy);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 09/27] migration: push Error **errp into qemu_loadvm_state_main()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (7 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 08/27] migration: push Error **errp into qemu_load_device_state() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 10/27] migration: push Error **errp into qemu_loadvm_section_start_full() Arun Menon
` (17 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_state_main() must report an error
in errp, in case of failure.
Set errp explicitly if it is NULL in case of failure in the out
section. This will be removed in the subsequent patch when all of
the calls are converted to passing errp.
The error message in the default case of qemu_loadvm_state_main()
has the word "savevm". This is removed because it can confuse the
user while reading destination side error logs.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/colo.c | 3 +--
migration/savevm.c | 36 +++++++++++++++++-------------------
migration/savevm.h | 3 ++-
3 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index 2f524b19e6a79dcfe3861f1dfb768222f65a718a..642d17487d92c0a9508e599ca3bf524e80b6fd46 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -686,11 +686,10 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
bql_lock();
cpu_synchronize_all_states();
- ret = qemu_loadvm_state_main(mis->from_src_file, mis);
+ ret = qemu_loadvm_state_main(mis->from_src_file, mis, errp);
bql_unlock();
if (ret < 0) {
- error_setg(errp, "Load VM's live state (ram) error");
return;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 5462eaff0dcec1fa207c08da6124fe91656201ac..eccb3f63f8dfc070263a1d19ec3415684e8fa60e 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2105,7 +2105,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
qemu_file_set_blocking(f, true);
/* TODO: sanity check that only postcopiable data will be loaded here */
- load_res = qemu_loadvm_state_main(f, mis);
+ load_res = qemu_loadvm_state_main(f, mis, &error_fatal);
/*
* This is tricky, but, mis->from_src_file can change after it
@@ -2456,10 +2456,7 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
qemu_coroutine_yield();
} while (1);
- ret = qemu_loadvm_state_main(packf, mis);
- if (ret < 0) {
- error_setg(errp, "VM state load failed: %d", ret);
- }
+ ret = qemu_loadvm_state_main(packf, mis, errp);
trace_loadvm_handle_cmd_packaged_main(ret);
qemu_fclose(packf);
object_unref(OBJECT(bioc));
@@ -3080,18 +3077,22 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
return true;
}
-int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
+ Error **errp)
{
+ ERRP_GUARD();
uint8_t section_type;
int ret = 0;
- Error *local_err = NULL;
retry:
while (true) {
section_type = qemu_get_byte(f);
- ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, NULL);
+ ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, errp);
if (ret) {
+ error_prepend(errp,
+ "Failed to load section ID: stream error: %d: ",
+ ret);
break;
}
@@ -3112,10 +3113,7 @@ retry:
}
break;
case QEMU_VM_COMMAND:
- ret = loadvm_process_command(f, &local_err);
- if (ret < 0) {
- error_report_err(local_err);
- }
+ ret = loadvm_process_command(f, errp);
trace_qemu_loadvm_state_section_command(ret);
if ((ret < 0) || (ret == LOADVM_QUIT)) {
goto out;
@@ -3125,7 +3123,7 @@ retry:
/* This is the end of migration */
goto out;
default:
- error_report("Unknown savevm section type %d", section_type);
+ error_setg(errp, "Unknown section type %d", section_type);
ret = -EINVAL;
goto out;
}
@@ -3133,6 +3131,9 @@ retry:
out:
if (ret < 0) {
+ if (*errp == NULL) {
+ error_setg(errp, "Loading VM state failed: %d", ret);
+ }
qemu_file_set_error(f, ret);
/* Cancel bitmaps incoming regardless of recovery */
@@ -3153,6 +3154,7 @@ out:
migrate_postcopy_ram() && postcopy_pause_incoming(mis)) {
/* Reset f to point to the newly created channel */
f = mis->from_src_file;
+ error_free_or_abort(errp);
goto retry;
}
}
@@ -3186,10 +3188,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
cpu_synchronize_all_pre_loadvm();
- ret = qemu_loadvm_state_main(f, mis);
- if (ret < 0) {
- error_setg(errp, "Load VM state failed: %d", ret);
- }
+ ret = qemu_loadvm_state_main(f, mis, errp);
qemu_event_set(&mis->main_thread_load_event);
trace_qemu_loadvm_state_post_main(ret);
@@ -3270,9 +3269,8 @@ int qemu_load_device_state(QEMUFile *f, Error **errp)
int ret;
/* Load QEMU_VM_SECTION_FULL section */
- ret = qemu_loadvm_state_main(f, mis);
+ ret = qemu_loadvm_state_main(f, mis, errp);
if (ret < 0) {
- error_setg(errp, "Failed to load device state: %d", ret);
return ret;
}
diff --git a/migration/savevm.h b/migration/savevm.h
index b12681839f0b1afa3255e45215d99c13a224b19f..c337e3e3d111a7f28a57b90f61e8f70b71803d4e 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -66,7 +66,8 @@ int qemu_save_device_state(QEMUFile *f);
int qemu_loadvm_state(QEMUFile *f, Error **errp);
void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
-int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
+ Error **errp);
int qemu_load_device_state(QEMUFile *f, Error **errp);
int qemu_loadvm_approve_switchover(void);
int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 10/27] migration: push Error **errp into qemu_loadvm_section_start_full()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (8 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 09/27] migration: push Error **errp into qemu_loadvm_state_main() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 11/27] migration: push Error **errp into qemu_loadvm_section_part_end() Arun Menon
` (16 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_section_start_full() must report an error
in errp, in case of failure.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index eccb3f63f8dfc070263a1d19ec3415684e8fa60e..01e1a9739aa784d90f80bf2b561fd62f5e4678f8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2722,21 +2722,21 @@ static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
}
static int
-qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
+qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
{
+ ERRP_GUARD();
bool trace_downtime = (type == QEMU_VM_SECTION_FULL);
uint32_t instance_id, version_id, section_id;
int64_t start_ts, end_ts;
SaveStateEntry *se;
char idstr[256];
int ret;
- Error *local_err = NULL;
/* Read section start */
section_id = qemu_get_be32(f);
if (!qemu_get_counted_string(f, idstr)) {
- error_report("Unable to read ID string for section %u",
- section_id);
+ error_setg(errp, "Unable to read ID string for section %u",
+ section_id);
return -EINVAL;
}
instance_id = qemu_get_be32(f);
@@ -2744,8 +2744,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
ret = qemu_file_get_error(f);
if (ret) {
- error_report("%s: Failed to read instance/version ID: %d",
- __func__, ret);
+ error_setg(errp, "Failed to read instance/version ID: %d", ret);
return ret;
}
@@ -2754,17 +2753,17 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
/* Find savevm section */
se = find_se(idstr, instance_id);
if (se == NULL) {
- error_report("Unknown savevm section or instance '%s' %"PRIu32". "
- "Make sure that your current VM setup matches your "
- "saved VM setup, including any hotplugged devices",
- idstr, instance_id);
+ error_setg(errp, "Unknown section or instance '%s' %"PRIu32". "
+ "Make sure that your current VM setup matches your "
+ "saved VM setup, including any hotplugged devices",
+ idstr, instance_id);
return -EINVAL;
}
/* Validate version */
if (version_id > se->version_id) {
- error_report("savevm: unsupported version %d for '%s' v%d",
- version_id, idstr, se->version_id);
+ error_setg(errp, "unsupported version %d for '%s' v%d",
+ version_id, idstr, se->version_id);
return -EINVAL;
}
se->load_version_id = version_id;
@@ -2772,7 +2771,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
/* Validate if it is a device's state */
if (xen_enabled() && se->is_ram) {
- error_report("loadvm: %s RAM loading not allowed on Xen", idstr);
+ error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
return -EINVAL;
}
@@ -2780,11 +2779,11 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
}
- ret = vmstate_load(f, se, &local_err);
+ ret = vmstate_load(f, se, errp);
if (ret < 0) {
- error_report("error while loading state for instance 0x%"PRIx32" of"
- " device '%s'", instance_id, idstr);
- error_report_err(local_err);
+ error_prepend(errp,
+ "error while loading state for instance 0x%"PRIx32" of"
+ " device '%s': ", instance_id, idstr);
return ret;
}
@@ -2795,6 +2794,8 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
}
if (!check_section_footer(f, se)) {
+ error_setg(errp, "Section footer error, section_id: %d",
+ section_id);
return -EINVAL;
}
@@ -3100,7 +3101,7 @@ retry:
switch (section_type) {
case QEMU_VM_SECTION_START:
case QEMU_VM_SECTION_FULL:
- ret = qemu_loadvm_section_start_full(f, section_type);
+ ret = qemu_loadvm_section_start_full(f, section_type, errp);
if (ret < 0) {
goto out;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 11/27] migration: push Error **errp into qemu_loadvm_section_part_end()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (9 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 10/27] migration: push Error **errp into qemu_loadvm_section_start_full() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 12/27] migration: Update qemu_file_get_return_path() docs and remove dead checks Arun Menon
` (15 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_section_part_end() must report an error
in errp, in case of failure.
This patch also removes the setting of errp when errp is NULL in the
out section as it is no longer required in the series.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 01e1a9739aa784d90f80bf2b561fd62f5e4678f8..c8234f2ef7f413b207aebae7192c82f1f4d6d91d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2803,21 +2803,19 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
}
static int
-qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
+qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type, Error **errp)
{
bool trace_downtime = (type == QEMU_VM_SECTION_END);
int64_t start_ts, end_ts;
uint32_t section_id;
SaveStateEntry *se;
int ret;
- Error *local_err = NULL;
section_id = qemu_get_be32(f);
ret = qemu_file_get_error(f);
if (ret) {
- error_report("%s: Failed to read section ID: %d",
- __func__, ret);
+ error_setg(errp, "Failed to read section ID: %d", ret);
return ret;
}
@@ -2828,7 +2826,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
}
}
if (se == NULL) {
- error_report("Unknown savevm section %d", section_id);
+ error_setg(errp, "Unknown section %d", section_id);
return -EINVAL;
}
@@ -2836,9 +2834,8 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
}
- ret = vmstate_load(f, se, &local_err);
+ ret = vmstate_load(f, se, errp);
if (ret < 0) {
- error_report_err(local_err);
return ret;
}
@@ -2849,6 +2846,8 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
}
if (!check_section_footer(f, se)) {
+ error_setg(errp, "Section footer error, section_id: %d",
+ section_id);
return -EINVAL;
}
@@ -3108,7 +3107,7 @@ retry:
break;
case QEMU_VM_SECTION_PART:
case QEMU_VM_SECTION_END:
- ret = qemu_loadvm_section_part_end(f, section_type);
+ ret = qemu_loadvm_section_part_end(f, section_type, errp);
if (ret < 0) {
goto out;
}
@@ -3132,9 +3131,6 @@ retry:
out:
if (ret < 0) {
- if (*errp == NULL) {
- error_setg(errp, "Loading VM state failed: %d", ret);
- }
qemu_file_set_error(f, ret);
/* Cancel bitmaps incoming regardless of recovery */
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 12/27] migration: Update qemu_file_get_return_path() docs and remove dead checks
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (10 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 11/27] migration: push Error **errp into qemu_loadvm_section_part_end() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 13/27] migration: make loadvm_postcopy_handle_resume() void Arun Menon
` (14 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
The documentation of qemu_file_get_return_path() states that it can
return NULL on failure. However, a review of the current implementation
reveals that it is guaranteed that it will always succeed and will never
return NULL.
As a result, the NULL checks post calling the function become redundant.
This commit updates the documentation for the function and removes all
NULL checks throughout the migration code.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/colo.c | 4 ----
migration/migration.c | 12 ++----------
migration/qemu-file.c | 1 -
migration/savevm.c | 4 ----
4 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index 642d17487d92c0a9508e599ca3bf524e80b6fd46..07a32044b58541eb9bd7c5e63eb6ca598f8a8933 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -847,10 +847,6 @@ static void *colo_process_incoming_thread(void *opaque)
failover_init_state();
mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
- if (!mis->to_src_file) {
- error_report("COLO incoming thread: Open QEMUFile to_src_file failed");
- goto out;
- }
/*
* Note: the communication between Primary side and Secondary side
* should be sequential, we set the fd to unblocked in migration incoming
diff --git a/migration/migration.c b/migration/migration.c
index c6768d88f45c870c7fad9b9957300766ff69effc..35f769bc55c235134abb5554ed01b94787d4bf78 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2647,12 +2647,9 @@ out:
return NULL;
}
-static int open_return_path_on_source(MigrationState *ms)
+static void open_return_path_on_source(MigrationState *ms)
{
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
- if (!ms->rp_state.from_dst_file) {
- return -1;
- }
trace_open_return_path_on_source();
@@ -2661,8 +2658,6 @@ static int open_return_path_on_source(MigrationState *ms)
ms->rp_state.rp_thread_created = true;
trace_open_return_path_on_source_continue();
-
- return 0;
}
/* Return true if error detected, or false otherwise */
@@ -4011,10 +4006,7 @@ void migration_connect(MigrationState *s, Error *error_in)
* QEMU uses the return path.
*/
if (migrate_postcopy_ram() || migrate_return_path()) {
- if (open_return_path_on_source(s)) {
- error_setg(&local_err, "Unable to open return-path for postcopy");
- goto fail;
- }
+ open_return_path_on_source(s);
}
/*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b6ac190034f777dbde0da1598483a892089d7538..f9ccee9a1091ecbd37e6b7d2081a4446442b544d 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -125,7 +125,6 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc, bool is_writable)
/*
* Result: QEMUFile* for a 'return path' for comms in the opposite direction
- * NULL if not available
*/
QEMUFile *qemu_file_get_return_path(QEMUFile *f)
{
diff --git a/migration/savevm.c b/migration/savevm.c
index c8234f2ef7f413b207aebae7192c82f1f4d6d91d..d54028b1be4d7a1e4615c2b05611535bd7478e2b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2588,10 +2588,6 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return 0;
}
mis->to_src_file = qemu_file_get_return_path(f);
- if (!mis->to_src_file) {
- error_setg(errp, "CMD_OPEN_RETURN_PATH failed");
- return -1;
- }
/*
* Switchover ack is enabled but no device uses it, so send an ACK to
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 13/27] migration: make loadvm_postcopy_handle_resume() void
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (11 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 12/27] migration: Update qemu_file_get_return_path() docs and remove dead checks Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 14/27] migration: push Error **errp into ram_postcopy_incoming_init() Arun Menon
` (13 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
Use warn_report() instead of error_report(); it ensures that
a resume command received while the migration is not
in postcopy recover state is not fatal. It only informs that
the command received is unusual, and therefore we should not set
errp with the error string.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index d54028b1be4d7a1e4615c2b05611535bd7478e2b..c079942b7ecb959b3b668b977174d906f73c2777 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2334,12 +2334,12 @@ static void migrate_send_rp_req_pages_pending(MigrationIncomingState *mis)
}
}
-static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
+static void loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
{
if (mis->state != MIGRATION_STATUS_POSTCOPY_RECOVER) {
- error_report("%s: illegal resume received", __func__);
+ warn_report("%s: illegal resume received", __func__);
/* Don't fail the load, only for this. */
- return 0;
+ return;
}
/*
@@ -2391,8 +2391,6 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
/* Kick the fast ram load thread too */
qemu_sem_post(&mis->postcopy_pause_sem_fast_load);
}
-
- return 0;
}
/**
@@ -2647,7 +2645,8 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return ret;
case MIG_CMD_POSTCOPY_RESUME:
- return loadvm_postcopy_handle_resume(mis);
+ loadvm_postcopy_handle_resume(mis);
+ return 0;
case MIG_CMD_RECV_BITMAP:
ret = loadvm_handle_recv_bitmap(mis, len);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 14/27] migration: push Error **errp into ram_postcopy_incoming_init()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (12 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 13/27] migration: make loadvm_postcopy_handle_resume() void Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 15/27] migration: push Error **errp into loadvm_postcopy_handle_advise() Arun Menon
` (12 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that ram_postcopy_incoming_init() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/postcopy-ram.c | 9 ++++++---
migration/postcopy-ram.h | 2 +-
migration/ram.c | 4 ++--
migration/ram.h | 2 +-
migration/savevm.c | 2 +-
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 45af9a361e8eacaad0fb217a5da2c5004416c1da..05617e5fbcad62226a54fe17d9f7d9a316baf1e4 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -681,6 +681,7 @@ out:
*/
static int init_range(RAMBlock *rb, void *opaque)
{
+ Error **errp = opaque;
const char *block_name = qemu_ram_get_idstr(rb);
void *host_addr = qemu_ram_get_host_addr(rb);
ram_addr_t offset = qemu_ram_get_offset(rb);
@@ -701,6 +702,8 @@ static int init_range(RAMBlock *rb, void *opaque)
* (Precopy will just overwrite this data, so doesn't need the discard)
*/
if (ram_discard_range(block_name, 0, length)) {
+ error_setg(errp, "failed to discard RAM block %s len=%zu",
+ block_name, length);
return -1;
}
@@ -749,9 +752,9 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
* postcopy later; must be called prior to any precopy.
* called from arch_init's similarly named ram_postcopy_incoming_init
*/
-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
+int postcopy_ram_incoming_init(MigrationIncomingState *mis, Error **errp)
{
- if (foreach_not_ignored_block(init_range, NULL)) {
+ if (foreach_not_ignored_block(init_range, errp)) {
return -1;
}
@@ -1703,7 +1706,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis, Error **errp)
return false;
}
-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
+int postcopy_ram_incoming_init(MigrationIncomingState *mis, Error **errp)
{
error_report("postcopy_ram_incoming_init: No OS support");
return -1;
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 3852141d7e37ab18bada4b46c137fef0969d0070..ca19433b246893fa5105bcebffb442c58a9a4f48 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -30,7 +30,7 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis);
* postcopy later; must be called prior to any precopy.
* called from ram.c's similarly named ram_postcopy_incoming_init
*/
-int postcopy_ram_incoming_init(MigrationIncomingState *mis);
+int postcopy_ram_incoming_init(MigrationIncomingState *mis, Error **errp);
/*
* At the end of a migration where postcopy_ram_incoming_init was called.
diff --git a/migration/ram.c b/migration/ram.c
index 7208bc114fb5c366740db380ee6956a91b3871a0..6a0dcc04f436524a37672c41c38f201f06773374 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3716,9 +3716,9 @@ static int ram_load_cleanup(void *opaque)
* postcopy-ram. postcopy-ram's similarly names
* postcopy_ram_incoming_init does the work.
*/
-int ram_postcopy_incoming_init(MigrationIncomingState *mis)
+int ram_postcopy_incoming_init(MigrationIncomingState *mis, Error **errp)
{
- return postcopy_ram_incoming_init(mis);
+ return postcopy_ram_incoming_init(mis, errp);
}
/**
diff --git a/migration/ram.h b/migration/ram.h
index 921c39a2c5c45bc2344be80854c46e4c10c09aeb..275709a99187f9429ccb4111e05281ec268ba0db 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -86,7 +86,7 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms);
void ram_postcopy_send_discard_bitmap(MigrationState *ms);
/* For incoming postcopy discard */
int ram_discard_range(const char *block_name, uint64_t start, size_t length);
-int ram_postcopy_incoming_init(MigrationIncomingState *mis);
+int ram_postcopy_incoming_init(MigrationIncomingState *mis, Error **errp);
int ram_load_postcopy(QEMUFile *f, int channel);
void ram_handle_zero(void *host, uint64_t size);
diff --git a/migration/savevm.c b/migration/savevm.c
index c079942b7ecb959b3b668b977174d906f73c2777..a1b606318f2ae8feff42b759ad308ebbd07318b7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1989,7 +1989,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
return -1;
}
- if (ram_postcopy_incoming_init(mis)) {
+ if (ram_postcopy_incoming_init(mis, NULL) < 0) {
return -1;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 15/27] migration: push Error **errp into loadvm_postcopy_handle_advise()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (13 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 14/27] migration: push Error **errp into ram_postcopy_incoming_init() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 16/27] migration: push Error **errp into loadvm_postcopy_handle_listen() Arun Menon
` (11 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_postcopy_handle_advise() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index a1b606318f2ae8feff42b759ad308ebbd07318b7..680f6e1c4bbc979f779d9fde28ab71ae234b5891 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1912,39 +1912,39 @@ enum LoadVMExitCodes {
* quickly.
*/
static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
- uint16_t len)
+ uint16_t len, Error **errp)
{
PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_ADVISE);
uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps;
size_t page_size = qemu_target_page_size();
- Error *local_err = NULL;
trace_loadvm_postcopy_handle_advise();
if (ps != POSTCOPY_INCOMING_NONE) {
- error_report("CMD_POSTCOPY_ADVISE in wrong postcopy state (%d)", ps);
+ error_setg(errp, "CMD_POSTCOPY_ADVISE in wrong postcopy state (%d)",
+ ps);
return -1;
}
switch (len) {
case 0:
if (migrate_postcopy_ram()) {
- error_report("RAM postcopy is enabled but have 0 byte advise");
+ error_setg(errp, "RAM postcopy is enabled but have 0 byte advise");
return -EINVAL;
}
return 0;
case 8 + 8:
if (!migrate_postcopy_ram()) {
- error_report("RAM postcopy is disabled but have 16 byte advise");
+ error_setg(errp,
+ "RAM postcopy is disabled but have 16 byte advise");
return -EINVAL;
}
break;
default:
- error_report("CMD_POSTCOPY_ADVISE invalid length (%d)", len);
+ error_setg(errp, "CMD_POSTCOPY_ADVISE invalid length (%d)", len);
return -EINVAL;
}
- if (!postcopy_ram_supported_by_host(mis, &local_err)) {
- error_report_err(local_err);
+ if (!postcopy_ram_supported_by_host(mis, errp)) {
postcopy_state_set(POSTCOPY_INCOMING_NONE);
return -1;
}
@@ -1967,9 +1967,10 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
* also fails when passed to an older qemu that doesn't
* do huge pages.
*/
- error_report("Postcopy needs matching RAM page sizes (s=%" PRIx64
- " d=%" PRIx64 ")",
- remote_pagesize_summary, local_pagesize_summary);
+ error_setg(errp,
+ "Postcopy needs matching RAM page sizes "
+ "(s=%" PRIx64 " d=%" PRIx64 ")",
+ remote_pagesize_summary, local_pagesize_summary);
return -1;
}
@@ -1979,17 +1980,18 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
* Again, some differences could be dealt with, but for now keep it
* simple.
*/
- error_report("Postcopy needs matching target page sizes (s=%d d=%zd)",
- (int)remote_tps, page_size);
+ error_setg(errp,
+ "Postcopy needs matching target page sizes (s=%d d=%zd)",
+ (int)remote_tps, page_size);
return -1;
}
- if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_ADVISE, &local_err)) {
- error_report_err(local_err);
+ if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_ADVISE, errp)) {
return -1;
}
- if (ram_postcopy_incoming_init(mis, NULL) < 0) {
+ if (ram_postcopy_incoming_init(mis, errp) < 0) {
+ error_prepend(errp, "Postcopy RAM incoming init failed: ");
return -1;
}
@@ -2617,11 +2619,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return loadvm_handle_cmd_packaged(mis, errp);
case MIG_CMD_POSTCOPY_ADVISE:
- ret = loadvm_postcopy_handle_advise(mis, len);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_postcopy_handle_advise(mis, len, errp);
case MIG_CMD_POSTCOPY_LISTEN:
ret = loadvm_postcopy_handle_listen(mis);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 16/27] migration: push Error **errp into loadvm_postcopy_handle_listen()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (14 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 15/27] migration: push Error **errp into loadvm_postcopy_handle_advise() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 17/27] migration: push Error **errp into loadvm_postcopy_handle_run() Arun Menon
` (10 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_postcopy_handle_listen() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 680f6e1c4bbc979f779d9fde28ab71ae234b5891..c681d6d21af356a97645f2a0d36e7440e3bd95c6 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2181,15 +2181,16 @@ static void *postcopy_ram_listen_thread(void *opaque)
}
/* After this message we must be able to immediately receive postcopy data */
-static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
+static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis,
+ Error **errp)
{
PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
- Error *local_err = NULL;
trace_loadvm_postcopy_handle_listen("enter");
if (ps != POSTCOPY_INCOMING_ADVISE && ps != POSTCOPY_INCOMING_DISCARD) {
- error_report("CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)", ps);
+ error_setg(errp,
+ "CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)", ps);
return -1;
}
if (ps == POSTCOPY_INCOMING_ADVISE) {
@@ -2212,14 +2213,14 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
if (migrate_postcopy_ram()) {
if (postcopy_ram_incoming_setup(mis)) {
postcopy_ram_incoming_cleanup(mis);
+ error_setg(errp, "Failed to setup incoming postcopy RAM blocks");
return -1;
}
}
trace_loadvm_postcopy_handle_listen("after uffd");
- if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, &local_err)) {
- error_report_err(local_err);
+ if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, errp)) {
return -1;
}
@@ -2622,11 +2623,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return loadvm_postcopy_handle_advise(mis, len, errp);
case MIG_CMD_POSTCOPY_LISTEN:
- ret = loadvm_postcopy_handle_listen(mis);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_postcopy_handle_listen(mis, errp);
case MIG_CMD_POSTCOPY_RUN:
ret = loadvm_postcopy_handle_run(mis);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 17/27] migration: push Error **errp into loadvm_postcopy_handle_run()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (15 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 16/27] migration: push Error **errp into loadvm_postcopy_handle_listen() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 18/27] migration: push Error **errp into loadvm_postcopy_ram_handle_discard() Arun Menon
` (9 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_postcopy_handle_run() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index c681d6d21af356a97645f2a0d36e7440e3bd95c6..3ab20bd3de980ecfd79ab29697e3e1527ab3e73c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2273,13 +2273,13 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
}
/* After all discards we can start running and asking for pages */
-static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
+static int loadvm_postcopy_handle_run(MigrationIncomingState *mis, Error **errp)
{
PostcopyState ps = postcopy_state_get();
trace_loadvm_postcopy_handle_run();
if (ps != POSTCOPY_INCOMING_LISTENING) {
- error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
+ error_setg(errp, "CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
return -1;
}
@@ -2626,11 +2626,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return loadvm_postcopy_handle_listen(mis, errp);
case MIG_CMD_POSTCOPY_RUN:
- ret = loadvm_postcopy_handle_run(mis);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_postcopy_handle_run(mis, errp);
case MIG_CMD_POSTCOPY_RAM_DISCARD:
ret = loadvm_postcopy_ram_handle_discard(mis, len);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 18/27] migration: push Error **errp into loadvm_postcopy_ram_handle_discard()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (16 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 17/27] migration: push Error **errp into loadvm_postcopy_handle_run() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:01 ` [PATCH v13 19/27] migration: push Error **errp into loadvm_handle_recv_bitmap() Arun Menon
` (8 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_postcopy_ram_handle_discard() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 3ab20bd3de980ecfd79ab29697e3e1527ab3e73c..01cc186b9e0324053e804c4fedebee02d8df4052 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2004,7 +2004,7 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
* There can be 0..many of these messages, each encoding multiple pages.
*/
static int loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
- uint16_t len)
+ uint16_t len, Error **errp)
{
int tmp;
char ramid[256];
@@ -2017,6 +2017,7 @@ static int loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
/* 1st discard */
tmp = postcopy_ram_prepare_discard(mis);
if (tmp) {
+ error_setg(errp, "Failed to prepare for RAM discard: %d", tmp);
return tmp;
}
break;
@@ -2026,8 +2027,9 @@ static int loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
break;
default:
- error_report("CMD_POSTCOPY_RAM_DISCARD in wrong postcopy state (%d)",
- ps);
+ error_setg(errp,
+ "CMD_POSTCOPY_RAM_DISCARD in wrong postcopy state (%d)",
+ ps);
return -1;
}
/* We're expecting a
@@ -2036,29 +2038,30 @@ static int loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
* then at least 1 16 byte chunk
*/
if (len < (1 + 1 + 1 + 1 + 2 * 8)) {
- error_report("CMD_POSTCOPY_RAM_DISCARD invalid length (%d)", len);
+ error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD invalid length (%d)", len);
return -1;
}
tmp = qemu_get_byte(mis->from_src_file);
if (tmp != postcopy_ram_discard_version) {
- error_report("CMD_POSTCOPY_RAM_DISCARD invalid version (%d)", tmp);
+ error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD invalid version (%d)", tmp);
return -1;
}
if (!qemu_get_counted_string(mis->from_src_file, ramid)) {
- error_report("CMD_POSTCOPY_RAM_DISCARD Failed to read RAMBlock ID");
+ error_setg(errp,
+ "CMD_POSTCOPY_RAM_DISCARD Failed to read RAMBlock ID");
return -1;
}
tmp = qemu_get_byte(mis->from_src_file);
if (tmp != 0) {
- error_report("CMD_POSTCOPY_RAM_DISCARD missing nil (%d)", tmp);
+ error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD missing nil (%d)", tmp);
return -1;
}
len -= 3 + strlen(ramid);
if (len % 16) {
- error_report("CMD_POSTCOPY_RAM_DISCARD invalid length (%d)", len);
+ error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD invalid length (%d)", len);
return -1;
}
trace_loadvm_postcopy_ram_handle_discard_header(ramid, len);
@@ -2070,6 +2073,7 @@ static int loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
len -= 16;
int ret = ram_discard_range(ramid, start_addr, block_length);
if (ret) {
+ error_setg(errp, "Failed to discard RAM range %s: %d", ramid, ret);
return ret;
}
}
@@ -2629,11 +2633,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return loadvm_postcopy_handle_run(mis, errp);
case MIG_CMD_POSTCOPY_RAM_DISCARD:
- ret = loadvm_postcopy_ram_handle_discard(mis, len);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_postcopy_ram_handle_discard(mis, len, errp);
case MIG_CMD_POSTCOPY_RESUME:
loadvm_postcopy_handle_resume(mis);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 19/27] migration: push Error **errp into loadvm_handle_recv_bitmap()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (17 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 18/27] migration: push Error **errp into loadvm_postcopy_ram_handle_discard() Arun Menon
@ 2025-08-29 20:01 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 20/27] migration: Return -1 on memory allocation failure in ram.c Arun Menon
` (7 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_handle_recv_bitmap() must report an error
in errp, in case of failure.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 01cc186b9e0324053e804c4fedebee02d8df4052..13a809564aebf6346228f46fd60f611ba6104bea 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2476,32 +2476,35 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
* len (1 byte) + ramblock_name (<255 bytes)
*/
static int loadvm_handle_recv_bitmap(MigrationIncomingState *mis,
- uint16_t len)
+ uint16_t len, Error **errp)
{
QEMUFile *file = mis->from_src_file;
RAMBlock *rb;
char block_name[256];
size_t cnt;
+ int ret;
cnt = qemu_get_counted_string(file, block_name);
if (!cnt) {
- error_report("%s: failed to read block name", __func__);
+ error_setg(errp, "failed to read block name");
return -EINVAL;
}
/* Validate before using the data */
- if (qemu_file_get_error(file)) {
- return qemu_file_get_error(file);
+ ret = qemu_file_get_error(file);
+ if (ret < 0) {
+ error_setg(errp, "loadvm failed: stream error: %d", ret);
+ return ret;
}
if (len != cnt + 1) {
- error_report("%s: invalid payload length (%d)", __func__, len);
+ error_setg(errp, "invalid payload length (%d)", len);
return -EINVAL;
}
rb = qemu_ram_block_by_name(block_name);
if (!rb) {
- error_report("%s: block '%s' not found", __func__, block_name);
+ error_setg(errp, "block '%s' not found", block_name);
return -EINVAL;
}
@@ -2640,11 +2643,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return 0;
case MIG_CMD_RECV_BITMAP:
- ret = loadvm_handle_recv_bitmap(mis, len);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_handle_recv_bitmap(mis, len, errp);
case MIG_CMD_ENABLE_COLO:
ret = loadvm_process_enable_colo(mis);
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 20/27] migration: Return -1 on memory allocation failure in ram.c
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (18 preceding siblings ...)
2025-08-29 20:01 ` [PATCH v13 19/27] migration: push Error **errp into loadvm_handle_recv_bitmap() Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 21/27] migration: push Error **errp into loadvm_process_enable_colo() Arun Menon
` (6 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
The function colo_init_ram_cache() currently returns -errno if
qemu_anon_ram_alloc() fails. However, the subsequent cleanup loop that
calls qemu_anon_ram_free() could potentially alter the value of errno.
This would cause the function to return a value that does not accurately
represent the original allocation failure.
This commit changes the return value to -1 on memory allocation failure.
This ensures that the return value is consistent and is not affected by
any errno changes that may occur during the free process.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/ram.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/migration/ram.c b/migration/ram.c
index 6a0dcc04f436524a37672c41c38f201f06773374..163265a57f26fb1dc88d9797629d58c278e9afb7 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3575,6 +3575,8 @@ static void colo_init_ram_state(void)
* colo cache: this is for secondary VM, we cache the whole
* memory of the secondary VM, it is need to hold the global lock
* to call this helper.
+ *
+ * Returns zero to indicate success or -1 on error.
*/
int colo_init_ram_cache(void)
{
@@ -3594,7 +3596,7 @@ int colo_init_ram_cache(void)
block->colo_cache = NULL;
}
}
- return -errno;
+ return -1;
}
if (!machine_dump_guest_core(current_machine)) {
qemu_madvise(block->colo_cache, block->used_length,
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 21/27] migration: push Error **errp into loadvm_process_enable_colo()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (19 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 20/27] migration: Return -1 on memory allocation failure in ram.c Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 22/27] migration: push Error **errp into loadvm_postcopy_handle_switchover_start() Arun Menon
` (5 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that loadvm_process_enable_colo() must report an error
in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
include/migration/colo.h | 2 +-
migration/migration.c | 12 ++++++------
migration/ram.c | 8 ++++----
migration/ram.h | 2 +-
migration/savevm.c | 26 ++++++++++++++------------
5 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/include/migration/colo.h b/include/migration/colo.h
index 43222ef5ae6adc3f7d8aa6a48bef79af33d09208..d4fe422e4d335d3bef4f860f56400fcd73287a0e 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -25,7 +25,7 @@ void migrate_start_colo_process(MigrationState *s);
bool migration_in_colo_state(void);
/* loadvm */
-int migration_incoming_enable_colo(void);
+int migration_incoming_enable_colo(Error **errp);
void migration_incoming_disable_colo(void);
bool migration_incoming_colo_enabled(void);
bool migration_incoming_in_colo_state(void);
diff --git a/migration/migration.c b/migration/migration.c
index 35f769bc55c235134abb5554ed01b94787d4bf78..39d6142abccf90c3a35e6c2df3df245052eb0bcf 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -623,22 +623,22 @@ void migration_incoming_disable_colo(void)
migration_colo_enabled = false;
}
-int migration_incoming_enable_colo(void)
+int migration_incoming_enable_colo(Error **errp)
{
#ifndef CONFIG_REPLICATION
- error_report("ENABLE_COLO command come in migration stream, but the "
- "replication module is not built in");
+ error_setg(errp, "ENABLE_COLO command come in migration stream, but the "
+ "replication module is not built in");
return -ENOTSUP;
#endif
if (!migrate_colo()) {
- error_report("ENABLE_COLO command come in migration stream, but x-colo "
- "capability is not set");
+ error_setg(errp, "ENABLE_COLO command come in migration stream"
+ ", but x-colo capability is not set");
return -EINVAL;
}
if (ram_block_discard_disable(true)) {
- error_report("COLO: cannot disable RAM discard");
+ error_setg(errp, "COLO: cannot disable RAM discard");
return -EBUSY;
}
migration_colo_enabled = true;
diff --git a/migration/ram.c b/migration/ram.c
index 163265a57f26fb1dc88d9797629d58c278e9afb7..a8e8d2cc6790336625de626fee070ecc51327200 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3578,7 +3578,7 @@ static void colo_init_ram_state(void)
*
* Returns zero to indicate success or -1 on error.
*/
-int colo_init_ram_cache(void)
+int colo_init_ram_cache(Error **errp)
{
RAMBlock *block;
@@ -3587,9 +3587,9 @@ int colo_init_ram_cache(void)
block->colo_cache = qemu_anon_ram_alloc(block->used_length,
NULL, false, false);
if (!block->colo_cache) {
- error_report("%s: Can't alloc memory for COLO cache of block %s,"
- "size 0x" RAM_ADDR_FMT, __func__, block->idstr,
- block->used_length);
+ error_setg(errp, "Can't alloc memory for COLO cache of "
+ "block %s, size 0x" RAM_ADDR_FMT,
+ block->idstr, block->used_length);
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
if (block->colo_cache) {
qemu_anon_ram_free(block->colo_cache, block->used_length);
diff --git a/migration/ram.h b/migration/ram.h
index 275709a99187f9429ccb4111e05281ec268ba0db..24cd0bf585762cfa1e86834dc03c6baeea2f0627 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -109,7 +109,7 @@ void ramblock_set_file_bmap_atomic(RAMBlock *block, ram_addr_t offset,
bool set);
/* ram cache */
-int colo_init_ram_cache(void);
+int colo_init_ram_cache(Error **errp);
void colo_flush_ram_cache(void);
void colo_release_ram_cache(void);
void colo_incoming_start_dirty_log(void);
diff --git a/migration/savevm.c b/migration/savevm.c
index 13a809564aebf6346228f46fd60f611ba6104bea..8e891243dc1e7f5d3bc3a4326c6c4a2f5c32e1a2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2515,15 +2515,21 @@ static int loadvm_handle_recv_bitmap(MigrationIncomingState *mis,
return 0;
}
-static int loadvm_process_enable_colo(MigrationIncomingState *mis)
+static int loadvm_process_enable_colo(MigrationIncomingState *mis,
+ Error **errp)
{
- int ret = migration_incoming_enable_colo();
+ ERRP_GUARD();
+ int ret;
- if (!ret) {
- ret = colo_init_ram_cache();
- if (ret) {
- migration_incoming_disable_colo();
- }
+ ret = migration_incoming_enable_colo(errp);
+ if (ret < 0) {
+ return ret;
+ }
+
+ ret = colo_init_ram_cache(errp);
+ if (ret) {
+ error_prepend(errp, "failed to init colo RAM cache: %d: ", ret);
+ migration_incoming_disable_colo();
}
return ret;
}
@@ -2646,11 +2652,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return loadvm_handle_recv_bitmap(mis, len, errp);
case MIG_CMD_ENABLE_COLO:
- ret = loadvm_process_enable_colo(mis);
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_process_enable_colo(mis, errp);
case MIG_CMD_SWITCHOVER_START:
ret = loadvm_postcopy_handle_switchover_start();
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 22/27] migration: push Error **errp into loadvm_postcopy_handle_switchover_start()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (20 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 21/27] migration: push Error **errp into loadvm_process_enable_colo() Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 23/27] migration: Capture error in postcopy_ram_listen_thread() Arun Menon
` (4 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé
This is an incremental step in converting vmstate loading code to report
error via Error objects instead of directly printing it to console/monitor.
It is ensured that loadvm_postcopy_handle_switchover_start() must report
an error in errp, in case of failure.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 8e891243dc1e7f5d3bc3a4326c6c4a2f5c32e1a2..b0fd51d0f142c30ce21bdc02204ce947283fea49 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2534,7 +2534,7 @@ static int loadvm_process_enable_colo(MigrationIncomingState *mis,
return ret;
}
-static int loadvm_postcopy_handle_switchover_start(void)
+static int loadvm_postcopy_handle_switchover_start(Error **errp)
{
SaveStateEntry *se;
@@ -2547,6 +2547,7 @@ static int loadvm_postcopy_handle_switchover_start(void)
ret = se->ops->switchover_start(se->opaque);
if (ret < 0) {
+ error_setg(errp, "Switchover start failed: %d", ret);
return ret;
}
}
@@ -2655,11 +2656,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
return loadvm_process_enable_colo(mis, errp);
case MIG_CMD_SWITCHOVER_START:
- ret = loadvm_postcopy_handle_switchover_start();
- if (ret < 0) {
- error_setg(errp, "Failed to load device state command: %d", ret);
- }
- return ret;
+ return loadvm_postcopy_handle_switchover_start(errp);
}
return 0;
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 23/27] migration: Capture error in postcopy_ram_listen_thread()
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (21 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 22/27] migration: push Error **errp into loadvm_postcopy_handle_switchover_start() Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 24/27] migration: Remove error variant of vmstate_save_state() function Arun Menon
` (3 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
postcopy_ram_listen_thread() calls qemu_loadvm_state_main()
to load the vm, and in case of a failure, it should set the error
in the migration object.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/savevm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index b0fd51d0f142c30ce21bdc02204ce947283fea49..71f4d8399349934134c2c02609d3ede6f61754f8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2095,6 +2095,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
QEMUFile *f = mis->from_src_file;
int load_res;
MigrationState *migr = migrate_get_current();
+ Error *local_err = NULL;
object_ref(OBJECT(migr));
@@ -2111,7 +2112,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
qemu_file_set_blocking(f, true);
/* TODO: sanity check that only postcopiable data will be loaded here */
- load_res = qemu_loadvm_state_main(f, mis, &error_fatal);
+ load_res = qemu_loadvm_state_main(f, mis, &local_err);
/*
* This is tricky, but, mis->from_src_file can change after it
@@ -2137,7 +2138,10 @@ static void *postcopy_ram_listen_thread(void *opaque)
__func__, load_res);
load_res = 0; /* prevent further exit() */
} else {
- error_report("%s: loadvm failed: %d", __func__, load_res);
+ error_prepend(&local_err,
+ "loadvm failed during postcopy: %d: ", load_res);
+ migrate_set_error(migr, local_err);
+ error_report_err(local_err);
migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
MIGRATION_STATUS_FAILED);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 24/27] migration: Remove error variant of vmstate_save_state() function
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (22 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 23/27] migration: Capture error in postcopy_ram_listen_thread() Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 25/27] migration: Rename post_save() to cleanup_save() and make it void Arun Menon
` (2 subsequent siblings)
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
This commit removes the redundant vmstate_save_state_with_err()
function.
Previously, commit 969298f9d7 introduced vmstate_save_state_with_err()
to handle error propagation, while vmstate_save_state() existed for
non-error scenarios.
This is because there were code paths where vmstate_save_state_v()
(called internally by vmstate_save_state) did not explicitly set
errors on failure.
This change unifies error handling by
- updating vmstate_save_state() to accept an Error **errp argument.
- vmstate_save_state_v() ensures errors are set directly within the errp
object, eliminating the need for two separate functions.
All calls to vmstate_save_state_with_err() are replaced with
vmstate_save_state(). This simplifies the API and improves code
maintainability.
vmstate_save_state() that only calls vmstate_save_state_v(),
by inference, also has errors set in errp in case of failure.
The errors are reported using error_report_err().
If we want the function to exit on error, then &error_fatal is
passed.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
hw/display/virtio-gpu.c | 3 ++-
hw/pci/pci.c | 2 +-
hw/s390x/virtio-ccw.c | 2 +-
hw/scsi/spapr_vscsi.c | 2 +-
hw/vfio/pci.c | 4 ++--
hw/virtio/virtio-mmio.c | 2 +-
hw/virtio/virtio-pci.c | 2 +-
hw/virtio/virtio.c | 6 ++++--
include/migration/vmstate.h | 2 --
migration/cpr.c | 3 +--
migration/savevm.c | 11 ++++++++---
migration/vmstate-types.c | 25 ++++++++++++++++++-------
migration/vmstate.c | 10 ++--------
tests/unit/test-vmstate.c | 20 +++++++++++++++++---
ui/vdagent.c | 3 ++-
15 files changed, 61 insertions(+), 36 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5dc31bc6bfb0272e29a4364ab10de2595a4bedf7..477ec700a130d77ad93346c264bb7eaa8bca76d5 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1246,7 +1246,8 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
}
qemu_put_be32(f, 0); /* end of list */
- return vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);
+ return vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL,
+ &error_fatal);
}
static bool virtio_gpu_load_restore_mapping(VirtIOGPU *g,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6be932d3bb67ff0c4808707db2a7b6378a90e82b..e53fd270118293ca82f40d45d8f97f62612736a1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -926,7 +926,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
* This makes us compatible with old devices
* which never set or clear this bit. */
s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
- vmstate_save_state(f, &vmstate_pci_device, s, NULL);
+ vmstate_save_state(f, &vmstate_pci_device, s, NULL, &error_fatal);
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
}
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 6a9641a03d5d3a38a4de7ceb9deffc0cc303bcff..4cb1ced001ae241c53c503ebfd7c90e336799c37 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1130,7 +1130,7 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f)
static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f)
{
VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL);
+ vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL, &error_fatal);
}
static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index da173f48676395cc02bd00cb3efa248afed4581b..f0a7dd2b882a13deec4a4c6d2eb4aae6d2fdbeb9 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -630,7 +630,7 @@ static void vscsi_save_request(QEMUFile *f, SCSIRequest *sreq)
vscsi_req *req = sreq->hba_private;
assert(req->active);
- vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL);
+ vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL, &error_fatal);
trace_spapr_vscsi_save_request(req->qtag, req->cur_desc_num,
req->cur_desc_offset);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1ecf150ed8218e8815655d3665f14ebac4382cf9..7c41687e4bab9e35b382bfceff66e82f68abaf56 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2789,8 +2789,8 @@ static int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error **errp)
{
VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
- return vmstate_save_state_with_err(f, &vmstate_vfio_pci_config, vdev, NULL,
- errp);
+ return vmstate_save_state(f, &vmstate_vfio_pci_config, vdev, NULL,
+ errp);
}
static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 0a688909fc606a3c9fde933667ae8c309ab527d0..fb58c36452730cfc92a0d26ff13e01e2d6654960 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -613,7 +613,7 @@ static void virtio_mmio_save_extra_state(DeviceState *opaque, QEMUFile *f)
{
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
- vmstate_save_state(f, &vmstate_virtio_mmio, proxy, NULL);
+ vmstate_save_state(f, &vmstate_virtio_mmio, proxy, NULL, &error_fatal);
}
static int virtio_mmio_load_extra_state(DeviceState *opaque, QEMUFile *f)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b04faa1e5c91b5cef40e54ec41d92422d16bfc13..d2595fbd55151aba2579bb01a59577c1a40b71e0 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -154,7 +154,7 @@ static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
- vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL);
+ vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL, &error_fatal);
}
static int virtio_pci_load_extra_state(DeviceState *d, QEMUFile *f)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 018803c80d13107eb4e5e63914f9c1f837ab1b19..0a68f1b6f1ddb97a968c9c03ed4f37a38484d2f9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2992,6 +2992,7 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
uint32_t guest_features_lo = (vdev->guest_features & 0xffffffff);
int i;
+ Error *local_err = NULL;
if (k->save_config) {
k->save_config(qbus->parent, f);
@@ -3035,14 +3036,15 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
}
if (vdc->vmsd) {
- int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL);
+ int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL, &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
}
/* Subsections */
- return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
+ return vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &error_fatal);
}
/* A wrapper for use as a VMState .put function */
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 056781b1c21e737583f081594d9f88b32adfd674..5fe9bbf39058d0cf97c1adab54cc516dbe8dc32a 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1198,8 +1198,6 @@ extern const VMStateInfo vmstate_info_qlist;
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);
-int vmstate_save_state_with_err(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,
diff --git a/migration/cpr.c b/migration/cpr.c
index 8abb6db76d2474157f804ece4c35ebfc8c22d21a..038f48f810aceb66d7988852240e2e5d9840ad1a 100644
--- a/migration/cpr.c
+++ b/migration/cpr.c
@@ -182,9 +182,8 @@ int 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);
+ ret = vmstate_save_state(f, &vmstate_cpr_state, &cpr_state, 0, errp);
if (ret) {
- error_setg(errp, "vmstate_save_state error %d", ret);
qemu_fclose(f);
return ret;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 71f4d8399349934134c2c02609d3ede6f61754f8..2c05b763148295d7e2095027aa143f45bc4c4676 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1056,8 +1056,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_with_err(f, se->vmsd, se->opaque, vmdesc,
- errp);
+ ret = vmstate_save_state(f, se->vmsd, se->opaque, vmdesc,
+ errp);
if (ret) {
return ret;
}
@@ -1285,6 +1285,7 @@ void qemu_savevm_state_header(QEMUFile *f)
{
MigrationState *s = migrate_get_current();
JSONWriter *vmdesc = s->vmdesc;
+ Error *local_err = NULL;
trace_savevm_state_header();
qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
@@ -1303,7 +1304,11 @@ void qemu_savevm_state_header(QEMUFile *f)
json_writer_start_object(vmdesc, "configuration");
}
- vmstate_save_state(f, &vmstate_configuration, &savevm_state, vmdesc);
+ vmstate_save_state(f, &vmstate_configuration, &savevm_state,
+ vmdesc, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ }
if (vmdesc) {
json_writer_end_object(vmdesc);
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index c5cfd861e3aa5391245ef46bec9c90106c467e6e..a1cd7a95fa0148858dd883acf9a707b2b076d89e 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -565,10 +565,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size,
const VMStateDescription *vmsd = field->vmsd;
void *tmp = g_malloc(size);
int ret;
+ Error *local_err = NULL;
/* Writes the parent field which is at the start of the tmp */
*(void **)tmp = pv;
- ret = vmstate_save_state(f, vmsd, tmp, vmdesc);
+ ret = vmstate_save_state(f, vmsd, tmp, vmdesc, &local_err);
+ if (ret) {
+ error_report_err(local_err);
+ }
g_free(tmp);
return ret;
@@ -676,13 +680,15 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,
size_t entry_offset = field->start;
void *elm;
int ret;
+ Error *local_err = NULL;
trace_put_qtailq(vmsd->name, vmsd->version_id);
QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {
qemu_put_byte(f, true);
- ret = vmstate_save_state(f, vmsd, elm, vmdesc);
+ ret = vmstate_save_state(f, vmsd, elm, vmdesc, &local_err);
if (ret) {
+ error_report_err(local_err);
return ret;
}
}
@@ -711,6 +717,7 @@ static gboolean put_gtree_elem(gpointer key, gpointer value, gpointer data)
struct put_gtree_data *capsule = (struct put_gtree_data *)data;
QEMUFile *f = capsule->f;
int ret;
+ Error *local_err = NULL;
qemu_put_byte(f, true);
@@ -718,16 +725,20 @@ static gboolean put_gtree_elem(gpointer key, gpointer value, gpointer data)
if (!capsule->key_vmsd) {
qemu_put_be64(f, (uint64_t)(uintptr_t)(key)); /* direct key */
} else {
- ret = vmstate_save_state(f, capsule->key_vmsd, key, capsule->vmdesc);
+ ret = vmstate_save_state(f, capsule->key_vmsd, key, capsule->vmdesc,
+ &local_err);
if (ret) {
+ error_report_err(local_err);
capsule->ret = ret;
return true;
}
}
/* put the data */
- ret = vmstate_save_state(f, capsule->val_vmsd, value, capsule->vmdesc);
+ ret = vmstate_save_state(f, capsule->val_vmsd, value, capsule->vmdesc,
+ &local_err);
if (ret) {
+ error_report_err(local_err);
capsule->ret = ret;
return true;
}
@@ -857,14 +868,14 @@ static int put_qlist(QEMUFile *f, void *pv, size_t unused_size,
size_t entry_offset = field->start;
void *elm;
int ret;
+ Error *local_err = NULL;
trace_put_qlist(field->name, vmsd->name, vmsd->version_id);
QLIST_RAW_FOREACH(elm, pv, entry_offset) {
qemu_put_byte(f, true);
- ret = vmstate_save_state(f, vmsd, elm, vmdesc);
+ ret = vmstate_save_state(f, vmsd, elm, vmdesc, &local_err);
if (ret) {
- error_report("%s: failed to save %s (%d)", field->name,
- vmsd->name, ret);
+ error_report_err(local_err);
return ret;
}
}
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 8d1e9eb62bb9a7506604016b0ed858855e685776..ad8e5b71ae2ce78e66a6426602e5c20405ec57c0 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -406,12 +406,6 @@ bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque)
int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, JSONWriter *vmdesc_id)
-{
- return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id, NULL);
-}
-
-int vmstate_save_state_with_err(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);
@@ -512,7 +506,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
if (inner_field->flags & VMS_STRUCT) {
ret = vmstate_save_state(f, inner_field->vmsd,
- curr_elem, vmdesc_loop);
+ 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,
@@ -674,7 +668,7 @@ 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_with_err(f, vmsdsub, opaque, vmdesc, errp);
+ ret = vmstate_save_state(f, vmsdsub, opaque, vmdesc, errp);
if (ret) {
return ret;
}
diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 4ff0ab632f7e08b922dfcf565f31b0e63c17f59e..cadbab3c5e2609e217e436d470d00356704f2c8f 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -67,9 +67,13 @@ static QEMUFile *open_test_file(bool write)
static void save_vmstate(const VMStateDescription *desc, void *obj)
{
QEMUFile *f = open_test_file(true);
+ Error *local_err = NULL;
/* Save file with vmstate */
- int ret = vmstate_save_state(f, desc, obj, NULL);
+ int ret = vmstate_save_state(f, desc, obj, NULL, &local_err);
+ if (ret) {
+ error_report_err(local_err);
+ }
g_assert(!ret);
qemu_put_byte(f, QEMU_VM_EOF);
g_assert(!qemu_file_get_error(f));
@@ -438,10 +442,15 @@ static const VMStateDescription vmstate_skipping = {
static void test_save_noskip(void)
{
+ Error *local_err = NULL;
QEMUFile *fsave = open_test_file(true);
TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
.skip_c_e = false };
- int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);
+ int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL,
+ &local_err);
+ if (ret) {
+ error_report_err(local_err);
+ }
g_assert(!ret);
g_assert(!qemu_file_get_error(fsave));
@@ -460,10 +469,15 @@ static void test_save_noskip(void)
static void test_save_skip(void)
{
+ Error *local_err = NULL;
QEMUFile *fsave = open_test_file(true);
TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
.skip_c_e = true };
- int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);
+ int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL,
+ &local_err);
+ if (ret) {
+ error_report_err(local_err);
+ }
g_assert(!ret);
g_assert(!qemu_file_get_error(fsave));
diff --git a/ui/vdagent.c b/ui/vdagent.c
index bc3c77f01332a4b594cba00f3f9f9a5bca4d739a..ddb91e75c64a2af09d3f01e1b4900bd253513dd7 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -992,7 +992,8 @@ static int put_cbinfo(QEMUFile *f, void *pv, size_t size,
}
}
- return vmstate_save_state(f, &vmstate_cbinfo_array, &cbinfo, vmdesc);
+ return vmstate_save_state(f, &vmstate_cbinfo_array, &cbinfo, vmdesc,
+ &error_fatal);
}
static int get_cbinfo(QEMUFile *f, void *pv, size_t size,
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 25/27] migration: Rename post_save() to cleanup_save() and make it void
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (23 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 24/27] migration: Remove error variant of vmstate_save_state() function Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 26/27] migration: Add error-parameterized function variants in VMSD struct Arun Menon
2025-08-29 20:02 ` [PATCH v13 27/27] backends/tpm: Propagate vTPM error on migration failure Arun Menon
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
The post_save() function call is responsible for cleaning up resources
after the device state has been saved.
Currently it is infallible, and does not return an error.
It is called regardless of whether there is a preceding error from
save or subsection save. That is, save and post_save() together are
considered to be an atomic logical operation.
It should not be confused as a counterpart of the post_load() function
because post_load() does some sanity checks and returns an error if it
fails. This commit, therefore, renames post_save() to cleanup_save()
and makes it a void function.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
docs/devel/migration/main.rst | 2 +-
hw/ppc/spapr_pci.c | 5 ++---
include/migration/vmstate.h | 2 +-
migration/savevm.c | 5 ++---
migration/vmstate.c | 12 ++++--------
target/arm/machine.c | 6 ++----
6 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index 6493c1d2bca48a2fa34d92f6c0979c215c56b8d5..a39fec2e21c26c4315c0cf13b105176d70679d4d 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 (*cleanup_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 1ac1185825e84ca908fd878f6cbe7e8cacac1d89..135265f075dff502af59fbc91babca1f9a26c54d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2118,14 +2118,13 @@ static int spapr_pci_pre_save(void *opaque)
return 0;
}
-static int spapr_pci_post_save(void *opaque)
+static void spapr_pci_cleanup_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)
@@ -2152,7 +2151,7 @@ static const VMStateDescription vmstate_spapr_pci = {
.version_id = 2,
.minimum_version_id = 2,
.pre_save = spapr_pci_pre_save,
- .post_save = spapr_pci_post_save,
+ .cleanup_save = spapr_pci_cleanup_save,
.post_load = spapr_pci_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINT64_EQUAL(buid, SpaprPhbState, NULL),
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 5fe9bbf39058d0cf97c1adab54cc516dbe8dc32a..c1d8e5b7a7d9d544fc8ce181372660f5538ef66b 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -208,7 +208,7 @@ struct VMStateDescription {
int (*pre_load)(void *opaque);
int (*post_load)(void *opaque, int version_id);
int (*pre_save)(void *opaque);
- int (*post_save)(void *opaque);
+ void (*cleanup_save)(void *opaque);
bool (*needed)(void *opaque);
bool (*dev_unplug_pending)(void *opaque);
diff --git a/migration/savevm.c b/migration/savevm.c
index 2c05b763148295d7e2095027aa143f45bc4c4676..693f77b728604f2072fb40bd2ec268b217da9727 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -322,14 +322,13 @@ static int configuration_pre_save(void *opaque)
return 0;
}
-static int configuration_post_save(void *opaque)
+static void configuration_cleanup_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)
@@ -544,7 +543,7 @@ static const VMStateDescription vmstate_configuration = {
.pre_load = configuration_pre_load,
.post_load = configuration_post_load,
.pre_save = configuration_pre_save,
- .post_save = configuration_post_save,
+ .cleanup_save = configuration_cleanup_save,
.fields = (const VMStateField[]) {
VMSTATE_UINT32(len, SaveState),
VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
diff --git a/migration/vmstate.c b/migration/vmstate.c
index ad8e5b71ae2ce78e66a6426602e5c20405ec57c0..a7562a5cfd8627e20c90e286e35c5e3429c48150 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -529,8 +529,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);
- if (vmsd->post_save) {
- vmsd->post_save(opaque);
+ if (vmsd->cleanup_save) {
+ vmsd->cleanup_save(opaque);
}
return ret;
}
@@ -557,12 +557,8 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
- 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);
- }
+ if (vmsd->cleanup_save) {
+ vmsd->cleanup_save(opaque);
}
return ret;
}
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 6986915bee876402c1bd2efb92f41ca1298c66a3..d070114da15de85fd50af71e1d8caa84f25846d8 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -903,15 +903,13 @@ static int cpu_pre_save(void *opaque)
return 0;
}
-static int cpu_post_save(void *opaque)
+static void cpu_cleanup_save(void *opaque)
{
ARMCPU *cpu = opaque;
if (!kvm_enabled()) {
pmu_op_finish(&cpu->env);
}
-
- return 0;
}
static int cpu_pre_load(void *opaque)
@@ -1048,7 +1046,7 @@ const VMStateDescription vmstate_arm_cpu = {
.version_id = 22,
.minimum_version_id = 22,
.pre_save = cpu_pre_save,
- .post_save = cpu_post_save,
+ .cleanup_save = cpu_cleanup_save,
.pre_load = cpu_pre_load,
.post_load = cpu_post_load,
.fields = (const VMStateField[]) {
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 26/27] migration: Add error-parameterized function variants in VMSD struct
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (24 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 25/27] migration: Rename post_save() to cleanup_save() and make it void Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
2025-08-29 20:02 ` [PATCH v13 27/27] backends/tpm: Propagate vTPM error on migration failure Arun Menon
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon
- We need to have good error reporting in the callbacks in
VMStateDescription struct. Specifically pre_save, pre_load
and post_load callbacks.
- It is not possible to change these functions everywhere in one
patch, therefore, we introduce a duplicate set of callbacks
with Error object passed to them.
- So, in this commit, we implement 'errp' variants of these callbacks,
introducing an explicit Error object parameter.
- This is a functional step towards transitioning the entire codebase
to the new error-parameterized functions.
- Deliberately called in mutual exclusion from their counterparts,
to prevent conflicts during the transition.
- New impls should preferentally use 'errp' variants of
these methods, and existing impls incrementally converted.
The variants without 'errp' are intended to be removed
once all usage is converted.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
docs/devel/migration/main.rst | 19 +++++++++++++++++++
include/migration/vmstate.h | 14 ++++++++++++++
migration/vmstate.c | 31 ++++++++++++++++++++++++++++---
3 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index a39fec2e21c26c4315c0cf13b105176d70679d4d..93620102ae875e7477af2e26eeffe636ca0cdbca 100644
--- a/docs/devel/migration/main.rst
+++ b/docs/devel/migration/main.rst
@@ -444,6 +444,25 @@ The functions to do that are inside a vmstate definition, and are called:
This function is called after we save the state of one device
(even upon failure, unless the call to pre_save returned an error).
+Following are the errp variants of these functions.
+
+- ``int (*pre_load_errp)(void *opaque, Error **errp);``
+
+ This function is called before we load the state of one device.
+
+- ``int (*post_load_errp)(void *opaque, int version_id, Error **errp);``
+
+ This function is called after we load the state of one device.
+
+- ``int (*pre_save_errp)(void *opaque, Error **errp);``
+
+ This function is called before we save the state of one device.
+
+New impls should preferentally use 'errp' variants of these
+methods and existing impls incrementally converted.
+The variants without 'errp' are intended to be removed
+once all usage is converted.
+
Example: You can look at hpet.c, that uses the first three functions
to massage the state that is transferred.
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index c1d8e5b7a7d9d544fc8ce181372660f5538ef66b..647d7a35ce5f2c8b8ca5fdb15b54c10a0896cea8 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -200,14 +200,28 @@ struct VMStateDescription {
* exclusive. For this reason, also early_setup VMSDs are migrated in a
* QEMU_VM_SECTION_FULL section, while save_setup() data is migrated in
* a QEMU_VM_SECTION_START section.
+ *
+ * There are duplicate impls of the post/pre save/load hooks.
+ * New impls should preferentally use 'errp' variants of these
+ * methods and existing impls incrementally converted.
+ * The variants without 'errp' are intended to be removed
+ * once all usage is converted.
+ *
+ * For the errp variants,
+ * Returns: 0 on success,
+ * <0 on error where -value is an error number from errno.h
*/
+
bool early_setup;
int version_id;
int minimum_version_id;
MigrationPriority priority;
int (*pre_load)(void *opaque);
+ int (*pre_load_errp)(void *opaque, Error **errp);
int (*post_load)(void *opaque, int version_id);
+ int (*post_load_errp)(void *opaque, int version_id, Error **errp);
int (*pre_save)(void *opaque);
+ int (*pre_save_errp)(void *opaque, Error **errp);
void (*cleanup_save)(void *opaque);
bool (*needed)(void *opaque);
bool (*dev_unplug_pending)(void *opaque);
diff --git a/migration/vmstate.c b/migration/vmstate.c
index a7562a5cfd8627e20c90e286e35c5e3429c48150..86293593e0fbb8409409b22d3f0653327d727c32 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -134,6 +134,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id, Error **errp)
{
+ ERRP_GUARD();
const VMStateField *field = vmsd->fields;
int ret = 0;
@@ -152,7 +153,16 @@ 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) {
+ if (vmsd->pre_load_errp) {
+ ret = vmsd->pre_load_errp(opaque, errp);
+ if (ret < 0) {
+ error_prepend(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;
+ }
+ } else if (vmsd->pre_load) {
ret = vmsd->pre_load(opaque);
if (ret) {
error_setg(errp, "pre load hook failed for: '%s', "
@@ -245,7 +255,14 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
qemu_file_set_error(f, ret);
return ret;
}
- if (vmsd->post_load) {
+ if (vmsd->post_load_errp) {
+ ret = vmsd->post_load_errp(opaque, version_id, errp);
+ if (ret < 0) {
+ error_prepend(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);
+ }
+ } else if (vmsd->post_load) {
ret = vmsd->post_load(opaque, version_id);
if (ret < 0) {
error_setg(errp,
@@ -414,12 +431,20 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc, int version_id, Error **errp)
{
+ ERRP_GUARD();
int ret = 0;
const VMStateField *field = vmsd->fields;
trace_vmstate_save_state_top(vmsd->name);
- if (vmsd->pre_save) {
+ if (vmsd->pre_save_errp) {
+ ret = vmsd->pre_save_errp(opaque, errp);
+ trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
+ if (ret < 0) {
+ error_prepend(errp, "pre-save for %s failed, ret: %d: ",
+ vmsd->name, ret);
+ }
+ } else if (vmsd->pre_save) {
ret = vmsd->pre_save(opaque);
trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
if (ret) {
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v13 27/27] backends/tpm: Propagate vTPM error on migration failure
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
` (25 preceding siblings ...)
2025-08-29 20:02 ` [PATCH v13 26/27] migration: Add error-parameterized function variants in VMSD struct Arun Menon
@ 2025-08-29 20:02 ` Arun Menon
26 siblings, 0 replies; 35+ messages in thread
From: Arun Menon @ 2025-08-29 20:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Akihiko Odaki,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm, Arun Menon, Daniel P. Berrangé,
Stefan Berger
- When migration of a VM with encrypted vTPM fails on the
destination host, (e.g., due to a mismatch in secret values),
the error message displayed on the source host is generic and unhelpful.
- For example, a typical error looks like this:
"operation failed: job 'migration out' failed: Sibling indicated error 1.
operation failed: job 'migration in' failed: load of migration failed:
Input/output error"
- Such generic errors are logged using error_report(), which prints to
the console/monitor but does not make the detailed error accessible via
the QMP query-migrate command.
- This change, along with the set of changes of passing errp Error object
to the VM state loading functions, help in addressing the issue.
We use the post_load_errp hook of VMStateDescription to propagate errors
by setting Error **errp objects in case of failure in the TPM backend.
- It can then be retrieved using QMP command:
{"execute" : "query-migrate"}
Buglink: https://issues.redhat.com/browse/RHEL-82826
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
backends/tpm/tpm_emulator.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 4a234ab2c0b19b2604bf0dd8cb5f4540c72a9438..dacfca5ab7eb0445ddc1ced97513068501b468bf 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -819,7 +819,8 @@ static int tpm_emulator_get_state_blobs(TPMEmulator *tpm_emu)
static int tpm_emulator_set_state_blob(TPMEmulator *tpm_emu,
uint32_t type,
TPMSizedBuffer *tsb,
- uint32_t flags)
+ uint32_t flags,
+ Error **errp)
{
ssize_t n;
ptm_setstate pss;
@@ -838,17 +839,18 @@ static int tpm_emulator_set_state_blob(TPMEmulator *tpm_emu,
/* write the header only */
if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_STATEBLOB, &pss,
offsetof(ptm_setstate, u.req.data), 0, 0) < 0) {
- error_report("tpm-emulator: could not set state blob type %d : %s",
- type, strerror(errno));
+ error_setg_errno(errp, errno,
+ "tpm-emulator: could not set state blob type %d",
+ type);
return -1;
}
/* now the body */
n = qemu_chr_fe_write_all(&tpm_emu->ctrl_chr, tsb->buffer, tsb->size);
if (n != tsb->size) {
- error_report("tpm-emulator: Writing the stateblob (type %d) "
- "failed; could not write %u bytes, but only %zd",
- type, tsb->size, n);
+ error_setg(errp, "tpm-emulator: Writing the stateblob (type %d) "
+ "failed; could not write %u bytes, but only %zd",
+ type, tsb->size, n);
return -1;
}
@@ -856,17 +858,17 @@ static int tpm_emulator_set_state_blob(TPMEmulator *tpm_emu,
n = qemu_chr_fe_read_all(&tpm_emu->ctrl_chr,
(uint8_t *)&pss, sizeof(pss.u.resp));
if (n != sizeof(pss.u.resp)) {
- error_report("tpm-emulator: Reading response from writing stateblob "
- "(type %d) failed; expected %zu bytes, got %zd", type,
- sizeof(pss.u.resp), n);
+ error_setg(errp, "tpm-emulator: Reading response from writing "
+ "stateblob (type %d) failed; expected %zu bytes, "
+ "got %zd", type, sizeof(pss.u.resp), n);
return -1;
}
tpm_result = be32_to_cpu(pss.u.resp.tpm_result);
if (tpm_result != 0) {
- error_report("tpm-emulator: Setting the stateblob (type %d) failed "
- "with a TPM error 0x%x %s", type, tpm_result,
- tpm_emulator_strerror(tpm_result));
+ error_setg(errp, "tpm-emulator: Setting the stateblob (type %d) "
+ "failed with a TPM error 0x%x %s", type, tpm_result,
+ tpm_emulator_strerror(tpm_result));
return -1;
}
@@ -880,7 +882,7 @@ static int tpm_emulator_set_state_blob(TPMEmulator *tpm_emu,
*
* Returns a negative errno code in case of error.
*/
-static int tpm_emulator_set_state_blobs(TPMBackend *tb)
+static int tpm_emulator_set_state_blobs(TPMBackend *tb, Error **errp)
{
TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
TPMBlobBuffers *state_blobs = &tpm_emu->state_blobs;
@@ -894,13 +896,13 @@ static int tpm_emulator_set_state_blobs(TPMBackend *tb)
if (tpm_emulator_set_state_blob(tpm_emu, PTM_BLOB_TYPE_PERMANENT,
&state_blobs->permanent,
- state_blobs->permanent_flags) < 0 ||
+ state_blobs->permanent_flags, errp) < 0 ||
tpm_emulator_set_state_blob(tpm_emu, PTM_BLOB_TYPE_VOLATILE,
&state_blobs->volatil,
- state_blobs->volatil_flags) < 0 ||
+ state_blobs->volatil_flags, errp) < 0 ||
tpm_emulator_set_state_blob(tpm_emu, PTM_BLOB_TYPE_SAVESTATE,
&state_blobs->savestate,
- state_blobs->savestate_flags) < 0) {
+ state_blobs->savestate_flags, errp) < 0) {
return -EIO;
}
@@ -948,12 +950,12 @@ static void tpm_emulator_vm_state_change(void *opaque, bool running,
*
* Returns negative errno codes in case of error.
*/
-static int tpm_emulator_post_load(void *opaque, int version_id)
+static int tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
{
TPMBackend *tb = opaque;
int ret;
- ret = tpm_emulator_set_state_blobs(tb);
+ ret = tpm_emulator_set_state_blobs(tb, errp);
if (ret < 0) {
return ret;
}
@@ -969,7 +971,7 @@ static const VMStateDescription vmstate_tpm_emulator = {
.name = "tpm-emulator",
.version_id = 0,
.pre_save = tpm_emulator_pre_save,
- .post_load = tpm_emulator_post_load,
+ .post_load_errp = tpm_emulator_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINT32(state_blobs.permanent_flags, TPMEmulator),
VMSTATE_UINT32(state_blobs.permanent.size, TPMEmulator),
--
2.51.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-29 20:01 ` [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state() Arun Menon
@ 2025-08-30 5:58 ` Akihiko Odaki
2025-08-31 15:45 ` Arun Menon
0 siblings, 1 reply; 35+ messages in thread
From: Akihiko Odaki @ 2025-08-30 5:58 UTC (permalink / raw)
To: Arun Menon, qemu-devel
Cc: Peter Xu, Fabiano Rosas, Alex Bennée, Dmitry Osipenko,
Michael S. Tsirkin, Marcel Apfelbaum, Cornelia Huck, Halil Pasic,
Eric Farman, Thomas Huth, Christian Borntraeger, Matthew Rosato,
Richard Henderson, David Hildenbrand, Ilya Leoshkevich,
Nicholas Piggin, Harsh Prateek Bora, Paolo Bonzini, Fam Zheng,
Alex Williamson, Cédric Le Goater, Steve Sistare,
Marc-André Lureau, qemu-s390x, qemu-ppc, Hailiang Zhang,
Stefan Berger, Peter Maydell, qemu-arm
On 2025/08/30 5:01, Arun Menon wrote:
> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> It is ensured that qemu_loadvm_state() must report an error
> in errp, in case of failure.
>
> When postcopy live migration runs, the device states are loaded by
> both the qemu coroutine process_incoming_migration_co() and the
> postcopy_ram_listen_thread(). Therefore, it is important that the
> coroutine also reports the error in case of failure, with
> error_report_err(). Otherwise, the source qemu will not display
> any errors before going into the postcopy pause state.
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
> migration/migration.c | 9 +++++----
> migration/savevm.c | 30 ++++++++++++++++++------------
> migration/savevm.h | 2 +-
> 3 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
> MIGRATION_STATUS_ACTIVE);
>
> mis->loadvm_co = qemu_coroutine_self();
> - ret = qemu_loadvm_state(mis->from_src_file);
> + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
> mis->loadvm_co = NULL;
>
> trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
> @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
> }
>
> if (ret < 0) {
> - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
> + error_prepend(&local_err, "load of migration failed: %s: ",
> + strerror(-ret));
> goto fail;
> }
>
> @@ -924,13 +925,13 @@ fail:
> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> MIGRATION_STATUS_FAILED);
> migrate_set_error(s, local_err);
> - error_free(local_err);
> + error_report_err(local_err);
This is problematic because it results in duplicate error reports when
!mis->exit_on_error; in that case the query-migrate QMP command reports
the error and this error reporting is redundant.
>
> migration_incoming_state_destroy();
>
> if (mis->exit_on_error) {
> WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
> - error_report_err(s->error);
> + error_free(s->error);
This change is problematic because s->error set somewhere else here will
be ignored.
I think the two changes I commented can be simply removed without
causing other problems.
> s->error = NULL;
> }
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index de5671ffd1cd06e728227a3056c3f895d3a6e6f3..0087fca15ce108685667d3808350d80d37b807b1 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -3159,28 +3159,24 @@ out:
> return ret;
> }
>
> -int qemu_loadvm_state(QEMUFile *f)
> +int qemu_loadvm_state(QEMUFile *f, Error **errp)
> {
> MigrationState *s = migrate_get_current();
> MigrationIncomingState *mis = migration_incoming_get_current();
> - Error *local_err = NULL;
> int ret;
>
> - if (qemu_savevm_state_blocked(&local_err)) {
> - error_report_err(local_err);
> + if (qemu_savevm_state_blocked(errp)) {
> return -EINVAL;
> }
>
> qemu_loadvm_thread_pool_create(mis);
>
> - ret = qemu_loadvm_state_header(f, &local_err);
> + ret = qemu_loadvm_state_header(f, errp);
> if (ret) {
> - error_report_err(local_err);
> return ret;
> }
>
> - if (qemu_loadvm_state_setup(f, &local_err) != 0) {
> - error_report_err(local_err);
> + if (qemu_loadvm_state_setup(f, errp) != 0) {
> return -EINVAL;
> }
>
> @@ -3191,6 +3187,9 @@ int qemu_loadvm_state(QEMUFile *f)
> cpu_synchronize_all_pre_loadvm();
>
> ret = qemu_loadvm_state_main(f, mis);
> + if (ret < 0) {
> + error_setg(errp, "Load VM state failed: %d", ret);
> + }
> qemu_event_set(&mis->main_thread_load_event);
>
> trace_qemu_loadvm_state_post_main(ret);
> @@ -3208,8 +3207,15 @@ int qemu_loadvm_state(QEMUFile *f)
> if (migrate_has_error(migrate_get_current()) ||
> !qemu_loadvm_thread_pool_wait(s, mis)) {
> ret = -EINVAL;
> + error_setg(errp,
> + "Error while loading vmstate");
> } else {
> ret = qemu_file_get_error(f);
> + if (ret < 0) {
> + error_setg(errp,
> + "Error while loading vmstate: stream error: %d",
> + ret);
> + }
> }
> }
> /*
> @@ -3474,6 +3480,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
>
> void qmp_xen_load_devices_state(const char *filename, Error **errp)
> {
> + ERRP_GUARD();
> QEMUFile *f;
> QIOChannelFile *ioc;
> int ret;
> @@ -3495,10 +3502,10 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> f = qemu_file_new_input(QIO_CHANNEL(ioc));
> object_unref(OBJECT(ioc));
>
> - ret = qemu_loadvm_state(f);
> + ret = qemu_loadvm_state(f, errp);
> qemu_fclose(f);
> if (ret < 0) {
> - error_setg(errp, "loading Xen device state failed");
> + error_prepend(errp, "loading Xen device state failed: ");
> }
> migration_incoming_state_destroy();
> }
> @@ -3569,13 +3576,12 @@ bool load_snapshot(const char *name, const char *vmstate,
> ret = -EINVAL;
> goto err_drain;
> }
> - ret = qemu_loadvm_state(f);
> + ret = qemu_loadvm_state(f, errp);
> migration_incoming_state_destroy();
>
> bdrv_drain_all_end();
>
> if (ret < 0) {
> - error_setg(errp, "Error %d while loading VM state", ret);
> return false;
> }
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 2d5e9c716686f06720325e82fe90c75335ced1de..b80770b7461a60e2ad6ba5e24a7baeae73d90955 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -64,7 +64,7 @@ void qemu_savevm_send_colo_enable(QEMUFile *f);
> void qemu_savevm_live_state(QEMUFile *f);
> int qemu_save_device_state(QEMUFile *f);
>
> -int qemu_loadvm_state(QEMUFile *f);
> +int qemu_loadvm_state(QEMUFile *f, Error **errp);
> void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> int qemu_load_device_state(QEMUFile *f);
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-30 5:58 ` Akihiko Odaki
@ 2025-08-31 15:45 ` Arun Menon
2025-08-31 16:04 ` Akihiko Odaki
0 siblings, 1 reply; 35+ messages in thread
From: Arun Menon @ 2025-08-31 15:45 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Alex Bennée,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm
Hi Akihiko,
Thanks for the review.
On Sat, Aug 30, 2025 at 02:58:05PM +0900, Akihiko Odaki wrote:
> On 2025/08/30 5:01, Arun Menon wrote:
> > This is an incremental step in converting vmstate loading
> > code to report error via Error objects instead of directly
> > printing it to console/monitor.
> > It is ensured that qemu_loadvm_state() must report an error
> > in errp, in case of failure.
> >
> > When postcopy live migration runs, the device states are loaded by
> > both the qemu coroutine process_incoming_migration_co() and the
> > postcopy_ram_listen_thread(). Therefore, it is important that the
> > coroutine also reports the error in case of failure, with
> > error_report_err(). Otherwise, the source qemu will not display
> > any errors before going into the postcopy pause state.
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > ---
> > migration/migration.c | 9 +++++----
> > migration/savevm.c | 30 ++++++++++++++++++------------
> > migration/savevm.h | 2 +-
> > 3 files changed, 24 insertions(+), 17 deletions(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
> > MIGRATION_STATUS_ACTIVE);
> > mis->loadvm_co = qemu_coroutine_self();
> > - ret = qemu_loadvm_state(mis->from_src_file);
> > + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
> > mis->loadvm_co = NULL;
> > trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
> > @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
> > }
> > if (ret < 0) {
> > - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
> > + error_prepend(&local_err, "load of migration failed: %s: ",
> > + strerror(-ret));
> > goto fail;
> > }
> > @@ -924,13 +925,13 @@ fail:
> > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> > MIGRATION_STATUS_FAILED);
> > migrate_set_error(s, local_err);
> > - error_free(local_err);
> > + error_report_err(local_err);
>
> This is problematic because it results in duplicate error reports when
> !mis->exit_on_error; in that case the query-migrate QMP command reports the
> error and this error reporting is redundant.
If I comment this change, then all of the errors propagated up to now, using
error_setg() will not be reported. This is the place where it is finally reported,
when qemu_loadvm_state() fails. In other words, all the error_reports() we removed
from all the files, replacing them with error_setg(), will finally be reported here
using error_report_err().
>
> > migration_incoming_state_destroy();
> > if (mis->exit_on_error) {
> > WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
> > - error_report_err(s->error);
> > + error_free(s->error);
>
> This change is problematic because s->error set somewhere else here will be
> ignored.
This is specific to the case when mis->exit_on_error is set.
since we did a migrate_set_error(s, local_err) before, we free the
error in s->error and set it to NULL, before an exit(EXIT_FAILURE)
>
> I think the two changes I commented can be simply removed without causing
> other problems.
Please correct me if I am wrong.
>
> > s->error = NULL;
> > }
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index de5671ffd1cd06e728227a3056c3f895d3a6e6f3..0087fca15ce108685667d3808350d80d37b807b1 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -3159,28 +3159,24 @@ out:
> > return ret;
> > }
> > -int qemu_loadvm_state(QEMUFile *f)
> > +int qemu_loadvm_state(QEMUFile *f, Error **errp)
> > {
> > MigrationState *s = migrate_get_current();
> > MigrationIncomingState *mis = migration_incoming_get_current();
> > - Error *local_err = NULL;
> > int ret;
> > - if (qemu_savevm_state_blocked(&local_err)) {
> > - error_report_err(local_err);
> > + if (qemu_savevm_state_blocked(errp)) {
> > return -EINVAL;
> > }
> > qemu_loadvm_thread_pool_create(mis);
> > - ret = qemu_loadvm_state_header(f, &local_err);
> > + ret = qemu_loadvm_state_header(f, errp);
> > if (ret) {
> > - error_report_err(local_err);
> > return ret;
> > }
> > - if (qemu_loadvm_state_setup(f, &local_err) != 0) {
> > - error_report_err(local_err);
> > + if (qemu_loadvm_state_setup(f, errp) != 0) {
> > return -EINVAL;
> > }
> > @@ -3191,6 +3187,9 @@ int qemu_loadvm_state(QEMUFile *f)
> > cpu_synchronize_all_pre_loadvm();
> > ret = qemu_loadvm_state_main(f, mis);
> > + if (ret < 0) {
> > + error_setg(errp, "Load VM state failed: %d", ret);
> > + }
> > qemu_event_set(&mis->main_thread_load_event);
> > trace_qemu_loadvm_state_post_main(ret);
> > @@ -3208,8 +3207,15 @@ int qemu_loadvm_state(QEMUFile *f)
> > if (migrate_has_error(migrate_get_current()) ||
> > !qemu_loadvm_thread_pool_wait(s, mis)) {
> > ret = -EINVAL;
> > + error_setg(errp,
> > + "Error while loading vmstate");
> > } else {
> > ret = qemu_file_get_error(f);
> > + if (ret < 0) {
> > + error_setg(errp,
> > + "Error while loading vmstate: stream error: %d",
> > + ret);
> > + }
> > }
> > }
> > /*
> > @@ -3474,6 +3480,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
> > void qmp_xen_load_devices_state(const char *filename, Error **errp)
> > {
> > + ERRP_GUARD();
> > QEMUFile *f;
> > QIOChannelFile *ioc;
> > int ret;
> > @@ -3495,10 +3502,10 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> > f = qemu_file_new_input(QIO_CHANNEL(ioc));
> > object_unref(OBJECT(ioc));
> > - ret = qemu_loadvm_state(f);
> > + ret = qemu_loadvm_state(f, errp);
> > qemu_fclose(f);
> > if (ret < 0) {
> > - error_setg(errp, "loading Xen device state failed");
> > + error_prepend(errp, "loading Xen device state failed: ");
> > }
> > migration_incoming_state_destroy();
> > }
> > @@ -3569,13 +3576,12 @@ bool load_snapshot(const char *name, const char *vmstate,
> > ret = -EINVAL;
> > goto err_drain;
> > }
> > - ret = qemu_loadvm_state(f);
> > + ret = qemu_loadvm_state(f, errp);
> > migration_incoming_state_destroy();
> > bdrv_drain_all_end();
> > if (ret < 0) {
> > - error_setg(errp, "Error %d while loading VM state", ret);
> > return false;
> > }
> > diff --git a/migration/savevm.h b/migration/savevm.h
> > index 2d5e9c716686f06720325e82fe90c75335ced1de..b80770b7461a60e2ad6ba5e24a7baeae73d90955 100644
> > --- a/migration/savevm.h
> > +++ b/migration/savevm.h
> > @@ -64,7 +64,7 @@ void qemu_savevm_send_colo_enable(QEMUFile *f);
> > void qemu_savevm_live_state(QEMUFile *f);
> > int qemu_save_device_state(QEMUFile *f);
> > -int qemu_loadvm_state(QEMUFile *f);
> > +int qemu_loadvm_state(QEMUFile *f, Error **errp);
> > void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> > int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> > int qemu_load_device_state(QEMUFile *f);
> >
>
Regards,
Arun Menon
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-31 15:45 ` Arun Menon
@ 2025-08-31 16:04 ` Akihiko Odaki
2025-08-31 16:38 ` Arun Menon
0 siblings, 1 reply; 35+ messages in thread
From: Akihiko Odaki @ 2025-08-31 16:04 UTC (permalink / raw)
To: armenon
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Alex Bennée,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm
On 2025/09/01 0:45, Arun Menon wrote:
> Hi Akihiko,
> Thanks for the review.
>
> On Sat, Aug 30, 2025 at 02:58:05PM +0900, Akihiko Odaki wrote:
>> On 2025/08/30 5:01, Arun Menon wrote:
>>> This is an incremental step in converting vmstate loading
>>> code to report error via Error objects instead of directly
>>> printing it to console/monitor.
>>> It is ensured that qemu_loadvm_state() must report an error
>>> in errp, in case of failure.
>>>
>>> When postcopy live migration runs, the device states are loaded by
>>> both the qemu coroutine process_incoming_migration_co() and the
>>> postcopy_ram_listen_thread(). Therefore, it is important that the
>>> coroutine also reports the error in case of failure, with
>>> error_report_err(). Otherwise, the source qemu will not display
>>> any errors before going into the postcopy pause state.
>>>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>> Signed-off-by: Arun Menon <armenon@redhat.com>
>>> ---
>>> migration/migration.c | 9 +++++----
>>> migration/savevm.c | 30 ++++++++++++++++++------------
>>> migration/savevm.h | 2 +-
>>> 3 files changed, 24 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
>>> MIGRATION_STATUS_ACTIVE);
>>> mis->loadvm_co = qemu_coroutine_self();
>>> - ret = qemu_loadvm_state(mis->from_src_file);
>>> + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
>>> mis->loadvm_co = NULL;
>>> trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
>>> @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
>>> }
>>> if (ret < 0) {
>>> - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
>>> + error_prepend(&local_err, "load of migration failed: %s: ",
>>> + strerror(-ret));
>>> goto fail;
>>> }
>>> @@ -924,13 +925,13 @@ fail:
>>> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>>> MIGRATION_STATUS_FAILED);
>>> migrate_set_error(s, local_err);
>>> - error_free(local_err);
>>> + error_report_err(local_err);
>>
>> This is problematic because it results in duplicate error reports when
>> !mis->exit_on_error; in that case the query-migrate QMP command reports the
>> error and this error reporting is redundant.
>
> If I comment this change, then all of the errors propagated up to now, using
> error_setg() will not be reported. This is the place where it is finally reported,
> when qemu_loadvm_state() fails. In other words, all the error_reports() we removed
> from all the files, replacing them with error_setg(), will finally be reported here
> using error_report_err().
My understanding of the code without these two changes is:
- If the migrate-incoming QMP command is used with false as
exit-on-error, this function will not report the error but
the query-migrate QMP command will report the error.
- Otherwise, this function reports the error.
With these two changes, if the migrate-incoming QMP command is used with
false as exit-on-error, this function will report the error *and* the
query-migrate QMP command will report the error, resulting in duplicate
reports.
>
>>
>>> migration_incoming_state_destroy();
>>> if (mis->exit_on_error) {
>>> WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
>>> - error_report_err(s->error);
>>> + error_free(s->error);
>>
>> This change is problematic because s->error set somewhere else here will be
>> ignored.
>
> This is specific to the case when mis->exit_on_error is set.
> since we did a migrate_set_error(s, local_err) before, we free the
> error in s->error and set it to NULL, before an exit(EXIT_FAILURE)
It shouldn't just free the error but should print it or the error will
be missed.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-31 16:04 ` Akihiko Odaki
@ 2025-08-31 16:38 ` Arun Menon
2025-08-31 17:12 ` Akihiko Odaki
0 siblings, 1 reply; 35+ messages in thread
From: Arun Menon @ 2025-08-31 16:38 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Alex Bennée,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm
Hi,
On Mon, Sep 01, 2025 at 01:04:40AM +0900, Akihiko Odaki wrote:
> On 2025/09/01 0:45, Arun Menon wrote:
> > Hi Akihiko,
> > Thanks for the review.
> >
> > On Sat, Aug 30, 2025 at 02:58:05PM +0900, Akihiko Odaki wrote:
> > > On 2025/08/30 5:01, Arun Menon wrote:
> > > > This is an incremental step in converting vmstate loading
> > > > code to report error via Error objects instead of directly
> > > > printing it to console/monitor.
> > > > It is ensured that qemu_loadvm_state() must report an error
> > > > in errp, in case of failure.
> > > >
> > > > When postcopy live migration runs, the device states are loaded by
> > > > both the qemu coroutine process_incoming_migration_co() and the
> > > > postcopy_ram_listen_thread(). Therefore, it is important that the
> > > > coroutine also reports the error in case of failure, with
> > > > error_report_err(). Otherwise, the source qemu will not display
> > > > any errors before going into the postcopy pause state.
> > > >
> > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > > > Signed-off-by: Arun Menon <armenon@redhat.com>
> > > > ---
> > > > migration/migration.c | 9 +++++----
> > > > migration/savevm.c | 30 ++++++++++++++++++------------
> > > > migration/savevm.h | 2 +-
> > > > 3 files changed, 24 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
> > > > MIGRATION_STATUS_ACTIVE);
> > > > mis->loadvm_co = qemu_coroutine_self();
> > > > - ret = qemu_loadvm_state(mis->from_src_file);
> > > > + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
> > > > mis->loadvm_co = NULL;
> > > > trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
> > > > @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
> > > > }
> > > > if (ret < 0) {
> > > > - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
> > > > + error_prepend(&local_err, "load of migration failed: %s: ",
> > > > + strerror(-ret));
> > > > goto fail;
> > > > }
> > > > @@ -924,13 +925,13 @@ fail:
> > > > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> > > > MIGRATION_STATUS_FAILED);
> > > > migrate_set_error(s, local_err);
> > > > - error_free(local_err);
> > > > + error_report_err(local_err);
> > >
> > > This is problematic because it results in duplicate error reports when
> > > !mis->exit_on_error; in that case the query-migrate QMP command reports the
> > > error and this error reporting is redundant.
> >
> > If I comment this change, then all of the errors propagated up to now, using
> > error_setg() will not be reported. This is the place where it is finally reported,
> > when qemu_loadvm_state() fails. In other words, all the error_reports() we removed
> > from all the files, replacing them with error_setg(), will finally be reported here
> > using error_report_err().
>
> My understanding of the code without these two changes is:
> - If the migrate-incoming QMP command is used with false as
> exit-on-error, this function will not report the error but
> the query-migrate QMP command will report the error.
> - Otherwise, this function reports the error.
With my limited experience in testing, I have a question,
So there are 2 scenarios,
1. running the virsh migrate command on the source host. Something like the following,
virsh -c 'qemu:///system' migrate --live --verbose --domain guest-vm --desturi qemu+ssh://10.6.120.20/system
OR for postcopy-ram,
virsh migrate guest-vm --live qemu+ssh://10.6.120.20/system --verbose --postcopy --timeout 10 --timeout-postcopy
2. Using QMP commands, performing a migration from source to destination.
Running something like the following on the destination:
{
"execute": "migrate-incoming",
"arguments": {
"uri": "tcp:127.0.0.1:7777",
"exit-on-error": false
}
}
{
"execute": "migrate-incoming",
"arguments": {
"uri": "tcp:127.0.0.1:7777",
"exit-on-error": false
}
}
and the somthing like the following on source:
{
"execute": "migrate",
"arguments": {
"uri": "tcp:127.0.0.1:7777"
}
}
{"execute" : "query-migrate"}
In 1, previously, the user used to get an error message on migration failure.
This was because there were error_report() calls in all of the files.
Now that they are replaced with error_setg() and the error is stored in errp,
we need to display that using error_report_err(). Hence I introduced an error_report_err()
call in the fail section.
In 2, we have 2 QMP sessions, one for the source and another for the destination.
The QMP command migrate will be issued on the source, and the errp will be set.
I did not understand the part where the message will be displayed because of the
error_report_err() call. I did not see such a message on failure scenario on both
the sessions.
If the user wants to check for errors, then the destination qemu will not exit
(exit-on-error = false ) and we can retrieve it using {"execute" : "query-migrate"}
Aren't the 2 scenarios different by nature?
>
> With these two changes, if the migrate-incoming QMP command is used with
> false as exit-on-error, this function will report the error *and* the
> query-migrate QMP command will report the error, resulting in duplicate
> reports.
>
> >
> > >
> > > > migration_incoming_state_destroy();
> > > > if (mis->exit_on_error) {
> > > > WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
> > > > - error_report_err(s->error);
> > > > + error_free(s->error);
> > >
> > > This change is problematic because s->error set somewhere else here will be
> > > ignored.
> >
> > This is specific to the case when mis->exit_on_error is set.
> > since we did a migrate_set_error(s, local_err) before, we free the
> > error in s->error and set it to NULL, before an exit(EXIT_FAILURE)
>
> It shouldn't just free the error but should print it or the error will be
> missed.
>
> Regards,
> Akihiko Odaki
>
Regards,
Arun Menon
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-31 16:38 ` Arun Menon
@ 2025-08-31 17:12 ` Akihiko Odaki
2025-09-03 6:47 ` Arun Menon
0 siblings, 1 reply; 35+ messages in thread
From: Akihiko Odaki @ 2025-08-31 17:12 UTC (permalink / raw)
To: armenon
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Alex Bennée,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm
On 2025/09/01 1:38, Arun Menon wrote:
> Hi,
>
> On Mon, Sep 01, 2025 at 01:04:40AM +0900, Akihiko Odaki wrote:
>> On 2025/09/01 0:45, Arun Menon wrote:
>>> Hi Akihiko,
>>> Thanks for the review.
>>>
>>> On Sat, Aug 30, 2025 at 02:58:05PM +0900, Akihiko Odaki wrote:
>>>> On 2025/08/30 5:01, Arun Menon wrote:
>>>>> This is an incremental step in converting vmstate loading
>>>>> code to report error via Error objects instead of directly
>>>>> printing it to console/monitor.
>>>>> It is ensured that qemu_loadvm_state() must report an error
>>>>> in errp, in case of failure.
>>>>>
>>>>> When postcopy live migration runs, the device states are loaded by
>>>>> both the qemu coroutine process_incoming_migration_co() and the
>>>>> postcopy_ram_listen_thread(). Therefore, it is important that the
>>>>> coroutine also reports the error in case of failure, with
>>>>> error_report_err(). Otherwise, the source qemu will not display
>>>>> any errors before going into the postcopy pause state.
>>>>>
>>>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>>>> Signed-off-by: Arun Menon <armenon@redhat.com>
>>>>> ---
>>>>> migration/migration.c | 9 +++++----
>>>>> migration/savevm.c | 30 ++++++++++++++++++------------
>>>>> migration/savevm.h | 2 +-
>>>>> 3 files changed, 24 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/migration/migration.c b/migration/migration.c
>>>>> index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
>>>>> --- a/migration/migration.c
>>>>> +++ b/migration/migration.c
>>>>> @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
>>>>> MIGRATION_STATUS_ACTIVE);
>>>>> mis->loadvm_co = qemu_coroutine_self();
>>>>> - ret = qemu_loadvm_state(mis->from_src_file);
>>>>> + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
>>>>> mis->loadvm_co = NULL;
>>>>> trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
>>>>> @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
>>>>> }
>>>>> if (ret < 0) {
>>>>> - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
>>>>> + error_prepend(&local_err, "load of migration failed: %s: ",
>>>>> + strerror(-ret));
>>>>> goto fail;
>>>>> }
>>>>> @@ -924,13 +925,13 @@ fail:
>>>>> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>>>>> MIGRATION_STATUS_FAILED);
>>>>> migrate_set_error(s, local_err);
>>>>> - error_free(local_err);
>>>>> + error_report_err(local_err);
>>>>
>>>> This is problematic because it results in duplicate error reports when
>>>> !mis->exit_on_error; in that case the query-migrate QMP command reports the
>>>> error and this error reporting is redundant.
>>>
>>> If I comment this change, then all of the errors propagated up to now, using
>>> error_setg() will not be reported. This is the place where it is finally reported,
>>> when qemu_loadvm_state() fails. In other words, all the error_reports() we removed
>>> from all the files, replacing them with error_setg(), will finally be reported here
>>> using error_report_err().
>>
>> My understanding of the code without these two changes is:
>> - If the migrate-incoming QMP command is used with false as
>> exit-on-error, this function will not report the error but
>> the query-migrate QMP command will report the error.
>> - Otherwise, this function reports the error.
>
> With my limited experience in testing, I have a question,
> So there are 2 scenarios,
> 1. running the virsh migrate command on the source host. Something like the following,
> virsh -c 'qemu:///system' migrate --live --verbose --domain guest-vm --desturi qemu+ssh://10.6.120.20/system
> OR for postcopy-ram,
> virsh migrate guest-vm --live qemu+ssh://10.6.120.20/system --verbose --postcopy --timeout 10 --timeout-postcopy
>
> 2. Using QMP commands, performing a migration from source to destination.
> Running something like the following on the destination:
> {
> "execute": "migrate-incoming",
> "arguments": {
> "uri": "tcp:127.0.0.1:7777",
> "exit-on-error": false
> }
> }
> {
> "execute": "migrate-incoming",
> "arguments": {
> "uri": "tcp:127.0.0.1:7777",
> "exit-on-error": false
> }
> }
> and the somthing like the following on source:
> {
> "execute": "migrate",
> "arguments": {
> "uri": "tcp:127.0.0.1:7777"
> }
> }
> {"execute" : "query-migrate"}
>
> In 1, previously, the user used to get an error message on migration failure.
> This was because there were error_report() calls in all of the files.
> Now that they are replaced with error_setg() and the error is stored in errp,
> we need to display that using error_report_err(). Hence I introduced an error_report_err()
> call in the fail section.
>
> In 2, we have 2 QMP sessions, one for the source and another for the destination.
> The QMP command migrate will be issued on the source, and the errp will be set.
> I did not understand the part where the message will be displayed because of the
> error_report_err() call. I did not see such a message on failure scenario on both
> the sessions.
> If the user wants to check for errors, then the destination qemu will not exit
> (exit-on-error = false ) and we can retrieve it using {"execute" : "query-migrate"}
>
> Aren't the 2 scenarios different by nature?
In 1, doesn't libvirt query the error with query-migrate and print it?
In any case, it would be nice if you describe how libvirt interacts with
QEMU in 1.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-08-31 17:12 ` Akihiko Odaki
@ 2025-09-03 6:47 ` Arun Menon
2025-09-06 3:22 ` Akihiko Odaki
0 siblings, 1 reply; 35+ messages in thread
From: Arun Menon @ 2025-09-03 6:47 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Alex Bennée,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm
Hi Akihiko,
It took some time to set up the machines; apologies for the delay in response.
On Mon, Sep 01, 2025 at 02:12:54AM +0900, Akihiko Odaki wrote:
> On 2025/09/01 1:38, Arun Menon wrote:
> > Hi,
> >
> > On Mon, Sep 01, 2025 at 01:04:40AM +0900, Akihiko Odaki wrote:
> > > On 2025/09/01 0:45, Arun Menon wrote:
> > > > Hi Akihiko,
> > > > Thanks for the review.
> > > >
> > > > On Sat, Aug 30, 2025 at 02:58:05PM +0900, Akihiko Odaki wrote:
> > > > > On 2025/08/30 5:01, Arun Menon wrote:
> > > > > > This is an incremental step in converting vmstate loading
> > > > > > code to report error via Error objects instead of directly
> > > > > > printing it to console/monitor.
> > > > > > It is ensured that qemu_loadvm_state() must report an error
> > > > > > in errp, in case of failure.
> > > > > >
> > > > > > When postcopy live migration runs, the device states are loaded by
> > > > > > both the qemu coroutine process_incoming_migration_co() and the
> > > > > > postcopy_ram_listen_thread(). Therefore, it is important that the
> > > > > > coroutine also reports the error in case of failure, with
> > > > > > error_report_err(). Otherwise, the source qemu will not display
> > > > > > any errors before going into the postcopy pause state.
> > > > > >
> > > > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > > > > > Signed-off-by: Arun Menon <armenon@redhat.com>
> > > > > > ---
> > > > > > migration/migration.c | 9 +++++----
> > > > > > migration/savevm.c | 30 ++++++++++++++++++------------
> > > > > > migration/savevm.h | 2 +-
> > > > > > 3 files changed, 24 insertions(+), 17 deletions(-)
> > > > > >
> > > > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > > > index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
> > > > > > --- a/migration/migration.c
> > > > > > +++ b/migration/migration.c
> > > > > > @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
> > > > > > MIGRATION_STATUS_ACTIVE);
> > > > > > mis->loadvm_co = qemu_coroutine_self();
> > > > > > - ret = qemu_loadvm_state(mis->from_src_file);
> > > > > > + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
> > > > > > mis->loadvm_co = NULL;
> > > > > > trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
> > > > > > @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
> > > > > > }
> > > > > > if (ret < 0) {
> > > > > > - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
> > > > > > + error_prepend(&local_err, "load of migration failed: %s: ",
> > > > > > + strerror(-ret));
> > > > > > goto fail;
> > > > > > }
> > > > > > @@ -924,13 +925,13 @@ fail:
> > > > > > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> > > > > > MIGRATION_STATUS_FAILED);
> > > > > > migrate_set_error(s, local_err);
> > > > > > - error_free(local_err);
> > > > > > + error_report_err(local_err);
> > > > >
> > > > > This is problematic because it results in duplicate error reports when
> > > > > !mis->exit_on_error; in that case the query-migrate QMP command reports the
> > > > > error and this error reporting is redundant.
> > > >
> > > > If I comment this change, then all of the errors propagated up to now, using
> > > > error_setg() will not be reported. This is the place where it is finally reported,
> > > > when qemu_loadvm_state() fails. In other words, all the error_reports() we removed
> > > > from all the files, replacing them with error_setg(), will finally be reported here
> > > > using error_report_err().
> > >
> > > My understanding of the code without these two changes is:
> > > - If the migrate-incoming QMP command is used with false as
> > > exit-on-error, this function will not report the error but
> > > the query-migrate QMP command will report the error.
> > > - Otherwise, this function reports the error.
> >
> > With my limited experience in testing, I have a question,
> > So there are 2 scenarios,
> > 1. running the virsh migrate command on the source host. Something like the following,
> > virsh -c 'qemu:///system' migrate --live --verbose --domain guest-vm --desturi qemu+ssh://10.6.120.20/system
> > OR for postcopy-ram,
> > virsh migrate guest-vm --live qemu+ssh://10.6.120.20/system --verbose --postcopy --timeout 10 --timeout-postcopy
> >
> > 2. Using QMP commands, performing a migration from source to destination.
> > Running something like the following on the destination:
> > {
> > "execute": "migrate-incoming",
> > "arguments": {
> > "uri": "tcp:127.0.0.1:7777",
> > "exit-on-error": false
> > }
> > }
> > {
> > "execute": "migrate-incoming",
> > "arguments": {
> > "uri": "tcp:127.0.0.1:7777",
> > "exit-on-error": false
> > }
> > }
> > and the somthing like the following on source:
> > {
> > "execute": "migrate",
> > "arguments": {
> > "uri": "tcp:127.0.0.1:7777"
> > }
> > }
> > {"execute" : "query-migrate"}
> >
> > In 1, previously, the user used to get an error message on migration failure.
> > This was because there were error_report() calls in all of the files.
> > Now that they are replaced with error_setg() and the error is stored in errp,
> > we need to display that using error_report_err(). Hence I introduced an error_report_err()
> > call in the fail section.
> >
> > In 2, we have 2 QMP sessions, one for the source and another for the destination.
> > The QMP command migrate will be issued on the source, and the errp will be set.
> > I did not understand the part where the message will be displayed because of the
> > error_report_err() call. I did not see such a message on failure scenario on both
> > the sessions.
> > If the user wants to check for errors, then the destination qemu will not exit
> > (exit-on-error = false ) and we can retrieve it using {"execute" : "query-migrate"}
> >
> > Aren't the 2 scenarios different by nature?
>
> In 1, doesn't libvirt query the error with query-migrate and print it?
Ideally it should find the the error, and print the whole thing. It does work
in the normal scenario. However, the postcopy scenario does not show the same result,
which is mentioned in the commit message.
>
> In any case, it would be nice if you describe how libvirt interacts with
> QEMU in 1.
Please find below the difference in the command output at source, when we run a live migration
with postcopy enabled.
=========
With the current changes:
[root@dell-per750-42 qemu-priv]# virsh migrate-setspeed guest-vm 1
[root@dell-per750-42 build]# virsh migrate guest-vm --live qemu+ssh://10.6.120.9/system --verbose --postcopy --timeout 10 --timeout-postcopy
root@10.6.120.9's password:
Migration: [ 1.26 %]error: internal error: QEMU unexpectedly closed the monitor (vm='guest-vm'): 2025-09-03T06:19:15.076547Z qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested (2) exceeds the recommended cpus supported by KVM (1)
2025-09-03T06:19:15.076586Z qemu-system-x86_64: -accel kvm: warning: Number of hotpluggable cpus requested (2) exceeds the recommended cpus supported by KVM (1)
2025-09-03T06:19:27.776715Z qemu-system-x86_64: load of migration failed: Input/output error: error while loading state for instance 0x0 of device 'tpm-emulator': post load hook failed for: tpm-emulator, version_id: 0, minimum_version: 0, ret: -5: tpm-emulator: Setting the stateblob (type 1) failed with a TPM error 0x21 decryption error
[root@dell-per750-42 build]#
=========
Without the current changes:
[root@dell-per750-42 qemu-priv]# virsh migrate-setspeed guest-vm 1
[root@dell-per750-42 qemu-priv]# virsh migrate guest-vm --live qemu+ssh://10.6.120.9/system --verbose --postcopy --timeout 10 --timeout-postcopy
root@10.6.120.9's password:
Migration: [ 1.28 %]error: internal error: QEMU unexpectedly closed the monitor (vm='guest-vm'): 2025-09-03T06:26:17.733786Z qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested (2) exceeds the recommended cpus supported by KVM (1)
2025-09-03T06:26:17.733830Z qemu-system-x86_64: -accel kvm: warning: Number of hotpluggable cpus requested (2) exceeds the recommended cpus supported by KVM (1)
[root@dell-per750-42 qemu-priv]#
=========
The original behavior was to print the error to the console regardless of whether the migration is normal or postcopy.
The source machine goes in to a paused state after this.
>
> Regards,
> Akihiko Odaki
>
Regards,
Arun Menon
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state()
2025-09-03 6:47 ` Arun Menon
@ 2025-09-06 3:22 ` Akihiko Odaki
0 siblings, 0 replies; 35+ messages in thread
From: Akihiko Odaki @ 2025-09-06 3:22 UTC (permalink / raw)
To: armenon
Cc: qemu-devel, Peter Xu, Fabiano Rosas, Alex Bennée,
Dmitry Osipenko, Michael S. Tsirkin, Marcel Apfelbaum,
Cornelia Huck, Halil Pasic, Eric Farman, Thomas Huth,
Christian Borntraeger, Matthew Rosato, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Nicholas Piggin,
Harsh Prateek Bora, Paolo Bonzini, Fam Zheng, Alex Williamson,
Cédric Le Goater, Steve Sistare, Marc-André Lureau,
qemu-s390x, qemu-ppc, Hailiang Zhang, Stefan Berger,
Peter Maydell, qemu-arm
On 2025/09/03 8:47, Arun Menon wrote:
> Hi Akihiko,
>
> It took some time to set up the machines; apologies for the delay in response.
>
> On Mon, Sep 01, 2025 at 02:12:54AM +0900, Akihiko Odaki wrote:
>> On 2025/09/01 1:38, Arun Menon wrote:
>>> Hi,
>>>
>>> On Mon, Sep 01, 2025 at 01:04:40AM +0900, Akihiko Odaki wrote:
>>>> On 2025/09/01 0:45, Arun Menon wrote:
>>>>> Hi Akihiko,
>>>>> Thanks for the review.
>>>>>
>>>>> On Sat, Aug 30, 2025 at 02:58:05PM +0900, Akihiko Odaki wrote:
>>>>>> On 2025/08/30 5:01, Arun Menon wrote:
>>>>>>> This is an incremental step in converting vmstate loading
>>>>>>> code to report error via Error objects instead of directly
>>>>>>> printing it to console/monitor.
>>>>>>> It is ensured that qemu_loadvm_state() must report an error
>>>>>>> in errp, in case of failure.
>>>>>>>
>>>>>>> When postcopy live migration runs, the device states are loaded by
>>>>>>> both the qemu coroutine process_incoming_migration_co() and the
>>>>>>> postcopy_ram_listen_thread(). Therefore, it is important that the
>>>>>>> coroutine also reports the error in case of failure, with
>>>>>>> error_report_err(). Otherwise, the source qemu will not display
>>>>>>> any errors before going into the postcopy pause state.
>>>>>>>
>>>>>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>>>>>> Signed-off-by: Arun Menon <armenon@redhat.com>
>>>>>>> ---
>>>>>>> migration/migration.c | 9 +++++----
>>>>>>> migration/savevm.c | 30 ++++++++++++++++++------------
>>>>>>> migration/savevm.h | 2 +-
>>>>>>> 3 files changed, 24 insertions(+), 17 deletions(-)
>>>>>>>
>>>>>>> diff --git a/migration/migration.c b/migration/migration.c
>>>>>>> index 10c216d25dec01f206eacad2edd24d21f00e614c..c6768d88f45c870c7fad9b9957300766ff69effc 100644
>>>>>>> --- a/migration/migration.c
>>>>>>> +++ b/migration/migration.c
>>>>>>> @@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
>>>>>>> MIGRATION_STATUS_ACTIVE);
>>>>>>> mis->loadvm_co = qemu_coroutine_self();
>>>>>>> - ret = qemu_loadvm_state(mis->from_src_file);
>>>>>>> + ret = qemu_loadvm_state(mis->from_src_file, &local_err);
>>>>>>> mis->loadvm_co = NULL;
>>>>>>> trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
>>>>>>> @@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
>>>>>>> }
>>>>>>> if (ret < 0) {
>>>>>>> - error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
>>>>>>> + error_prepend(&local_err, "load of migration failed: %s: ",
>>>>>>> + strerror(-ret));
>>>>>>> goto fail;
>>>>>>> }
>>>>>>> @@ -924,13 +925,13 @@ fail:
>>>>>>> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>>>>>>> MIGRATION_STATUS_FAILED);
>>>>>>> migrate_set_error(s, local_err);
>>>>>>> - error_free(local_err);
>>>>>>> + error_report_err(local_err);
>>>>>>
>>>>>> This is problematic because it results in duplicate error reports when
>>>>>> !mis->exit_on_error; in that case the query-migrate QMP command reports the
>>>>>> error and this error reporting is redundant.
>>>>>
>>>>> If I comment this change, then all of the errors propagated up to now, using
>>>>> error_setg() will not be reported. This is the place where it is finally reported,
>>>>> when qemu_loadvm_state() fails. In other words, all the error_reports() we removed
>>>>> from all the files, replacing them with error_setg(), will finally be reported here
>>>>> using error_report_err().
>>>>
>>>> My understanding of the code without these two changes is:
>>>> - If the migrate-incoming QMP command is used with false as
>>>> exit-on-error, this function will not report the error but
>>>> the query-migrate QMP command will report the error.
>>>> - Otherwise, this function reports the error.
>>>
>>> With my limited experience in testing, I have a question,
>>> So there are 2 scenarios,
>>> 1. running the virsh migrate command on the source host. Something like the following,
>>> virsh -c 'qemu:///system' migrate --live --verbose --domain guest-vm --desturi qemu+ssh://10.6.120.20/system
>>> OR for postcopy-ram,
>>> virsh migrate guest-vm --live qemu+ssh://10.6.120.20/system --verbose --postcopy --timeout 10 --timeout-postcopy
>>>
>>> 2. Using QMP commands, performing a migration from source to destination.
>>> Running something like the following on the destination:
>>> {
>>> "execute": "migrate-incoming",
>>> "arguments": {
>>> "uri": "tcp:127.0.0.1:7777",
>>> "exit-on-error": false
>>> }
>>> }
>>> {
>>> "execute": "migrate-incoming",
>>> "arguments": {
>>> "uri": "tcp:127.0.0.1:7777",
>>> "exit-on-error": false
>>> }
>>> }
>>> and the somthing like the following on source:
>>> {
>>> "execute": "migrate",
>>> "arguments": {
>>> "uri": "tcp:127.0.0.1:7777"
>>> }
>>> }
>>> {"execute" : "query-migrate"}
>>>
>>> In 1, previously, the user used to get an error message on migration failure.
>>> This was because there were error_report() calls in all of the files.
>>> Now that they are replaced with error_setg() and the error is stored in errp,
>>> we need to display that using error_report_err(). Hence I introduced an error_report_err()
>>> call in the fail section.
>>>
>>> In 2, we have 2 QMP sessions, one for the source and another for the destination.
>>> The QMP command migrate will be issued on the source, and the errp will be set.
>>> I did not understand the part where the message will be displayed because of the
>>> error_report_err() call. I did not see such a message on failure scenario on both
>>> the sessions.
>>> If the user wants to check for errors, then the destination qemu will not exit
>>> (exit-on-error = false ) and we can retrieve it using {"execute" : "query-migrate"}
>>>
>>> Aren't the 2 scenarios different by nature?
>>
>> In 1, doesn't libvirt query the error with query-migrate and print it?
>
> Ideally it should find the the error, and print the whole thing. It does work
> in the normal scenario. However, the postcopy scenario does not show the same result,
> which is mentioned in the commit message.
>
>>
>> In any case, it would be nice if you describe how libvirt interacts with
>> QEMU in 1.
>
> Please find below the difference in the command output at source, when we run a live migration
> with postcopy enabled.
>
> =========
> With the current changes:
> [root@dell-per750-42 qemu-priv]# virsh migrate-setspeed guest-vm 1
>
> [root@dell-per750-42 build]# virsh migrate guest-vm --live qemu+ssh://10.6.120.9/system --verbose --postcopy --timeout 10 --timeout-postcopy
> root@10.6.120.9's password:
> Migration: [ 1.26 %]error: internal error: QEMU unexpectedly closed the monitor (vm='guest-vm'): 2025-09-03T06:19:15.076547Z qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested (2) exceeds the recommended cpus supported by KVM (1)
> 2025-09-03T06:19:15.076586Z qemu-system-x86_64: -accel kvm: warning: Number of hotpluggable cpus requested (2) exceeds the recommended cpus supported by KVM (1)
> 2025-09-03T06:19:27.776715Z qemu-system-x86_64: load of migration failed: Input/output error: error while loading state for instance 0x0 of device 'tpm-emulator': post load hook failed for: tpm-emulator, version_id: 0, minimum_version: 0, ret: -5: tpm-emulator: Setting the stateblob (type 1) failed with a TPM error 0x21 decryption error
>
> [root@dell-per750-42 build]#
>
> =========
>
> Without the current changes:
> [root@dell-per750-42 qemu-priv]# virsh migrate-setspeed guest-vm 1
>
> [root@dell-per750-42 qemu-priv]# virsh migrate guest-vm --live qemu+ssh://10.6.120.9/system --verbose --postcopy --timeout 10 --timeout-postcopy
> root@10.6.120.9's password:
> Migration: [ 1.28 %]error: internal error: QEMU unexpectedly closed the monitor (vm='guest-vm'): 2025-09-03T06:26:17.733786Z qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested (2) exceeds the recommended cpus supported by KVM (1)
> 2025-09-03T06:26:17.733830Z qemu-system-x86_64: -accel kvm: warning: Number of hotpluggable cpus requested (2) exceeds the recommended cpus supported by KVM (1)
>
> [root@dell-per750-42 qemu-priv]#
>
> =========
> The original behavior was to print the error to the console regardless of whether the migration is normal or postcopy.
This was true for messages in qemu_loadvm_state(), but the message "load
of migration failed" was printed or queried with query-migrate, not
both. We should think of which behavior is more appropriate, and I think
we should avoid duplicate reports.
> The source machine goes in to a paused state after this.
The output is informative. It implies the destination machine exited,
and it makes sense to print error messages as it is done for
mis->exit_on_error. I wonder if it is possible to detect the condition
and treat it identically to mis->exit_on_error.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2025-09-06 3:24 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 20:01 [PATCH v13 00/27] migration: propagate vTPM errors using Error objects Arun Menon
2025-08-29 20:01 ` [PATCH v13 01/27] migration: push Error **errp into vmstate_subsection_load() Arun Menon
2025-08-29 20:01 ` [PATCH v13 02/27] migration: push Error **errp into vmstate_load_state() Arun Menon
2025-08-29 20:01 ` [PATCH v13 03/27] migration: push Error **errp into qemu_loadvm_state_header() Arun Menon
2025-08-29 20:01 ` [PATCH v13 04/27] migration: push Error **errp into vmstate_load() Arun Menon
2025-08-29 20:01 ` [PATCH v13 05/27] migration: push Error **errp into loadvm_process_command() Arun Menon
2025-08-29 20:01 ` [PATCH v13 06/27] migration: push Error **errp into loadvm_handle_cmd_packaged() Arun Menon
2025-08-29 20:01 ` [PATCH v13 07/27] migration: push Error **errp into qemu_loadvm_state() Arun Menon
2025-08-30 5:58 ` Akihiko Odaki
2025-08-31 15:45 ` Arun Menon
2025-08-31 16:04 ` Akihiko Odaki
2025-08-31 16:38 ` Arun Menon
2025-08-31 17:12 ` Akihiko Odaki
2025-09-03 6:47 ` Arun Menon
2025-09-06 3:22 ` Akihiko Odaki
2025-08-29 20:01 ` [PATCH v13 08/27] migration: push Error **errp into qemu_load_device_state() Arun Menon
2025-08-29 20:01 ` [PATCH v13 09/27] migration: push Error **errp into qemu_loadvm_state_main() Arun Menon
2025-08-29 20:01 ` [PATCH v13 10/27] migration: push Error **errp into qemu_loadvm_section_start_full() Arun Menon
2025-08-29 20:01 ` [PATCH v13 11/27] migration: push Error **errp into qemu_loadvm_section_part_end() Arun Menon
2025-08-29 20:01 ` [PATCH v13 12/27] migration: Update qemu_file_get_return_path() docs and remove dead checks Arun Menon
2025-08-29 20:01 ` [PATCH v13 13/27] migration: make loadvm_postcopy_handle_resume() void Arun Menon
2025-08-29 20:01 ` [PATCH v13 14/27] migration: push Error **errp into ram_postcopy_incoming_init() Arun Menon
2025-08-29 20:01 ` [PATCH v13 15/27] migration: push Error **errp into loadvm_postcopy_handle_advise() Arun Menon
2025-08-29 20:01 ` [PATCH v13 16/27] migration: push Error **errp into loadvm_postcopy_handle_listen() Arun Menon
2025-08-29 20:01 ` [PATCH v13 17/27] migration: push Error **errp into loadvm_postcopy_handle_run() Arun Menon
2025-08-29 20:01 ` [PATCH v13 18/27] migration: push Error **errp into loadvm_postcopy_ram_handle_discard() Arun Menon
2025-08-29 20:01 ` [PATCH v13 19/27] migration: push Error **errp into loadvm_handle_recv_bitmap() Arun Menon
2025-08-29 20:02 ` [PATCH v13 20/27] migration: Return -1 on memory allocation failure in ram.c Arun Menon
2025-08-29 20:02 ` [PATCH v13 21/27] migration: push Error **errp into loadvm_process_enable_colo() Arun Menon
2025-08-29 20:02 ` [PATCH v13 22/27] migration: push Error **errp into loadvm_postcopy_handle_switchover_start() Arun Menon
2025-08-29 20:02 ` [PATCH v13 23/27] migration: Capture error in postcopy_ram_listen_thread() Arun Menon
2025-08-29 20:02 ` [PATCH v13 24/27] migration: Remove error variant of vmstate_save_state() function Arun Menon
2025-08-29 20:02 ` [PATCH v13 25/27] migration: Rename post_save() to cleanup_save() and make it void Arun Menon
2025-08-29 20:02 ` [PATCH v13 26/27] migration: Add error-parameterized function variants in VMSD struct Arun Menon
2025-08-29 20:02 ` [PATCH v13 27/27] backends/tpm: Propagate vTPM error on migration failure Arun Menon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).