Re: [PATCH 09/14] vfio: Add Error** argument to .vfio_save_config() handler

2024-02-12 Thread Avihai Horon 



On 07/02/2024 15:33, 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.

Signed-off-by: Cédric Le Goater 
---
  include/hw/vfio/vfio-common.h |  2 +-
  hw/vfio/migration.c   | 18 --
  hw/vfio/pci.c |  5 +++--
  3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 
9b7ef7d02b5a0ad5266bcc4d06cd6874178978e4..710e0d6a880b97848af6ddc2e7968a01054fa122
 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -133,7 +133,7 @@ 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);
+int (*vfio_save_config)(VFIODevice *vdev, QEMUFile *f, Error **errp);
  int (*vfio_load_config)(VFIODevice *vdev, QEMUFile *f);
  };

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 
2e0a79967cc97f44d9be5575c3cfe18c9f349dab..fb264c1ef57bbbde4306901e5449e0dfbd0ce3b7
 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -190,14 +190,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 = 0;


Nit: 0 assignment is redundant.

Thanks.



  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);
@@ -579,13 +584,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, _err);
  if (ret) {
-error_report("%s: Failed to save device config space",
- vbasedev->name);
-qemu_file_set_error(f, ret);
+error_prepend(_err, "%s: Failed to save device config space",
+  vbasedev->name);
+qemu_file_set_error_obj(f, ret, local_err);
  }
  }

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 
4fa387f0430d62ca2ba1b5ae5b7037f8f06b33f9..99d86e1d40ef25133fc76ad6e58294b07bd20843
 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2585,11 +2585,12 @@ 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, _vfio_pci_config, vdev, NULL);
+return vmstate_save_state_with_err(f, _vfio_pci_config, vdev, NULL,
+   errp);
  }

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

Re: [PATCH 09/14] vfio: Add Error** argument to .vfio_save_config() handler

2024-02-07 Thread Philippe Mathieu-Daudé 

On 7/2/24 14:33, Cédric Le Goater wrote:

Use vmstate_save_state_with_err() to improve error reporting in the
callers.

Signed-off-by: Cédric Le Goater 
---
  include/hw/vfio/vfio-common.h |  2 +-
  hw/vfio/migration.c   | 18 --
  hw/vfio/pci.c |  5 +++--
  3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 
9b7ef7d02b5a0ad5266bcc4d06cd6874178978e4..710e0d6a880b97848af6ddc2e7968a01054fa122
 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -133,7 +133,7 @@ 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);
+int (*vfio_save_config)(VFIODevice *vdev, QEMUFile *f, Error **errp);


Worth a one-line docstring?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé 


  int (*vfio_load_config)(VFIODevice *vdev, QEMUFile *f);
  };

[PATCH 09/14] vfio: Add Error** argument to .vfio_save_config() handler

2024-02-07 Thread Cédric Le Goater 
Use vmstate_save_state_with_err() to improve error reporting in the
callers.

Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-common.h |  2 +-
 hw/vfio/migration.c   | 18 --
 hw/vfio/pci.c |  5 +++--
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 
9b7ef7d02b5a0ad5266bcc4d06cd6874178978e4..710e0d6a880b97848af6ddc2e7968a01054fa122
 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -133,7 +133,7 @@ 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);
+int (*vfio_save_config)(VFIODevice *vdev, QEMUFile *f, Error **errp);
 int (*vfio_load_config)(VFIODevice *vdev, QEMUFile *f);
 };
 
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 
2e0a79967cc97f44d9be5575c3cfe18c9f349dab..fb264c1ef57bbbde4306901e5449e0dfbd0ce3b7
 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -190,14 +190,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 = 0;
 
 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);
@@ -579,13 +584,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, _err);
 if (ret) {
-error_report("%s: Failed to save device config space",
- vbasedev->name);
-qemu_file_set_error(f, ret);
+error_prepend(_err, "%s: Failed to save device config space",
+  vbasedev->name);
+qemu_file_set_error_obj(f, ret, local_err);
 }
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 
4fa387f0430d62ca2ba1b5ae5b7037f8f06b33f9..99d86e1d40ef25133fc76ad6e58294b07bd20843
 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2585,11 +2585,12 @@ 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, _vfio_pci_config, vdev, NULL);
+return vmstate_save_state_with_err(f, _vfio_pci_config, vdev, NULL,
+   errp);
 }
 
 static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
-- 
2.43.0