[PATCH] vfio/migration: Don't reset a device on a declined stop transition

[PATCH] vfio/migration: Don't reset a device on a declined stop transition

[Pankaj Kumar]

QEMU resets a VFIO device whenever a migration state transition it requests
fails. This is done by vfio_migration_set_state_or_reset(), which passes
VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
VFIO_DEVICE_RESET.

However, a device may legitimately decline a state transition, e.g. a vGPU
that refuses to stop while a profiling/debug session is active in the guest.
Such a device is still healthy and running the guest, so resetting it destroys
a perfectly good VM.

Instead of resetting on a declined stop, recover the device to its current
state. The current device state is always a valid, already-reached state, so
recovering to it is effectively a no-op transition that cannot itself fail and
trigger a reset:

 - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
   current device state for the stop direction. The resume direction keeps
   ERROR as the recover state, since a device that cannot resume is genuinely
   broken.

 - vfio_save_complete_precopy() recovers to the current device state rather
   than STOP. If it runs after an earlier stop was declined (the device still
   in a running state), the stop-copy attempt is then rolled back instead of
   resetting the device.

This lets vendor drivers decline a stop as a matter of policy without losing
the guest, and lets such drivers drop their own workarounds that swallowed the
spurious reset.

Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
Reviewed-by: Avihai Horon <avihaih@nvidia.com>
---
 hw/vfio/migration.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 73c49d8c24..4a98afa7c2 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -771,9 +771,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
 
     trace_vfio_save_complete_precopy_start(vbasedev->name);
 
-    /* We reach here with device state STOP or STOP_COPY only */
+    /*
+     * We normally reach here with the device in STOP or STOP_COPY. However, if
+     * an earlier stop transition was declined by the device, it may still be in
+     * a running state. Recover to the current device state rather than STOP, so
+     * a device that declines the stop-copy transition is rolled back to its
+     * current state instead of being reset out from under the guest.
+     */
     ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
-                                   VFIO_DEVICE_STATE_STOP, &local_err);
+                                   vbasedev->migration->device_state,
+                                   &local_err);
     if (ret) {
         error_report_err(local_err);
         return ret;
@@ -993,6 +1000,7 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
     VFIODevice *vbasedev = opaque;
     VFIOMigration *migration = vbasedev->migration;
     enum vfio_device_mig_state new_state;
+    enum vfio_device_mig_state recover_state;
     Error *local_err = NULL;
     int ret;
 
@@ -1009,7 +1017,15 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
         vfio_final_precopy_reinit_check(vbasedev);
     }
 
