* Re: [PATCH v2] vfio/migration: Don't reset a device on a declined stop transition
2026-10-13 4:45 ` [PATCH v2] " Pankaj Kumar
@ 2026-10-13 6:33 ` Cédric Le Goater
2026-10-13 8:36 ` Avihai Horon
2026-10-13 12:13 ` Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2026-10-13 6:33 UTC (permalink / raw)
To: Pankaj Kumar, alex
Cc: qemu-devel, pmohite, dnigam, avihaih, alwilliamson, smitra,
kwankhede, vkale, targupta
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] vfio/migration: Don't reset a device on a declined stop transition
2026-10-13 4:45 ` [PATCH v2] " Pankaj Kumar
2026-10-13 6:33 ` Cédric Le Goater
@ 2026-10-13 8:36 ` Avihai Horon
2026-10-13 12:13 ` Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Avihai Horon @ 2026-10-13 8:36 UTC (permalink / raw)
To: Pankaj Kumar, alex, clg
Cc: qemu-devel, pmohite, dnigam, alwilliamson, smitra, kwankhede,
vkale, targupta
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()
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] vfio/migration: Don't reset a device on a declined stop transition
2026-10-13 4:45 ` [PATCH v2] " Pankaj Kumar
2026-10-13 6:33 ` Cédric Le Goater
2026-10-13 8:36 ` Avihai Horon
@ 2026-10-13 12:13 ` Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2026-10-13 12:13 UTC (permalink / raw)
To: Pankaj Kumar, alex
Cc: qemu-devel, pmohite, dnigam, avihaih, alwilliamson, smitra,
kwankhede, vkale, targupta
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.
^ permalink raw reply [flat|nested] 6+ messages in thread