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

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

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