-    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
+    /*
+     * If the device declines a stop-direction transition, roll it back to its
+     * current state instead of resetting it out from under the guest. On resume
+     * (running), a device that cannot transition is genuinely broken, so reset
+     * it by using ERROR as the recover state.
+     */
+    recover_state = running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
+    ret = vfio_migration_set_state(vbasedev, new_state, recover_state,
+                                   &local_err);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
@@ -1026,21 +1042,34 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
 static void vfio_vmstate_change(void *opaque, bool running, RunState state)
 {
     VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
     enum vfio_device_mig_state new_state;
+    enum vfio_device_mig_state recover_state;
     Error *local_err = NULL;
     int ret;
 
     if (running) {
         new_state = VFIO_DEVICE_STATE_RUNNING;
+        /*
+         * If the device fails to resume, it is genuinely broken, so reset it
+         * by using ERROR as the recover state.
+         */
+        recover_state = VFIO_DEVICE_STATE_ERROR;
     } else {
         new_state =
             (vfio_device_state_is_precopy(vbasedev) &&
              (state == RUN_STATE_FINISH_MIGRATE || state == RUN_STATE_PAUSED)) ?
                 VFIO_DEVICE_STATE_STOP_COPY :
                 VFIO_DEVICE_STATE_STOP;
+        /*
+         * A device may decline to stop (e.g. it is busy). Roll it back to its
+         * current state rather than resetting it out from under the guest.
+         */
+        recover_state = migration->device_state;
     }
 
-    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
+    ret = vfio_migration_set_state(vbasedev, new_state, recover_state,
+                                   &local_err);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
-- 
2.25.1

Re: [PATCH] vfio/migration: Don't reset a device on a declined stop transition

[Cédric Le Goater]

Hello Pankaj

On 10/8/26 04:43, Pankaj Kumar wrote:
> QEMU resets a VFIO device whenever a migration state transition it requests
> fails. This is done by vfio_migration_set_state_or_reset(), which passes
> VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
> VFIO_DEVICE_RESET.
> 
> However, a device may legitimately decline a state transition, e.g. a vGPU
> that refuses to stop while a profiling/debug session is active in the guest.
> Such a device is still healthy and running the guest, so resetting it destroys
> a perfectly good VM.
> 
> Instead of resetting on a declined stop, recover the device to its current
> state. The current device state is always a valid, already-reached state, so
> recovering to it is effectively a no-op transition that cannot itself fail and
> trigger a reset:
> 
>   - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
>     current device state for the stop direction. The resume direction keeps
>     ERROR as the recover state, since a device that cannot resume is genuinely
>     broken.
> 
>   - vfio_save_complete_precopy() recovers to the current device state rather
>     than STOP. If it runs after an earlier stop was declined (the device still
>     in a running state), the stop-copy attempt is then rolled back instead of
>     resetting the device.
> 
> This lets vendor drivers decline a stop as a matter of policy without losing
> the guest, and lets such drivers drop their own workarounds that swallowed the
> spurious reset.
> 
> Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
> Reviewed-by: Avihai Horon <avihaih@nvidia.com>

I don't remember seeing this change before. If so please remove any
trailers before sending. Reviews should be done on the list, even
more so when the reviewer is a well known and trusted contributor !

> ---
>   hw/vfio/migration.c | 37 +++++++++++++++++++++++++++++++++----
>   1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 73c49d8c24..4a98afa7c2 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -771,9 +771,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
>   
>       trace_vfio_save_complete_precopy_start(vbasedev->name);
>   
> -    /* We reach here with device state STOP or STOP_COPY only */
> +    /*
> +     * We normally reach here with the device in STOP or STOP_COPY. However, if
> +     * an earlier stop transition was declined by the device, it may still be in
> +     * a running state. Recover to the current device state rather than STOP, so
> +     * a device that declines the stop-copy transition is rolled back to its
> +     * current state instead of being reset out from under the guest.
> +     */
>       ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
> -                                   VFIO_DEVICE_STATE_STOP, &local_err);
> +                                   vbasedev->migration->device_state,
> +                                   &local_err);
>       if (ret) {
>           error_report_err(local_err);
>           return ret;
> @@ -993,6 +1000,7 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>       VFIODevice *vbasedev = opaque;
>       VFIOMigration *migration = vbasedev->migration;
>       enum vfio_device_mig_state new_state;
> +    enum vfio_device_mig_state recover_state;
>       Error *local_err = NULL;
>       int ret;
>   
> @@ -1009,7 +1017,15 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>           vfio_final_precopy_reinit_check(vbasedev);
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    /*
> +     * If the device declines a stop-direction transition, roll it back to its
> +     * current state instead of resetting it out from under the guest. On resume
> +     * (running), a device that cannot transition is genuinely broken, so reset
> +     * it by using ERROR as the recover state.
> +     */
> +    recover_state = running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
> +    ret = vfio_migration_set_state(vbasedev, new_state, recover_state,
> +                                   &local_err);

How about introducing a common helper :

   static int vfio_migration_set_state_or_recover(VFIODevice *vbasedev,
                                       enum vfio_device_mig_state new_state,
                                       bool running, Error **errp)
   {
       VFIOMigration *migration = vbasedev->migration;
       enum vfio_device_mig_state recover_state =
           running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;

       return vfio_migration_set_state(vbasedev, new_state, recover_state, errp);
   }


C.


>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()
> @@ -1026,21 +1042,34 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>   static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>   {
>       VFIODevice *vbasedev = opaque;
> +    VFIOMigration *migration = vbasedev->migration;
>       enum vfio_device_mig_state new_state;
> +    enum vfio_device_mig_state recover_state;
>       Error *local_err = NULL;
>       int ret;
>   
>       if (running) {
>           new_state = VFIO_DEVICE_STATE_RUNNING;
> +        /*
> +         * If the device fails to resume, it is genuinely broken, so reset it
> +         * by using ERROR as the recover state.
> +         */
> +        recover_state = VFIO_DEVICE_STATE_ERROR;
>       } else {
>           new_state =
>               (vfio_device_state_is_precopy(vbasedev) &&
>                (state == RUN_STATE_FINISH_MIGRATE || state == RUN_STATE_PAUSED)) ?
>                   VFIO_DEVICE_STATE_STOP_COPY :
>                   VFIO_DEVICE_STATE_STOP;
> +        /*
> +         * A device may decline to stop (e.g. it is busy). Roll it back to its
> +         * current state rather than resetting it out from under the guest.
> +         */
> +        recover_state = migration->device_state;
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state(vbasedev, new_state, recover_state,
> +                                   &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()

[PATCH v2] vfio/migration: Don't reset a device on a declined stop transition

[Pankaj Kumar]

QEMU resets a VFIO device whenever a migration state transition it requests
fails. This is done by vfio_migration_set_state_or_reset(), which passes
VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
VFIO_DEVICE_RESET.

However, a device may legitimately decline a state transition, e.g. a vGPU
that refuses to stop while a profiling/debug session is active in the guest.
Such a device is still healthy and running the guest, so resetting it destroys
a perfectly good VM.

Instead of resetting on a declined stop, recover the device to its current
state. The current device state is always a valid, already-reached state, so
recovering to it is effectively a no-op transition that cannot itself fail and
trigger a reset.

Introduce a helper, vfio_migration_set_state_or_recover(), that selects the
recover state based on the transition direction, and route the vmstate change
and save-completion paths through it:

 - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
   current device state for the stop direction. The resume direction keeps
   ERROR as the recover state, since a device that cannot resume is genuinely
   broken.

 - vfio_save_complete_precopy() recovers to the current device state rather
   than STOP. If it runs after an earlier stop was declined (the device still
   in a running state), the stop-copy attempt is then rolled back instead of
   resetting the device.

The existing vfio_migration_set_state_or_reset() is retained for the paths
that must still reset on failure (save cleanup and the migration-failed
notifier).

This lets vendor drivers decline a stop as a matter of policy without losing
the guest, and lets such drivers drop their own workarounds that swallowed the
spurious reset.

Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
---
v2:
 - Introduce a vfio_migration_set_state_or_recover() helper that selects the
   recover state by transition direction, and route all three call sites
   through it instead of open-coding the recover-state selection (Cédric).
 - Keep vfio_migration_set_state_or_reset() for the paths that must still
   reset on failure (save cleanup and the migration-failed notifier).

v1: https://lore.kernel.org/qemu-devel/20261008024329.1996536-1-pshit@nvidia.com/

 hw/vfio/migration.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 73c49d8c24..69d50cebdd 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -246,6 +246,25 @@ vfio_migration_set_state_or_reset(VFIODevice *vbasedev,
                                     VFIO_DEVICE_STATE_ERROR, errp);
 }
 
+/*
+ * On a stop-direction transition failure, recover the device to its current
+ * state instead of resetting it: the current state is valid and already
+ * reached, so recovering to it is effectively a no-op that cannot itself fail.
+ * On a resume-direction (running) failure, the device is genuinely broken, so
+ * reset it by using ERROR as the recover state.
+ */
+static int
+vfio_migration_set_state_or_recover(VFIODevice *vbasedev,
+                                    enum vfio_device_mig_state new_state,
+                                    bool running, Error **errp)
+{
+    VFIOMigration *migration = vbasedev->migration;
+    enum vfio_device_mig_state recover_state =
+        running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
+
+    return vfio_migration_set_state(vbasedev, new_state, recover_state, errp);
+}
+
 static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
                             uint64_t data_size)
 {
@@ -771,9 +790,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
 
     trace_vfio_save_complete_precopy_start(vbasedev->name);
 
-    /* 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, &local_err);
+    /*
+     * We normally reach here with the device in STOP or STOP_COPY. However, if
+     * an earlier stop transition was declined by the device, it may still be in
+     * a running state. Recover to the current device state rather than STOP, so
+     * a device that declines the stop-copy transition is rolled back to its
+     * current state instead of being reset out from under the guest.
+     */
+    ret = vfio_migration_set_state_or_recover(vbasedev,
+                                              VFIO_DEVICE_STATE_STOP_COPY,
+                                              false, &local_err);
     if (ret) {
         error_report_err(local_err);
         return ret;
@@ -1009,7 +1035,8 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
         vfio_final_precopy_reinit_check(vbasedev);
     }
 
-    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
+    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
+                                              &local_err);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
@@ -1040,7 +1067,8 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
                 VFIO_DEVICE_STATE_STOP;
     }
 
-    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
+    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
+                                              &local_err);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
-- 
2.25.1

