[PATCH hci-dev v2 0/1] vfio/migration: don't touch the device on guest shutdown #VSTOR-153976

[PATCH hci-dev v2 0/1] vfio/migration: don't touch the device on guest shutdown #VSTOR-153976

[Andrey Drobyshev]

v1 -> v2, after Sashiko's review:

 - skip the STOP only when the shutdown action is "pause", so quit, a
   signal and a plain guest shutdown still stop the device on exit;
 - stop the device in vfio_save_setup() if a migration or savevm is
   started on the stopped VM, since no VM state change comes then.

v1: http://lore.virtuozzo.com/qemu/010701a1b7cff882-2e4d6e19-0372-4865-8ed1-19d654b0ba16-000000@eu-central-1.amazonses.com

Andrey Drobyshev (1):
  vfio/migration: don't touch the device on guest shutdown #VSTOR-153976

 hw/vfio/migration.c       | 30 ++++++++++++++++++++++++++++++
 include/system/runstate.h |  1 +
 system/runstate.c         |  5 +++++
 3 files changed, 36 insertions(+)

--
2.47.1

[PATCH hci-vev v2 1/1] vfio/migration: don't touch the device on guest shutdown #VSTOR-153976

[Andrey Drobyshev]

When the guest shuts down and QEMU runs with -no-shutdown, the VM stays
stopped and can only be reset or quit. A libvirt soft reboot is exactly
that: guest shutdown, system_reset, cont. Today vfio_vmstate_change()
treats this stop like any other and moves the device to STOP, then cont
brings it back to RUNNING.

NVIDIA's vGPU manager (595.71.03) takes that STOP for the start of a
migration, finds no guest driver, breaks its dirty page tracking and
never repairs it when the driver loads again. Every live migration of
the VM after a soft reboot then fails: "Error saving page in pipelined
mode".

Do not touch the device state in this case. There is nothing to
quiesce, and the reset that must follow resets the device anyway. The
case is told apart by the shutdown action: only "pause" (-no-shutdown)
keeps the VM alive. quit, a signal and a plain guest shutdown use
"poweroff" and still stop the device on the way out, as before. If a
migration or savevm is started on the stopped VM, no VM state change
comes at switchover, so vfio_save_setup() stops the device itself.

With this patch applied, soft reboot + subsequent live migration pass
successfully.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/vfio/migration.c       | 30 ++++++++++++++++++++++++++++++
 include/system/runstate.h |  1 +
 system/runstate.c         |  5 +++++
 3 files changed, 36 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 7e7fb496bbe..2ff3e49d5de 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -473,6 +473,19 @@ static int vfio_save_setup(QEMUFile *f, void *opaque, Error **errp)
     migration->event_save_iterate_started = false;
     migration->event_precopy_empty_hit = false;
 
+    /*
+     * A VM that is already stopped gets no VM state change at switchover.
+     * If a guest shutdown left the device running, stop it now.
+     */
+    if (!runstate_is_running() &&
+        migration->device_state == VFIO_DEVICE_STATE_RUNNING) {
+        ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
+                                       VFIO_DEVICE_STATE_RUNNING, errp);
+        if (ret) {
+            return ret;
+        }
+    }
+
     if (vfio_precopy_supported(vbasedev)) {
         switch (migration->device_state) {
         case VFIO_DEVICE_STATE_RUNNING:
@@ -899,6 +912,12 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
         return;
     }
 
+    /* The guest has shut down and QEMU keeps the VM stopped: leave it */
+    if (!running && state == RUN_STATE_SHUTDOWN &&
+        qemu_shutdown_action_is_pause()) {
+        return;
+    }
+
     new_state = migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ?
                     VFIO_DEVICE_STATE_PRE_COPY_P2P :
                     VFIO_DEVICE_STATE_RUNNING_P2P;
@@ -936,6 +955,17 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
         return;
     }
 
