Re: [PATCH 08/14] vfio: Use new Error** argument in vfio_save_setup()

[Philippe Mathieu-Daudé]

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

Add an Error** argument to vfio_migration_set_state() and adjust
callers, including vfio_save_setup(). The error will be propagated up
to qemu_savevm_state_setup() where the save_setup() handler is
executed.

Signed-off-by: Cédric Le Goater <c...@redhat.com>
---
  hw/vfio/migration.c | 62 +++++++++++++++++++++++++++++----------------
  1 file changed, 40 insertions(+), 22 deletions(-)

@@ -429,13 +431,18 @@ static void vfio_save_cleanup(void *opaque)
  {
      VFIODevice *vbasedev = opaque;
      VFIOMigration *migration = vbasedev->migration;
+    Error *local_err = NULL;

/*

       * Changing device state from STOP_COPY to STOP can take time. Do it here,
       * after migration has completed, so it won't increase downtime.
       */
      if (migration->device_state == VFIO_DEVICE_STATE_STOP_COPY) {
-        vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_STOP);
+        vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_STOP,
+                                          &local_err);
+        if (local_err) {

Please check callee return value instead.

+            error_report_err(local_err);
+        }
      }

g_free(migration->data_buffer);

@@ -541,11 +548,13 @@ static int vfio_save_complete_precopy(QEMUFile *f, void 
*opaque)
      VFIODevice *vbasedev = opaque;
      ssize_t data_size;
      int ret;
+    Error *local_err = NULL;

/* We reach here with device state STOP or STOP_COPY only */

      ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
-                                   VFIO_DEVICE_STATE_STOP);
-    if (ret) {
+                                   VFIO_DEVICE_STATE_STOP, &local_err);
+    if (local_err) {

Ditto.

+        error_report_err(local_err);
          return ret;
      }

@@ -760,6 +773,7 @@ static void vfio_migration_state_notifier(Notifier 
*notifier, void *data)
      VFIOMigration *migration = container_of(notifier, VFIOMigration,
                                              migration_state);
      VFIODevice *vbasedev = migration->vbasedev;
+    Error *local_err = NULL;

trace_vfio_migration_state_notifier(vbasedev->name,

                                          MigrationStatus_str(s->state));
@@ -768,7 +782,11 @@ static void vfio_migration_state_notifier(Notifier 
*notifier, void *data)
      case MIGRATION_STATUS_CANCELLING:
      case MIGRATION_STATUS_CANCELLED:
      case MIGRATION_STATUS_FAILED:
-        vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_RUNNING);
+        vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_RUNNING,
+                                          &local_err);
+        if (local_err) {

Ditto.

+            error_report_err(local_err);
+        }
      }
  }