Re: [PATCH v2] vfio/migration: Don't reset a device on a declined stop transition

[Cédric Le Goater]

On 10/13/26 06:45, Pankaj Kumar wrote:
> QEMU resets a VFIO device whenever a migration state transition it requests
> fails. This is done by vfio_migration_set_state_or_reset(), which passes
> VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
> VFIO_DEVICE_RESET.
> 
> However, a device may legitimately decline a state transition, e.g. a vGPU
> that refuses to stop while a profiling/debug session is active in the guest.
> Such a device is still healthy and running the guest, so resetting it destroys
> a perfectly good VM.
> 
> Instead of resetting on a declined stop, recover the device to its current
> state. The current device state is always a valid, already-reached state, so
> recovering to it is effectively a no-op transition that cannot itself fail and
> trigger a reset.
> 
> Introduce a helper, vfio_migration_set_state_or_recover(), that selects the
> recover state based on the transition direction, and route the vmstate change
> and save-completion paths through it:
> 
>   - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
>     current device state for the stop direction. The resume direction keeps
>     ERROR as the recover state, since a device that cannot resume is genuinely
>     broken.
> 
>   - vfio_save_complete_precopy() recovers to the current device state rather
>     than STOP. If it runs after an earlier stop was declined (the device still
>     in a running state), the stop-copy attempt is then rolled back instead of
>     resetting the device.
> 
> The existing vfio_migration_set_state_or_reset() is retained for the paths
> that must still reset on failure (save cleanup and the migration-failed
> notifier).
> 
> This lets vendor drivers decline a stop as a matter of policy without losing
> the guest, and lets such drivers drop their own workarounds that swallowed the
> spurious reset.
> 
> Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
> ---
> v2:
>   - Introduce a vfio_migration_set_state_or_recover() helper that selects the
>     recover state by transition direction, and route all three call sites
>     through it instead of open-coding the recover-state selection (Cédric).
>   - Keep vfio_migration_set_state_or_reset() for the paths that must still
>     reset on failure (save cleanup and the migration-failed notifier).
> 
> v1: https://lore.kernel.org/qemu-devel/20261008024329.1996536-1-pshit@nvidia.com/
> 
>   hw/vfio/migration.c | 38 +++++++++++++++++++++++++++++++++-----
>   1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 73c49d8c24..69d50cebdd 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -246,6 +246,25 @@ vfio_migration_set_state_or_reset(VFIODevice *vbasedev,
>                                       VFIO_DEVICE_STATE_ERROR, errp);
>   }
>   
> +/*
> + * On a stop-direction transition failure, recover the device to its current
> + * state instead of resetting it: the current state is valid and already
> + * reached, so recovering to it is effectively a no-op that cannot itself fail.
> + * On a resume-direction (running) failure, the device is genuinely broken, so
> + * reset it by using ERROR as the recover state.
> + */
> +static int
> +vfio_migration_set_state_or_recover(VFIODevice *vbasedev,
> +                                    enum vfio_device_mig_state new_state,
> +                                    bool running, Error **errp)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +    enum vfio_device_mig_state recover_state =
> +        running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
> +
> +    return vfio_migration_set_state(vbasedev, new_state, recover_state, errp);
> +}
> +
>   static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
>                               uint64_t data_size)
>   {
> @@ -771,9 +790,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
>   
>       trace_vfio_save_complete_precopy_start(vbasedev->name);
>   
> -    /* 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, &local_err);
> +    /*
> +     * We normally reach here with the device in STOP or STOP_COPY. However, if
> +     * an earlier stop transition was declined by the device, it may still be in
> +     * a running state. Recover to the current device state rather than STOP, so
> +     * a device that declines the stop-copy transition is rolled back to its
> +     * current state instead of being reset out from under the guest.
> +     */
> +    ret = vfio_migration_set_state_or_recover(vbasedev,
> +                                              VFIO_DEVICE_STATE_STOP_COPY,
> +                                              false, &local_err);
>       if (ret) {
>           error_report_err(local_err);
>           return ret;
> @@ -1009,7 +1035,8 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>           vfio_final_precopy_reinit_check(vbasedev);
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
> +                                              &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()
> @@ -1040,7 +1067,8 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>                   VFIO_DEVICE_STATE_STOP;
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
> +                                              &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()

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

Thanks,

C.

Re: [PATCH v2] vfio/migration: Don't reset a device on a declined stop transition

[Avihai Horon]

On 10/13/2026 7:45 AM, Pankaj Kumar wrote:
> QEMU resets a VFIO device whenever a migration state transition it requests
> fails. This is done by vfio_migration_set_state_or_reset(), which passes
> VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
> VFIO_DEVICE_RESET.
>
> However, a device may legitimately decline a state transition, e.g. a vGPU
> that refuses to stop while a profiling/debug session is active in the guest.
> Such a device is still healthy and running the guest, so resetting it destroys
> a perfectly good VM.
>
> Instead of resetting on a declined stop, recover the device to its current
> state. The current device state is always a valid, already-reached state, so
> recovering to it is effectively a no-op transition that cannot itself fail and
> trigger a reset.
>
> Introduce a helper, vfio_migration_set_state_or_recover(), that selects the
> recover state based on the transition direction, and route the vmstate change
> and save-completion paths through it:
>
>   - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
>     current device state for the stop direction. The resume direction keeps
>     ERROR as the recover state, since a device that cannot resume is genuinely
>     broken.
>
>   - vfio_save_complete_precopy() recovers to the current device state rather
>     than STOP. If it runs after an earlier stop was declined (the device still
>     in a running state), the stop-copy attempt is then rolled back instead of
>     resetting the device.
>
> The existing vfio_migration_set_state_or_reset() is retained for the paths
> that must still reset on failure (save cleanup and the migration-failed
> notifier).
>
> This lets vendor drivers decline a stop as a matter of policy without losing
> the guest, and lets such drivers drop their own workarounds that swallowed the
> spurious reset.
>
> Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
> ---
> v2:
>   - Introduce a vfio_migration_set_state_or_recover() helper that selects the
>     recover state by transition direction, and route all three call sites
>     through it instead of open-coding the recover-state selection (Cédric).
>   - Keep vfio_migration_set_state_or_reset() for the paths that must still
>     reset on failure (save cleanup and the migration-failed notifier).
>
> v1: https://lore.kernel.org/qemu-devel/20261008024329.1996536-1-pshit@nvidia.com/
>
>   hw/vfio/migration.c | 38 +++++++++++++++++++++++++++++++++-----
>   1 file changed, 33 insertions(+), 5 deletions(-)

Reviewed-by: Avihai Horon <avihaih@nvidia.com>

Thanks.

> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 73c49d8c24..69d50cebdd 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -246,6 +246,25 @@ vfio_migration_set_state_or_reset(VFIODevice *vbasedev,
>                                       VFIO_DEVICE_STATE_ERROR, errp);
>   }
>   
> +/*
> + * On a stop-direction transition failure, recover the device to its current
> + * state instead of resetting it: the current state is valid and already
> + * reached, so recovering to it is effectively a no-op that cannot itself fail.
> + * On a resume-direction (running) failure, the device is genuinely broken, so
> + * reset it by using ERROR as the recover state.
> + */
> +static int
> +vfio_migration_set_state_or_recover(VFIODevice *vbasedev,
> +                                    enum vfio_device_mig_state new_state,
> +                                    bool running, Error **errp)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +    enum vfio_device_mig_state recover_state =
> +        running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
> +
> +    return vfio_migration_set_state(vbasedev, new_state, recover_state, errp);
> +}
> +
>   static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
>                               uint64_t data_size)
>   {
> @@ -771,9 +790,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
>   
>       trace_vfio_save_complete_precopy_start(vbasedev->name);
>   
> -    /* 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, &local_err);
> +    /*
> +     * We normally reach here with the device in STOP or STOP_COPY. However, if
> +     * an earlier stop transition was declined by the device, it may still be in
> +     * a running state. Recover to the current device state rather than STOP, so
> +     * a device that declines the stop-copy transition is rolled back to its
> +     * current state instead of being reset out from under the guest.
> +     */
> +    ret = vfio_migration_set_state_or_recover(vbasedev,
> +                                              VFIO_DEVICE_STATE_STOP_COPY,
> +                                              false, &local_err);
>       if (ret) {
>           error_report_err(local_err);
>           return ret;
> @@ -1009,7 +1035,8 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>           vfio_final_precopy_reinit_check(vbasedev);
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
> +                                              &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()
> @@ -1040,7 +1067,8 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>                   VFIO_DEVICE_STATE_STOP;
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
> +                                              &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()

Re: [PATCH v2] vfio/migration: Don't reset a device on a declined stop transition

[Cédric Le Goater]

On 10/13/26 06:45, Pankaj Kumar wrote:
> QEMU resets a VFIO device whenever a migration state transition it requests
> fails. This is done by vfio_migration_set_state_or_reset(), which passes
> VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
> VFIO_DEVICE_RESET.
> 
> However, a device may legitimately decline a state transition, e.g. a vGPU
> that refuses to stop while a profiling/debug session is active in the guest.
> Such a device is still healthy and running the guest, so resetting it destroys
> a perfectly good VM.
> 
> Instead of resetting on a declined stop, recover the device to its current
> state. The current device state is always a valid, already-reached state, so
> recovering to it is effectively a no-op transition that cannot itself fail and
> trigger a reset.
> 
> Introduce a helper, vfio_migration_set_state_or_recover(), that selects the
> recover state based on the transition direction, and route the vmstate change
> and save-completion paths through it:
> 
>   - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
>     current device state for the stop direction. The resume direction keeps
>     ERROR as the recover state, since a device that cannot resume is genuinely
>     broken.
> 
>   - vfio_save_complete_precopy() recovers to the current device state rather
>     than STOP. If it runs after an earlier stop was declined (the device still
>     in a running state), the stop-copy attempt is then rolled back instead of
>     resetting the device.
> 
> The existing vfio_migration_set_state_or_reset() is retained for the paths
> that must still reset on failure (save cleanup and the migration-failed
> notifier).
> 
> This lets vendor drivers decline a stop as a matter of policy without losing
> the guest, and lets such drivers drop their own workarounds that swallowed the
> spurious reset.
> 
> Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
> ---
> v2:
>   - Introduce a vfio_migration_set_state_or_recover() helper that selects the
>     recover state by transition direction, and route all three call sites
>     through it instead of open-coding the recover-state selection (Cédric).
>   - Keep vfio_migration_set_state_or_reset() for the paths that must still
>     reset on failure (save cleanup and the migration-failed notifier).
> 
> v1: https://lore.kernel.org/qemu-devel/20261008024329.1996536-1-pshit@nvidia.com/
> 
>   hw/vfio/migration.c | 38 +++++++++++++++++++++++++++++++++-----
>   1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 73c49d8c24..69d50cebdd 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -246,6 +246,25 @@ vfio_migration_set_state_or_reset(VFIODevice *vbasedev,
>                                       VFIO_DEVICE_STATE_ERROR, errp);
>   }
>   
> +/*
> + * On a stop-direction transition failure, recover the device to its current
> + * state instead of resetting it: the current state is valid and already
> + * reached, so recovering to it is effectively a no-op that cannot itself fail.
> + * On a resume-direction (running) failure, the device is genuinely broken, so
> + * reset it by using ERROR as the recover state.
> + */
> +static int
> +vfio_migration_set_state_or_recover(VFIODevice *vbasedev,
> +                                    enum vfio_device_mig_state new_state,
> +                                    bool running, Error **errp)
> +{
> +    VFIOMigration *migration = vbasedev->migration;
> +    enum vfio_device_mig_state recover_state =
> +        running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
> +
> +    return vfio_migration_set_state(vbasedev, new_state, recover_state, errp);
> +}
> +
>   static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
>                               uint64_t data_size)
>   {
> @@ -771,9 +790,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
>   
>       trace_vfio_save_complete_precopy_start(vbasedev->name);
>   
> -    /* 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, &local_err);
> +    /*
> +     * We normally reach here with the device in STOP or STOP_COPY. However, if
> +     * an earlier stop transition was declined by the device, it may still be in
> +     * a running state. Recover to the current device state rather than STOP, so
> +     * a device that declines the stop-copy transition is rolled back to its
> +     * current state instead of being reset out from under the guest.
> +     */
> +    ret = vfio_migration_set_state_or_recover(vbasedev,
> +                                              VFIO_DEVICE_STATE_STOP_COPY,
> +                                              false, &local_err);
>       if (ret) {
>           error_report_err(local_err);
>           return ret;
> @@ -1009,7 +1035,8 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>           vfio_final_precopy_reinit_check(vbasedev);
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
> +                                              &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()
> @@ -1040,7 +1067,8 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>                   VFIO_DEVICE_STATE_STOP;
>       }
>   
> -    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
> +    ret = vfio_migration_set_state_or_recover(vbasedev, new_state, running,
> +                                              &local_err);
>       if (ret) {
>           /*
>            * Migration should be aborted in this case, but vm_state_notify()

Applied to

     https://github.com/legoater/qemu vfio-next

Thanks,

C.