+    /*
+     * The guest has shut down and QEMU keeps the VM stopped. It can only
+     * be reset or quit now, so leave the device state as it is: the reset
+     * resets the device too. The exit path uses the same run state with
+     * the poweroff action and stops the device as before.
+     */
+    if (!running && state == RUN_STATE_SHUTDOWN &&
+        qemu_shutdown_action_is_pause()) {
+        return;
+    }
+
     if (running) {
         new_state = VFIO_DEVICE_STATE_RUNNING;
     } else {
diff --git a/include/system/runstate.h b/include/system/runstate.h
index 929379adae4..29a919348da 100644
--- a/include/system/runstate.h
+++ b/include/system/runstate.h
@@ -148,6 +148,7 @@ void qemu_system_vmstop_request_prepare(void);
 bool qemu_vmstop_requested(RunState *r);
 ShutdownCause qemu_shutdown_requested_get(void);
 bool qemu_force_shutdown_requested(void);
+bool qemu_shutdown_action_is_pause(void);
 ShutdownCause qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
 void qemu_system_reset(ShutdownCause reason);
diff --git a/system/runstate.c b/system/runstate.c
index 6178b0091a3..1cdecde0fcf 100644
--- a/system/runstate.c
+++ b/system/runstate.c
@@ -433,6 +433,11 @@ bool qemu_force_shutdown_requested(void)
     return force_shutdown;
 }
 
