Re: [PATCH v5 02/10] vfio: Add Error** argument to vfio_devices_dma_logging_start()

[Avihai Horon]

On 06/05/2024 12:20, Cédric Le Goater wrote:

External email: Use caution opening links or attachments


This allows to update the Error argument of the VFIO log_global_start()
handler. Errors detected when device level logging is started will be
propagated up to qemu_savevm_state_setup() when the ram save_setup()
handler is executed.

Errors for container based logging will also be propagated now.

The vfio_set_migration_error() call becomes redundant in
vfio_devices_dma_logging_start(). Remove it.

Becomes redundant in vfio_listener_log_global_start()?

Other than that,
Reviewed-by: Avihai Horon <avih...@nvidia.com>

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Signed-off-by: Cédric Le Goater <c...@redhat.com>
---

  Changes in v5:

  - Used error_setg_errno() in vfio_devices_dma_logging_start()

  hw/vfio/common.c | 26 +++++++++++++++-----------
  1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
485e53916491f1164d29e739fb7106c0c77df737..b5102f54a6474a50c6366e8fbce23812d55e384e
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1027,7 +1027,8 @@ static void vfio_device_feature_dma_logging_start_destroy(
      g_free(feature);
  }

-static int vfio_devices_dma_logging_start(VFIOContainerBase *bcontainer)
+static int vfio_devices_dma_logging_start(VFIOContainerBase *bcontainer,
+                                          Error **errp)
  {
      struct vfio_device_feature *feature;
      VFIODirtyRanges ranges;
@@ -1038,6 +1039,7 @@ static int 
vfio_devices_dma_logging_start(VFIOContainerBase *bcontainer)
      feature = vfio_device_feature_dma_logging_start_create(bcontainer,
                                                             &ranges);
      if (!feature) {
+        error_setg_errno(errp, errno, "Failed to prepare DMA logging");
          return -errno;
      }

@@ -1049,8 +1051,8 @@ static int 
vfio_devices_dma_logging_start(VFIOContainerBase *bcontainer)
          ret = ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature);
          if (ret) {
              ret = -errno;
-            error_report("%s: Failed to start DMA logging, err %d (%s)",
-                         vbasedev->name, ret, strerror(errno));
+            error_setg_errno(errp, errno, "%s: Failed to start DMA logging",
+                             vbasedev->name);
              goto out;
          }
          vbasedev->dirty_tracking = true;
@@ -1069,20 +1071,19 @@ out:
  static bool vfio_listener_log_global_start(MemoryListener *listener,
                                             Error **errp)
  {
+    ERRP_GUARD();
      VFIOContainerBase *bcontainer = container_of(listener, VFIOContainerBase,
                                                   listener);
      int ret;

      if (vfio_devices_all_device_dirty_tracking(bcontainer)) {
-        ret = vfio_devices_dma_logging_start(bcontainer);
+        ret = vfio_devices_dma_logging_start(bcontainer, errp);
      } else {
-        ret = vfio_container_set_dirty_page_tracking(bcontainer, true, NULL);
+        ret = vfio_container_set_dirty_page_tracking(bcontainer, true, errp);
      }

      if (ret) {
-        error_report("vfio: Could not start dirty page tracking, err: %d (%s)",
-                     ret, strerror(-ret));
-        vfio_set_migration_error(ret);
+        error_prepend(errp, "vfio: Could not start dirty page tracking - ");
      }
      return !ret;
  }
@@ -1091,17 +1092,20 @@ static void 
vfio_listener_log_global_stop(MemoryListener *listener)
  {
      VFIOContainerBase *bcontainer = container_of(listener, VFIOContainerBase,
                                                   listener);
+    Error *local_err = NULL;
      int ret = 0;

      if (vfio_devices_all_device_dirty_tracking(bcontainer)) {
          vfio_devices_dma_logging_stop(bcontainer);
      } else {
-        ret = vfio_container_set_dirty_page_tracking(bcontainer, false, NULL);
+        ret = vfio_container_set_dirty_page_tracking(bcontainer, false,
+                                                     &local_err);
      }

      if (ret) {
-        error_report("vfio: Could not stop dirty page tracking, err: %d (%s)",
-                     ret, strerror(-ret));
+        error_prepend(&local_err,
+                      "vfio: Could not stop dirty page tracking - ");
+        error_report_err(local_err);
          vfio_set_migration_error(ret);
      }
  }
--
2.45.0