Vladimir Sementsov-Ogievskiy <[email protected]> writes:
> Stop ignoring the error on save path.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
> hw/pci/pci.c | 27 +++++++++++----------------
> hw/s390x/virtio-ccw.c | 19 ++++---------------
> hw/virtio/virtio-mmio.c | 10 +++++++---
> hw/virtio/virtio-pci.c | 24 +++++++++++++++---------
> hw/virtio/virtio.c | 11 +++++++----
> include/hw/pci/pci.h | 4 ++--
> include/hw/virtio/virtio-bus.h | 4 ++--
> 7 files changed, 48 insertions(+), 51 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d3191609e28..76d006fa1d7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -940,37 +940,32 @@ const VMStateDescription vmstate_pci_device = {
> };
>
>
> -void pci_device_save(PCIDevice *s, QEMUFile *f)
> +bool pci_device_save(PCIDevice *s, QEMUFile *f, Error **errp)
> {
> - Error *local_err = NULL;
> - int ret;
> + bool ok;
>
> /* 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. */
> s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> - ret = vmstate_save_state(f, &vmstate_pci_device, s, NULL, &local_err);
> - if (ret < 0) {
> - error_report_err(local_err);
> - }
> +
> + ok = vmstate_save_vmsd(f, &vmstate_pci_device, s, NULL, errp);
> +
> /* Restore the interrupt status bit. */
> pci_update_irq_status(s);
> +
> + return ok;
> }
>
> -int pci_device_load(PCIDevice *s, QEMUFile *f)
> +bool pci_device_load(PCIDevice *s, QEMUFile *f, Error **errp)
> {
> - Error *local_err = NULL;
> - int ret;
> + bool ok = vmstate_load_vmsd(f, &vmstate_pci_device, s, s->version_id,
> errp);
>
> - ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id,
> - &local_err);
> - if (ret < 0) {
> - error_report_err(local_err);
> - }
> /* Restore the interrupt status bit. */
> pci_update_irq_status(s);
> - return ret;
> +
> + return ok;
> }
>
> static void pci_set_default_subsystem_id(PCIDevice *pci_dev)
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index d82874ed27e..848ed09e94b 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -1129,29 +1129,18 @@ static int virtio_ccw_load_queue(DeviceState *d, int
> n, QEMUFile *f)
> return 0;
> }
>
> -static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f)
> +static bool virtio_ccw_save_config(DeviceState *d, QEMUFile *f, Error **errp)
> {
> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> - 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);
> - }
> + return vmstate_save_vmsd(f, &vmstate_virtio_ccw_dev, dev, NULL, errp);
> }
>
> -static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
> +static bool virtio_ccw_load_config(DeviceState *d, QEMUFile *f, Error **errp)
> {
> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> - 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);
> - }
> - return ret;
> + return vmstate_load_vmsd(f, &vmstate_virtio_ccw_dev, dev, 1, errp);
> }
>
> static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp)
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 559363e4438..ef34b76913e 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -547,23 +547,27 @@ static void virtio_mmio_update_irq(DeviceState *opaque,
> uint16_t vector)
> qemu_set_irq(proxy->irq, level);
> }
>
> -static int virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f)
> +static bool virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f,
> + Error **errp)
> {
> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
>
> proxy->host_features_sel = qemu_get_be32(f);
> proxy->guest_features_sel = qemu_get_be32(f);
> proxy->guest_page_shift = qemu_get_be32(f);
> - return 0;
> + return true;
> }
>
> -static void virtio_mmio_save_config(DeviceState *opaque, QEMUFile *f)
> +static bool virtio_mmio_save_config(DeviceState *opaque, QEMUFile *f,
> + Error **errp)
> {
> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
>
> qemu_put_be32(f, proxy->host_features_sel);
> qemu_put_be32(f, proxy->guest_features_sel);
> qemu_put_be32(f, proxy->guest_page_shift);
> +
> + return true;
> }
>
> static const VMStateDescription vmstate_virtio_mmio_queue_state = {
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 7a5d4f35e33..07941319be2 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -84,15 +84,21 @@ static void virtio_pci_notify(DeviceState *d, uint16_t
> vector)
> }
> }
>
> -static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
> +static bool virtio_pci_save_config(DeviceState *d, QEMUFile *f, Error **errp)
> {
> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>
> - pci_device_save(&proxy->pci_dev, f);
> + if (!pci_device_save(&proxy->pci_dev, f, errp)) {
> + return false;
> + }
> +
> msix_save(&proxy->pci_dev, f);
> - if (msix_present(&proxy->pci_dev))
> + if (msix_present(&proxy->pci_dev)) {
> qemu_put_be16(f, vdev->config_vector);
> + }
> +
> + return true;
> }
>
> static const VMStateDescription vmstate_virtio_pci_modern_queue_state = {
> @@ -209,24 +215,24 @@ static void virtio_pci_save_queue(DeviceState *d, int
> n, QEMUFile *f)
> qemu_put_be16(f, virtio_queue_vector(vdev, n));
> }
>
> -static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
> +static bool virtio_pci_load_config(DeviceState *d, QEMUFile *f, Error **errp)
> {
This function is returning 0
> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> uint16_t vector;
>
> - int ret;
> - ret = pci_device_load(&proxy->pci_dev, f);
> - if (ret) {
> - return ret;
> + if (!pci_device_load(&proxy->pci_dev, f, errp)) {
> + return false;
> }
> +
> msix_unuse_all_vectors(&proxy->pci_dev);
> msix_load(&proxy->pci_dev, f);
> if (msix_present(&proxy->pci_dev)) {
> qemu_get_be16s(f, &vector);
>
> if (vector != VIRTIO_NO_VECTOR && vector >= proxy->nvectors) {
> - return -EINVAL;
> + error_setg(errp, "load config: unexpected vector %" PRIu16,
> vector);
> + return false;
> }
> } else {
> vector = VIRTIO_NO_VECTOR;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 8230a383aff..0f7c8acead0 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3077,7 +3077,10 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
> Error *local_err = NULL;
>
> if (k->save_config) {
> - k->save_config(qbus->parent, f);
> + if (!k->save_config(qbus->parent, f, &local_err)) {
> + error_report_err(local_err);
> + return -EINVAL;
> + }
> }
>
> qemu_put_8s(f, &vdev->status);
> @@ -3526,9 +3529,9 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN;
>
> if (k->load_config) {
> - ret = k->load_config(qbus->parent, f);
> - if (ret)
> - return ret;
> + if (!k->load_config(qbus->parent, f, &local_err)) {
> + return -EINVAL;
> + }
> }
>
> qemu_get_8s(f, &vdev->status);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index f2448e941a0..7c3fdc05630 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -270,8 +270,8 @@ uint32_t pci_default_read_config(PCIDevice *d,
> uint32_t address, int len);
> void pci_default_write_config(PCIDevice *d,
> uint32_t address, uint32_t val, int len);
> -void pci_device_save(PCIDevice *s, QEMUFile *f);
> -int pci_device_load(PCIDevice *s, QEMUFile *f);
> +bool pci_device_save(PCIDevice *s, QEMUFile *f, Error **errp);
> +bool pci_device_load(PCIDevice *s, QEMUFile *f, Error **errp);
> MemoryRegion *pci_address_space(PCIDevice *dev);
> MemoryRegion *pci_address_space_io(PCIDevice *dev);
>
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 255ecb2fcb2..1b9867002b8 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -41,10 +41,10 @@ struct VirtioBusClass {
> /* This is what a VirtioBus must implement */
> BusClass parent;
> void (*notify)(DeviceState *d, uint16_t vector);
> - void (*save_config)(DeviceState *d, QEMUFile *f);
> + bool (*save_config)(DeviceState *d, QEMUFile *f, Error **errp);
> void (*save_queue)(DeviceState *d, int n, QEMUFile *f);
> bool (*save_extra_state)(DeviceState *d, QEMUFile *f, Error **errp);
> - int (*load_config)(DeviceState *d, QEMUFile *f);
> + bool (*load_config)(DeviceState *d, QEMUFile *f, Error **errp);
> int (*load_queue)(DeviceState *d, int n, QEMUFile *f);
> int (*load_done)(DeviceState *d, QEMUFile *f);
> bool (*load_extra_state)(DeviceState *d, QEMUFile *f, Error **errp);