+bool qemu_shutdown_action_is_pause(void)
+{
+    return shutdown_action == SHUTDOWN_ACTION_PAUSE;
+}
+
 ShutdownCause qemu_reset_requested_get(void)
 {
     return reset_requested;
-- 
2.47.1

Re: [PATCH hci-vev v2 1/1] vfio/migration: don't touch the device on guest shutdown #VSTOR-153976

[Denis V. Lunev]

On 11/8/26 21:50, Andrey Drobyshev wrote:
> When the guest shuts down and QEMU runs with -no-shutdown, the VM stays
> stopped and can only be reset or quit. A libvirt soft reboot is exactly
> that: guest shutdown, system_reset, cont. Today vfio_vmstate_change()
> treats this stop like any other and moves the device to STOP, then cont
> brings it back to RUNNING.
>
> NVIDIA's vGPU manager (595.71.03) takes that STOP for the start of a
> migration, finds no guest driver, breaks its dirty page tracking and
> never repairs it when the driver loads again. Every live migration of
> the VM after a soft reboot then fails: "Error saving page in pipelined
> mode".
>
> Do not touch the device state in this case. There is nothing to
> quiesce, and the reset that must follow resets the device anyway. The
> case is told apart by the shutdown action: only "pause" (-no-shutdown)
> keeps the VM alive. quit, a signal and a plain guest shutdown use
> "poweroff" and still stop the device on the way out, as before. If a
> migration or savevm is started on the stopped VM, no VM state change
> comes at switchover, so vfio_save_setup() stops the device itself.
>
> With this patch applied, soft reboot + subsequent live migration pass
> successfully.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
>  hw/vfio/migration.c       | 30 ++++++++++++++++++++++++++++++
>  include/system/runstate.h |  1 +
>  system/runstate.c         |  5 +++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 7e7fb496bbe..2ff3e49d5de 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -473,6 +473,19 @@ static int vfio_save_setup(QEMUFile *f, void *opaque, Error **errp)
>      migration->event_save_iterate_started = false;
>      migration->event_precopy_empty_hit = false;
>  
> +    /*
> +     * A VM that is already stopped gets no VM state change at switchover.
> +     * If a guest shutdown left the device running, stop it now.
> +     */
> +    if (!runstate_is_running() &&
> +        migration->device_state == VFIO_DEVICE_STATE_RUNNING) {
> +        ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
> +                                       VFIO_DEVICE_STATE_RUNNING, errp);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
>      if (vfio_precopy_supported(vbasedev)) {
>          switch (migration->device_state) {
>          case VFIO_DEVICE_STATE_RUNNING:
> @@ -899,6 +912,12 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>          return;
>      }
>  
> +    /* The guest has shut down and QEMU keeps the VM stopped: leave it */
> +    if (!running && state == RUN_STATE_SHUTDOWN &&
> +        qemu_shutdown_action_is_pause()) {
> +        return;
> +    }
> +
>      new_state = migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ?
>                      VFIO_DEVICE_STATE_PRE_COPY_P2P :
>                      VFIO_DEVICE_STATE_RUNNING_P2P;
> @@ -936,6 +955,17 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>          return;
>      }
>  
> +    /*
> +     * The guest has shut down and QEMU keeps the VM stopped. It can only
> +     * be reset or quit now, so leave the device state as it is: the reset
> +     * resets the device too. The exit path uses the same run state with
> +     * the poweroff action and stops the device as before.
> +     */
> +    if (!running && state == RUN_STATE_SHUTDOWN &&
> +        qemu_shutdown_action_is_pause()) {
> +        return;
> +    }
> +
>      if (running) {
>          new_state = VFIO_DEVICE_STATE_RUNNING;
>      } else {
> diff --git a/include/system/runstate.h b/include/system/runstate.h
> index 929379adae4..29a919348da 100644
> --- a/include/system/runstate.h
> +++ b/include/system/runstate.h
> @@ -148,6 +148,7 @@ void qemu_system_vmstop_request_prepare(void);
>  bool qemu_vmstop_requested(RunState *r);
>  ShutdownCause qemu_shutdown_requested_get(void);
>  bool qemu_force_shutdown_requested(void);
> +bool qemu_shutdown_action_is_pause(void);
>  ShutdownCause qemu_reset_requested_get(void);
>  void qemu_system_killed(int signal, pid_t pid);
>  void qemu_system_reset(ShutdownCause reason);
> diff --git a/system/runstate.c b/system/runstate.c
> index 6178b0091a3..1cdecde0fcf 100644
> --- a/system/runstate.c
> +++ b/system/runstate.c
> @@ -433,6 +433,11 @@ bool qemu_force_shutdown_requested(void)
>      return force_shutdown;
>  }
>  
> +bool qemu_shutdown_action_is_pause(void)
> +{
> +    return shutdown_action == SHUTDOWN_ACTION_PAUSE;
> +}
> +
>  ShutdownCause qemu_reset_requested_get(void)
>  {
>      return reset_requested;
Reviewed-by: Denis V. Lunev <den@openvz.org>

Re: [PATCH hci-vev v2 1/1] vfio/migration: don't touch the device on guest shutdown #VSTOR-153976

[Andrey Drobyshev]

On 11/9/26 2:26 PM, Denis V. Lunev wrote:
> On 11/8/26 21:50, Andrey Drobyshev wrote:
>> When the guest shuts down and QEMU runs with -no-shutdown, the VM stays
>> stopped and can only be reset or quit. A libvirt soft reboot is exactly
>> that: guest shutdown, system_reset, cont. Today vfio_vmstate_change()
>> treats this stop like any other and moves the device to STOP, then cont
>> brings it back to RUNNING.
>>
>> NVIDIA's vGPU manager (595.71.03) takes that STOP for the start of a
>> migration, finds no guest driver, breaks its dirty page tracking and
>> never repairs it when the driver loads again. Every live migration of
>> the VM after a soft reboot then fails: "Error saving page in pipelined
>> mode".
>>
>> Do not touch the device state in this case. There is nothing to
>> quiesce, and the reset that must follow resets the device anyway. The
>> case is told apart by the shutdown action: only "pause" (-no-shutdown)
>> keeps the VM alive. quit, a signal and a plain guest shutdown use
>> "poweroff" and still stop the device on the way out, as before. If a
>> migration or savevm is started on the stopped VM, no VM state change
>> comes at switchover, so vfio_save_setup() stops the device itself.
>>
>> With this patch applied, soft reboot + subsequent live migration pass
>> successfully.
>>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>>  hw/vfio/migration.c       | 30 ++++++++++++++++++++++++++++++
>>  include/system/runstate.h |  1 +
>>  system/runstate.c         |  5 +++++
>>  3 files changed, 36 insertions(+)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 7e7fb496bbe..2ff3e49d5de 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -473,6 +473,19 @@ static int vfio_save_setup(QEMUFile *f, void *opaque, Error **errp)
>>      migration->event_save_iterate_started = false;
>>      migration->event_precopy_empty_hit = false;
>>  
>> +    /*
>> +     * A VM that is already stopped gets no VM state change at switchover.
>> +     * If a guest shutdown left the device running, stop it now.
>> +     */
>> +    if (!runstate_is_running() &&
>> +        migration->device_state == VFIO_DEVICE_STATE_RUNNING) {
>> +        ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
>> +                                       VFIO_DEVICE_STATE_RUNNING, errp);
>> +        if (ret) {
>> +            return ret;
>> +        }
>> +    }
>> +
>>      if (vfio_precopy_supported(vbasedev)) {
>>          switch (migration->device_state) {
>>          case VFIO_DEVICE_STATE_RUNNING:
>> @@ -899,6 +912,12 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>>          return;
>>      }
>>  
>> +    /* The guest has shut down and QEMU keeps the VM stopped: leave it */
>> +    if (!running && state == RUN_STATE_SHUTDOWN &&
>> +        qemu_shutdown_action_is_pause()) {
>> +        return;
>> +    }
>> +
>>      new_state = migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ?
>>                      VFIO_DEVICE_STATE_PRE_COPY_P2P :
>>                      VFIO_DEVICE_STATE_RUNNING_P2P;
>> @@ -936,6 +955,17 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>>          return;
>>      }
>>  
>> +    /*
>> +     * The guest has shut down and QEMU keeps the VM stopped. It can only
>> +     * be reset or quit now, so leave the device state as it is: the reset
>> +     * resets the device too. The exit path uses the same run state with
>> +     * the poweroff action and stops the device as before.
>> +     */
>> +    if (!running && state == RUN_STATE_SHUTDOWN &&
>> +        qemu_shutdown_action_is_pause()) {
>> +        return;
>> +    }
>> +
>>      if (running) {
>>          new_state = VFIO_DEVICE_STATE_RUNNING;
>>      } else {
>> diff --git a/include/system/runstate.h b/include/system/runstate.h
>> index 929379adae4..29a919348da 100644
>> --- a/include/system/runstate.h
>> +++ b/include/system/runstate.h
>> @@ -148,6 +148,7 @@ void qemu_system_vmstop_request_prepare(void);
>>  bool qemu_vmstop_requested(RunState *r);
>>  ShutdownCause qemu_shutdown_requested_get(void);
>>  bool qemu_force_shutdown_requested(void);
>> +bool qemu_shutdown_action_is_pause(void);
>>  ShutdownCause qemu_reset_requested_get(void);
>>  void qemu_system_killed(int signal, pid_t pid);
>>  void qemu_system_reset(ShutdownCause reason);
>> diff --git a/system/runstate.c b/system/runstate.c
>> index 6178b0091a3..1cdecde0fcf 100644
>> --- a/system/runstate.c
>> +++ b/system/runstate.c
>> @@ -433,6 +433,11 @@ bool qemu_force_shutdown_requested(void)
>>      return force_shutdown;
>>  }
>>  
>> +bool qemu_shutdown_action_is_pause(void)
>> +{
>> +    return shutdown_action == SHUTDOWN_ACTION_PAUSE;
>> +}
>> +
>>  ShutdownCause qemu_reset_requested_get(void)
>>  {
>>      return reset_requested;
> Reviewed-by: Denis V. Lunev <den@openvz.org>

This version didn't work either. I've just sent another.

Andrey