From: Cédric Le Goater <clg@redhat.com>
Date: Mon, 24 Mar 2025 12:33:15 +0000 (+0100)
Subject: vfio: Open code vfio_migration_set_error()
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=10c7f1cf2c9d5af28dbc342634eb1f3979a7c379;p=thirdparty%2Fqemu.git

vfio: Open code vfio_migration_set_error()

VFIO uses migration_file_set_error() in a couple of places where an
'Error **' parameter is not provided. In MemoryListener handlers :

  vfio_listener_region_add
  vfio_listener_log_global_stop
  vfio_listener_log_sync

and in callback routines for IOMMU notifiers :

  vfio_iommu_map_notify
  vfio_iommu_map_dirty_notify

Hopefully, one day, we will be able to extend these callbacks with an
'Error **' parameter and avoid setting the global migration error.
Until then, it seems sensible to clearly identify the use cases, which
are limited, and open code vfio_migration_set_error(). One other
benefit is an improved error reporting when migration is running.

While at it, slightly modify error reporting to only report errors
when migration is not active and not always as is currently done.

Cc: Prasad Pandit <pjp@fedoraproject.org>
Reviewed-by: Avihai Horon <avihaih@nvidia.com>
Link: https://lore.kernel.org/qemu-devel/20250324123315.637827-1-clg@redhat.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 9b493451c5..5ef14931cd 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -145,13 +145,6 @@ bool vfio_viommu_preset(VFIODevice *vbasedev)
     return vbasedev->bcontainer->space->as != &address_space_memory;
 }
 
-static void vfio_set_migration_error(int ret)
-{
-    if (migration_is_running()) {
-        migration_file_set_error(ret, NULL);
-    }
-}
-
 bool vfio_device_state_is_running(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
@@ -287,9 +280,14 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
                                 iova, iova + iotlb->addr_mask);
 
     if (iotlb->target_as != &address_space_memory) {
-        error_report("Wrong target AS \"%s\", only system memory is allowed",
-                     iotlb->target_as->name ? iotlb->target_as->name : "none");
-        vfio_set_migration_error(-EINVAL);
+        error_setg(&local_err,
+                   "Wrong target AS \"%s\", only system memory is allowed",
+                   iotlb->target_as->name ? iotlb->target_as->name : "none");
+        if (migration_is_running()) {
+            migration_file_set_error(-EINVAL, local_err);
+        } else {
+            error_report_err(local_err);
+        }
         return;
     }
 
@@ -322,11 +320,16 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
         ret = vfio_container_dma_unmap(bcontainer, iova,
                                        iotlb->addr_mask + 1, iotlb);
         if (ret) {
-            error_report("vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
-                         "0x%"HWADDR_PRIx") = %d (%s)",
-                         bcontainer, iova,
-                         iotlb->addr_mask + 1, ret, strerror(-ret));
-            vfio_set_migration_error(ret);
+            error_setg(&local_err,
+                       "vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+                       "0x%"HWADDR_PRIx") = %d (%s)",
+                       bcontainer, iova,
+                       iotlb->addr_mask + 1, ret, strerror(-ret));
+            if (migration_is_running()) {
+                migration_file_set_error(ret, local_err);
+            } else {
+                error_report_err(local_err);
+            }
         }
     }
 out:
@@ -1108,8 +1111,11 @@ static void vfio_listener_log_global_stop(MemoryListener *listener)
     if (ret) {
         error_prepend(&local_err,
                       "vfio: Could not stop dirty page tracking - ");
-        error_report_err(local_err);
-        vfio_set_migration_error(ret);
+        if (migration_is_running()) {
+            migration_file_set_error(ret, local_err);
+        } else {
+            error_report_err(local_err);
+        }
     }
 }
 
@@ -1225,14 +1231,14 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
     trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);
 
     if (iotlb->target_as != &address_space_memory) {
-        error_report("Wrong target AS \"%s\", only system memory is allowed",
-                     iotlb->target_as->name ? iotlb->target_as->name : "none");
+        error_setg(&local_err,
+                   "Wrong target AS \"%s\", only system memory is allowed",
+                   iotlb->target_as->name ? iotlb->target_as->name : "none");
         goto out;
     }
 
     rcu_read_lock();
     if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL, &local_err)) {
-        error_report_err(local_err);
         goto out_unlock;
     }
 
@@ -1243,7 +1249,6 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
                       "vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
                       "0x%"HWADDR_PRIx") failed - ", bcontainer, iova,
                       iotlb->addr_mask + 1);
-        error_report_err(local_err);
     }
 
 out_unlock:
@@ -1251,7 +1256,11 @@ out_unlock:
 
 out:
     if (ret) {
-        vfio_set_migration_error(ret);
+        if (migration_is_running()) {
+            migration_file_set_error(ret, local_err);
+        } else {
+            error_report_err(local_err);
+        }
     }
 }
 
@@ -1384,8 +1393,11 @@ static void vfio_listener_log_sync(MemoryListener *listener,
     if (vfio_log_sync_needed(bcontainer)) {
         ret = vfio_sync_dirty_bitmap(bcontainer, section, &local_err);
         if (ret) {
-            error_report_err(local_err);
-            vfio_set_migration_error(ret);
+            if (migration_is_running()) {
+                migration_file_set_error(ret, local_err);
+            } else {
+                error_report_err(local_err);
+            }
         }
     }
 }