Re: [PATCH hci-dev v6 2/2] vfio/migration: don't stop the device on guest shutdown #VSTOR-153976

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

On 11/9/26 22:57, no-reply@sashiko.virtuozzo.com wrote:
> [Severity: Low]
> The predicate tests only !running, state == RUN_STATE_SHUTDOWN and
> qemu_shutdown_action_is_pause(). It does not look at what caused the stop.
> [...]
> Under -no-shutdown, these host-originated stops would also skip the device
> STOP, even though the guest driver is not necessarily gone.

Yes, and that is fine. Every stop with the "pause" action ends the same
way: the VM can only be reset, quit or migrated, and each of those
resets or stops the device. Until then the vCPUs are stopped and nothing
new is asked of the device. Whether the guest driver is still loaded
makes no difference to that, so the cause is not needed.

> [Severity: Medium]
> This early return leaves the device in RUNNING or RUNNING_P2P while the VM
> sits in RUN_STATE_SHUTDOWN. The commit message and the new comment name only
> reset, quit and migration as the follow-up paths that stop the device.
>
> What about device removal?
> [...]
> Before this patch the device was always in STOP by the time it was
> unrealized. Can the device now be closed while still in RUNNING or
> RUNNING_P2P if it is unplugged after a guest shutdown, with no reset, quit or
> save in between?

A hot-unplug needs the guest to eject the device, through the attention
button on PCIe or the ACPI notification on legacy PCI. The guest is
halted, so the request stays pending until the reset and cont, and then
the device is handled as on any running VM. The one shortcut that skips
the guest, a slot the guest had already powered off, means the guest had
already released that device. And that shortcut closes a running device
on any running VM today, that is how every VFIO hot-unplug works. So
nothing new is reachable here.

> [Severity: Low]
> This skip covers only the final STOP. vfio_vmstate_change_prepare() is not
> gated by the new check:
> [...]
> RUNNING_P2P is a migration-flow state. Could a vendor manager that treats
> migration-flow transitions as the start of a migration still be confused by
> it?
>
> The commit message says the P2P round is deliberate, but it does not say
> whether the NVIDIA vGPU manager tolerates RUNNING_P2P. It also does not say
> whether the testing was done on a device with P2P support.

It does. The NVIDIA vGPU VF driver takes RUNNING to RUNNING_P2P and back
without telling its manager, that transition is a no-op in its state
table. All testing was done on that device, which supports P2P, so the
P2P round ran at every guest shutdown: 16 soft reboots followed by a
live migration, and four runs of the vGPU test suite with 29 migrations
each, all passed. I will say so in the commit message.