On 5/13/24 15:30, Avihai Horon wrote:

On 06/05/2024 12:20, Cédric Le Goater wrote:
External email: Use caution opening links or attachments


Use vmstate_save_state_with_err() to improve error reporting in the
callers and store a reported error under the migration stream. Add
documentation while at it.

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Signed-off-by: Cédric Le Goater <c...@redhat.com>
---
  include/hw/vfio/vfio-common.h | 25 ++++++++++++++++++++++++-
  hw/vfio/migration.c           | 18 ++++++++++++------
  hw/vfio/pci.c                 |  5 +++--
  3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 
b9da6c08ef41174610eb92726c590309a53696a3..46f88493634b5634a9c14a5caa33a463fbf2c50d
 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -133,7 +133,30 @@ struct VFIODeviceOps {
      int (*vfio_hot_reset_multi)(VFIODevice *vdev);
      void (*vfio_eoi)(VFIODevice *vdev);
      Object *(*vfio_get_object)(VFIODevice *vdev);
-    void (*vfio_save_config)(VFIODevice *vdev, QEMUFile *f);
+
+    /**
+     * @vfio_save_config
+     *
+     * Save device config state
+     *
+     * @vdev: #VFIODevice for which to save the config
+     * @f: #QEMUFile where to send the data
+     * @errp: pointer to Error*, to store an error if it happens.
+     *
+     * Returns zero to indicate success and negative for error
+     */
+    int (*vfio_save_config)(VFIODevice *vdev, QEMUFile *f, Error **errp);
+
+    /**
+     * @vfio_load_config
+     *
+     * Load device config state
+     *
+     * @vdev: #VFIODevice for which to load the config
+     * @f: #QEMUFile where to get the data
+     *
+     * Returns zero to indicate success and negative for error
+     */
      int (*vfio_load_config)(VFIODevice *vdev, QEMUFile *f);
  };

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 
9b6375c949f7a8dca857ead2506855f63fa051e4..87437490bd50321b3eb27770c932078597053746
 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -189,14 +189,19 @@ static int vfio_load_buffer(QEMUFile *f, VFIODevice 
*vbasedev,
      return ret;
  }

-static int vfio_save_device_config_state(QEMUFile *f, void *opaque)
+static int vfio_save_device_config_state(QEMUFile *f, void *opaque,
+                                         Error **errp)
  {
      VFIODevice *vbasedev = opaque;
+    int ret;

      qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE);

      if (vbasedev->ops && vbasedev->ops->vfio_save_config) {
-        vbasedev->ops->vfio_save_config(vbasedev, f);
+        ret = vbasedev->ops->vfio_save_config(vbasedev, f, errp);
+        if (ret) {
+            return ret;
+        }
      }

      qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);

Below we have:

return qemu_file_get_error(f);

Need to set errp in case of error.

yep. I will change the returned type to bool while at it.


@@ -588,13 +593,14 @@ static int vfio_save_complete_precopy(QEMUFile *f, void 
*opaque)
  static void vfio_save_state(QEMUFile *f, void *opaque)
  {
      VFIODevice *vbasedev = opaque;
+    Error *local_err = NULL;
      int ret;

-    ret = vfio_save_device_config_state(f, opaque);
+    ret = vfio_save_device_config_state(f, opaque, &local_err);
      if (ret) {
-        error_report("%s: Failed to save device config space",
-                     vbasedev->name);
-        qemu_file_set_error(f, ret);
+        error_prepend(&local_err, "%s: Failed to save device config space",

Add " - " ("... device config space - "), like in the other patches?

ok. I will check how (in)consistent I was when doing these
error_prepend changes.


Thanks,

C.



Thanks.

+                      vbasedev->name);
+        qemu_file_set_error_obj(f, ret, local_err);
      }
  }

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 
64780d1b793345c8e8996fe6b7987059ce831c11..fc6e54e871508bb0e2a3ac9079a195c086531f21
 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2586,11 +2586,12 @@ static const VMStateDescription vmstate_vfio_pci_config 
= {
      }
  };

-static void vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f)
+static int vfio_pci_save_config(VFIODevice *vbasedev, QEMUFile *f, Error 
**errp)
  {
      VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);

-    vmstate_save_state(f, &vmstate_vfio_pci_config, vdev, NULL);
+    return vmstate_save_state_with_err(f, &vmstate_vfio_pci_config, vdev, NULL,
+                                       errp);
  }

  static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
--
2.45.0




Reply via email to