Hi Cornelia, On Mon, Oct 27, 2025 at 12:18:58PM +0100, Cornelia Huck wrote: > On Fri, Oct 24 2025, Arun Menon <[email protected]> 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. > > > > Signed-off-by: Arun Menon <[email protected]> > > --- > > hw/display/virtio-gpu.c | 20 ++++++++++++++------ > > 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(+), 19 deletions(-) > > > > (...) > > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > > index > > 4cb1ced001ae241c53c503ebfd7c90e336799c37..41c7d62a482de3c618e71dd07c0cd23e1bcd5578 > > 100644 > > --- a/hw/s390x/virtio-ccw.c > > +++ b/hw/s390x/virtio-ccw.c > > @@ -1130,13 +1130,24 @@ 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, > > &error_fatal); > > + int ret; > > + Error *local_err = NULL; > > + ret = vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL, > > &local_err); > > + if (ret < 0) { > > + error_report_err(local_err); > > + } > > } > > > > 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, > > &error_fatal); > > + int ret; > > + Error *local_err = NULL; > > + ret = vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1, > > &local_err); > > + if (ret < 0) { > > + error_report_err(local_err); > > + } > > + return ret; > > } > > > > static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) > > I was wondering whether it would make sense to pass the error object in > VirtioBusClass's ->{load,save}_config instead, but it seems virtio-ccw > is the only one using that pattern there (virtio-mmio and virtio-pci are > doing that dance in their ->{load,save}_extra_state callbacks); so let's > just do it this way and think about more error object passing later. > > Reviewed-by: Cornelia Huck <[email protected]>
Thank you for the review. > Regards, Arun
