Re: [PATCH 10/14] vfio/migration: Add Error ** parameter to vfio_migration_init()

[Avihai Horon]

On 5/8/2026 4:18 PM, Avihai Horon wrote:

On 5/7/2026 10:59 AM, Cédric Le Goater wrote:
External email: Use caution opening links or attachments


Hello Avihai,

On 5/5/26 10:14, Avihai Horon wrote:
vfio_migration_init() already has many failure points and a new one 
will
be added in next patch.

Add Error ** parameter to vfio_migration_init() to report a detailed
error message through it.

Signed-off-by: Avihai Horon <avih...@nvidia.com>
---
  hw/vfio/migration.c | 29 +++++++++++++++++------------
  1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 243624b5fe..b7e929274a 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -1038,7 +1038,7 @@ static bool 
vfio_dma_logging_supported(VFIODevice *vbasedev)
      return !ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature);
  }

-static int vfio_migration_init(VFIODevice *vbasedev)
+static int vfio_migration_init(VFIODevice *vbasedev, Error **errp)
  {
      int ret;
      Object *obj;
@@ -1047,23 +1047,38 @@ static int vfio_migration_init(VFIODevice 
*vbasedev)
      g_autofree char *path = NULL, *oid = NULL;
      uint64_t mig_flags = 0;
      VMChangeStateHandler *prepare_cb;
+    g_autofree char *error_prefix =
+        g_strdup_printf("%s: VFIO migration init failed:", 
vbasedev->name);

We have error_prepend() for this purpose.

Right.
I was trying to avoid duplicating the prefix on each fail branch.

Do you suggest to add a common "err:" goto label at the bottom and put 
there a single:

  error_prepend(errp, "%s: VFIO migration init failed:", vbasedev->name);

?

Or alternatively, put it in vfio_migration_realize():

    ret = vfio_migration_init(vbasedev, &err);
    if (ret) {
        error_prepend(err, "%s: VFIO migration init failed:", 
vbasedev->name);
        return !vfio_block_migration(vbasedev, err, errp);
    }

?



Thanks.




      if (!vbasedev->ops->vfio_get_object) {
+        error_setg(errp, "%s no vfio_get_object handler", 
error_prefix);
          return -EINVAL;
      }

      obj = vbasedev->ops->vfio_get_object(vbasedev);
      if (!obj) {
+        error_setg(errp, "%s failed to get object", error_prefix);
          return -EINVAL;
      }

      ret = vfio_migration_query_flags(vbasedev, &mig_flags);
      if (ret) {
+        if (ret == -ENOTTY) {
+            error_setg_errno(errp, -ret,
+                             "%s migration is not supported in 
kernel",
+                             error_prefix);
+        } else {
+            error_setg_errno(errp, -ret, "%s failed to query 
migration flags",
+                             error_prefix);
+        }
+
          return ret;
      }

      /* Basic migration functionality must be supported */
      if (!(mig_flags & VFIO_MIGRATION_STOP_COPY)) {
+        error_setg(errp, "%s VFIO_MIGRATION_STOP_COPY is not 
supported",
+                   error_prefix);
          return -EOPNOTSUPP;
      }

@@ -1261,18 +1276,8 @@ bool vfio_migration_realize(VFIODevice 
*vbasedev, Error **errp)
          return !vfio_block_migration(vbasedev, err, errp);
      }

-    ret = vfio_migration_init(vbasedev);
+    ret = vfio_migration_init(vbasedev, &err);
      if (ret) {
-        if (ret == -ENOTTY) {
-            error_setg(&err, "%s: VFIO migration is not supported 
in kernel",
-                       vbasedev->name);
-        } else {
-            error_setg(&err,
-                       "%s: Migration couldn't be initialized for 
VFIO device, "
-                       "err: %d (%s)",
-                       vbasedev->name, ret, strerror(-ret));
-        }
-
          return !vfio_block_migration(vbasedev, err, errp);
      }