On Tue, Oct 28, 2025 at 03:42:54PM +0900, Akihiko Odaki wrote: > On 2025/10/28 15:21, Arun Menon wrote: > > error_fatal is passed to vmstate_load_state() and vmstate_save_state() > > functions. This was introduced in commit c632ffbd74. This would exit(1) > > on error, and therefore does not allow to propagate the error back to > > the caller. > > > > To maintain consistency with prior error handling i.e. either propagating > > the error to the caller or reporting it, we must set the error within a > > local Error object instead of using error_fatal. > > > > Reviewed-by: Cornelia Huck <[email protected]> > > Signed-off-by: Arun Menon <[email protected]> > > --- > > hw/display/virtio-gpu.c | 19 ++++++++++++++----- > > hw/pci/pci.c | 13 +++++++++++-- > > hw/s390x/virtio-ccw.c | 15 +++++++++++++-- > > hw/scsi/spapr_vscsi.c | 9 +++++++-- > > hw/virtio/virtio-mmio.c | 15 +++++++++++++-- > > hw/virtio/virtio-pci.c | 15 +++++++++++++-- > > hw/virtio/virtio.c | 10 +++++++--- > > 7 files changed, 78 insertions(+), 18 deletions(-) > > > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > > index > > 3a555125be60aa4c243cfb870caa517995de8183..63263ecc5bda889e5327aa59ada53cb41b0219cb > > 100644 > > --- a/hw/display/virtio-gpu.c > > +++ b/hw/display/virtio-gpu.c > > @@ -1225,7 +1225,9 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, > > size_t size, > > { > > VirtIOGPU *g = opaque; > > struct virtio_gpu_simple_resource *res; > > + Error *err = NULL; > > int i; > > + int ret = 0; > > I prefer not to have a variable initialized unless necessary because it > suppresses compiler warnings to tell accidental usage of variable. > > Besides, the code change for other files in this patch and the existing code > in this file doesn't initialize such variables, making the code style > inconsistent. > > That said, I feel it is too nitpicky so I give: > > Reviewed-by: Akihiko Odaki <[email protected]> > > You may or may not resend this patch to drop the initialization.
I'll fix it when queuing, thanks both. While at it, I also touched a few other things on reordering of vars (follow rev-christmas tree), spacing, etc. The fixup is attached at the end. Please shoot if anyone disagrees. I'll skip patch 2 because we already have a fix here: https://lore.kernel.org/r/[email protected] Thanks, ===8<=== >From 3cd8f712b5d18b3729dc78127598ebbb2b1afae2 Mon Sep 17 00:00:00 2001 From: Peter Xu <[email protected]> Date: Tue, 28 Oct 2025 10:23:35 -0400 Subject: [PATCH] fixup! migration: Fix regression of passing error_fatal into vmstate_load_state() Signed-off-by: Peter Xu <[email protected]> --- hw/display/virtio-gpu.c | 6 ++---- hw/pci/pci.c | 5 +++-- hw/s390x/virtio-ccw.c | 6 ++++-- hw/scsi/spapr_vscsi.c | 3 ++- hw/virtio/virtio-mmio.c | 2 +- hw/virtio/virtio-pci.c | 4 ++-- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 63263ecc5b..43e88a4daf 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1226,8 +1226,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size, VirtIOGPU *g = opaque; struct virtio_gpu_simple_resource *res; Error *err = NULL; - int i; - int ret = 0; + int i, ret; /* in 2d mode we should never find unprocessed commands here */ assert(QTAILQ_EMPTY(&g->cmdq)); @@ -1294,8 +1293,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, Error *err = NULL; struct virtio_gpu_simple_resource *res; uint32_t resource_id, pformat; - int i; - int ret = 0; + int i, ret; g->hostmem = 0; diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 0090c72560..ac16c9521d 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -921,12 +921,13 @@ const VMStateDescription vmstate_pci_device = { void pci_device_save(PCIDevice *s, QEMUFile *f) { + Error *local_err = NULL; + int ret; + /* Clear interrupt status bit: it is implicit * in irq_state which we are saving. * This makes us compatible with old devices * which never set or clear this bit. */ - int ret; - Error *local_err = NULL; s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT; ret = vmstate_save_state(f, &vmstate_pci_device, s, NULL, &local_err); if (ret < 0) { diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 41c7d62a48..4a3ffb84f8 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -1130,8 +1130,9 @@ 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); - int ret; Error *local_err = NULL; + int ret; + ret = vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL, &local_err); if (ret < 0) { error_report_err(local_err); @@ -1141,8 +1142,9 @@ 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); - int ret; Error *local_err = NULL; + int ret; + ret = vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1, &local_err); if (ret < 0) { error_report_err(local_err); diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index af4debc2f8..a6591319db 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -628,8 +628,9 @@ static const VMStateDescription vmstate_spapr_vscsi_req = { static void vscsi_save_request(QEMUFile *f, SCSIRequest *sreq) { vscsi_req *req = sreq->hba_private; - int rc; Error *local_err = NULL; + int rc; + assert(req->active); rc = vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL, &local_err); diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index ffdda63e27..c05c00bcd4 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -624,8 +624,8 @@ static void virtio_mmio_save_extra_state(DeviceState *opaque, QEMUFile *f) static int virtio_mmio_load_extra_state(DeviceState *opaque, QEMUFile *f) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); - int ret; Error *local_err = NULL; + int ret; ret = vmstate_load_state(f, &vmstate_virtio_mmio, proxy, 1, &local_err); if (ret < 0) { diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index f245f5c3c5..99cb30fe59 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -187,8 +187,8 @@ static bool virtio_pci_has_extra_state(DeviceState *d) static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); - int ret; Error *local_err = NULL; + int ret; ret = vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL, &local_err); if (ret < 0) { @@ -199,8 +199,8 @@ static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f) static int virtio_pci_load_extra_state(DeviceState *d, QEMUFile *f) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); - int ret; Error *local_err = NULL; + int ret; ret = vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1, &local_err); if (ret < 0) { -- 2.50.1 -- Peter Xu
