diff mbox series

vfio/migration: Add helper function to set state or reset device

Message ID 20231231104818.17666-1-avihaih@nvidia.com (mailing list archive)
State New, archived
Headers show
Series vfio/migration: Add helper function to set state or reset device | expand

Commit Message

Avihai Horon Dec. 31, 2023, 10:48 a.m. UTC 
There are several places where failure in setting the device state leads
to a device reset, which is done by setting ERROR as the recover state.

Add a helper function that sets the device state and resets the device
in case of failure. This will make the code cleaner and remove duplicate
comments.

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

Comments

Cédric Le Goater Jan. 2, 2024, 9:20 a.m. UTC | #1 
On 12/31/23 11:48, Avihai Horon wrote:
> There are several places where failure in setting the device state leads
> to a device reset, which is done by setting ERROR as the recover state.
> 
> Add a helper function that sets the device state and resets the device
> in case of failure. This will make the code cleaner and remove duplicate
> comments.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.
Philippe Mathieu-Daudé Jan. 2, 2024, 9:55 a.m. UTC | #2 
On 31/12/23 11:48, Avihai Horon wrote:
> There are several places where failure in setting the device state leads
> to a device reset, which is done by setting ERROR as the recover state.
> 
> Add a helper function that sets the device state and resets the device
> in case of failure. This will make the code cleaner and remove duplicate
> comments.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
>   hw/vfio/migration.c | 41 +++++++++++++++++------------------------
>   1 file changed, 17 insertions(+), 24 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Cédric Le Goater Jan. 2, 2024, 10:22 a.m. UTC | #3 
On 12/31/23 11:48, Avihai Horon wrote:
> There are several places where failure in setting the device state leads
> to a device reset, which is done by setting ERROR as the recover state.
> 
> Add a helper function that sets the device state and resets the device
> in case of failure. This will make the code cleaner and remove duplicate
> comments.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>


Applied to vfio-next.

Thanks,

C.
diff mbox series

Patch

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 28d422b39f..70e6b1a709 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -163,6 +163,19 @@  reset_device:
     return ret;
 }
 
+/*
+ * Some device state transitions require resetting the device if they fail.
+ * This function sets the device in new_state and resets the device if that
+ * fails. Reset is done by using ERROR as the recover state.
+ */
+static int
+vfio_migration_set_state_or_reset(VFIODevice *vbasedev,
+                                  enum vfio_device_mig_state new_state)
+{
+    return vfio_migration_set_state(vbasedev, new_state,
+                                    VFIO_DEVICE_STATE_ERROR);
+}
+
 static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
                             uint64_t data_size)
 {
@@ -422,12 +435,7 @@  static void vfio_save_cleanup(void *opaque)
      * after migration has completed, so it won't increase downtime.
      */
     if (migration->device_state == VFIO_DEVICE_STATE_STOP_COPY) {
-        /*
-         * If setting the device in STOP state fails, the device should be
-         * reset. To do so, use ERROR state as a recover state.
-         */
-        vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
-                                 VFIO_DEVICE_STATE_ERROR);
+        vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_STOP);
     }
 
     g_free(migration->data_buffer);
@@ -699,12 +707,7 @@  static void vfio_vmstate_change_prepare(void *opaque, bool running,
                     VFIO_DEVICE_STATE_PRE_COPY_P2P :
                     VFIO_DEVICE_STATE_RUNNING_P2P;
 
-    /*
-     * If setting the device in new_state fails, the device should be reset.
-     * To do so, use ERROR state as a recover state.
-     */
-    ret = vfio_migration_set_state(vbasedev, new_state,
-                                   VFIO_DEVICE_STATE_ERROR);
+    ret = vfio_migration_set_state_or_reset(vbasedev, new_state);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
@@ -736,12 +739,7 @@  static void vfio_vmstate_change(void *opaque, bool running, RunState state)
                 VFIO_DEVICE_STATE_STOP;
     }
 
-    /*
-     * If setting the device in new_state fails, the device should be reset.
-     * To do so, use ERROR state as a recover state.
-     */
-    ret = vfio_migration_set_state(vbasedev, new_state,
-                                   VFIO_DEVICE_STATE_ERROR);
+    ret = vfio_migration_set_state_or_reset(vbasedev, new_state);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
@@ -770,12 +768,7 @@  static void vfio_migration_state_notifier(Notifier *notifier, void *data)
     case MIGRATION_STATUS_CANCELLING:
     case MIGRATION_STATUS_CANCELLED:
     case MIGRATION_STATUS_FAILED:
-        /*
-         * If setting the device in RUNNING state fails, the device should
-         * be reset. To do so, use ERROR state as a recover state.
-         */
-        vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING,
-                                 VFIO_DEVICE_STATE_ERROR);
+        vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_RUNNING);
     }
 }