Re: [PATCH] migration: Fix regression of passing error_fatal into vmstate_load_state()

[Cornelia Huck <cohuck@redhat.com>]

On Fri, Oct 24 2025, Arun Menon <armenon@redhat.com> 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 <armenon@redhat.com>
> ---
>  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 <cohuck@redhat.com>