On 6/23/26 9:27 PM, Maciej S. Szmigiero wrote:
> On 19.06.2026 12:55, Andrey Drobyshev wrote:
>> When preserving a vhost fd using CPR, call VHOST_RESET_OWNER prior to CPR
>> in old QEMU. Otherwise, new QEMU will fail when it calls VHOST_SET_OWNER
>> during vhost_dev_init.
>>
>> This patch is a rework of the change originally proposed by Steve, Ben
>> and Mark at [0].
>>
>> [0]
>> https://lore.kernel.org/qemu-devel/[email protected]
>>
>> Originally-by: Mark Kanda <[email protected]>
>> Originally-by: Steve Sistare <[email protected]>
>> Originally-by: Ben Chaney <[email protected]>
>> Signed-off-by: Andrey Drobyshev <[email protected]>
>> ---
>> hw/virtio/vhost-kernel.c | 6 ++++++
>> hw/virtio/vhost.c | 33 +++++++++++++++++++++++++++++++
>> include/hw/virtio/vhost-backend.h | 1 +
>> include/hw/virtio/vhost.h | 1 +
>> 4 files changed, 41 insertions(+)
>>
>> diff --git a/hw/virtio/vhost-kernel.c b/hw/virtio/vhost-kernel.c
>> index 3390b48c6f1..55d316b88e9 100644
>> --- a/hw/virtio/vhost-kernel.c
>> +++ b/hw/virtio/vhost-kernel.c
>> @@ -261,6 +261,11 @@ static int vhost_kernel_set_owner(struct vhost_dev *dev)
>> return vhost_kernel_call(dev, VHOST_SET_OWNER, NULL);
>> }
>>
>> +static int vhost_kernel_reset_owner(struct vhost_dev *dev)
>> +{
>> + return vhost_kernel_call(dev, VHOST_RESET_OWNER, NULL);
>> +}
>> +
>> static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
>> {
>> assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
>> @@ -385,6 +390,7 @@ const VhostOps kernel_ops = {
>> .vhost_get_features_ex = vhost_kernel_get_features,
>> .vhost_set_backend_cap = vhost_kernel_set_backend_cap,
>> .vhost_set_owner = vhost_kernel_set_owner,
>> + .vhost_reset_owner = vhost_kernel_reset_owner,
>> .vhost_get_vq_index = vhost_kernel_get_vq_index,
>> .vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
>> .vhost_vsock_set_running = vhost_kernel_vsock_set_running,
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index af41841b529..1521b50cbff 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -24,6 +24,7 @@
>> #include "standard-headers/linux/vhost_types.h"
>> #include "hw/virtio/virtio-bus.h"
>> #include "hw/mem/memory-device.h"
>> +#include "migration/misc.h"
>> #include "migration/blocker.h"
>> #include "migration/qemu-file-types.h"
>> #include "system/dma.h"
>> @@ -1667,6 +1668,32 @@ static int vhost_dev_init_features(struct vhost_dev
>> *hdev)
>> return r;
>> }
>>
>> +static int vhost_cpr_notifier(NotifierWithReturn *notifier,
>> + MigrationEvent *e, Error **errp)
>> +{
>> + struct vhost_dev *dev;
>> + int r;
>> +
>> + dev = container_of(notifier, struct vhost_dev, cpr_notifier);
>> +
>> + if (dev->vhost_ops->backend_type != VHOST_BACKEND_TYPE_KERNEL) {
>> + return 0;
>> + }
>> +
>> + if (e->type == MIG_EVENT_SETUP) {
>> + r = dev->vhost_ops->vhost_reset_owner(dev);
>> + if (r < 0) {
>> + VHOST_OPS_DEBUG(r, "vhost_reset_owner failed");
>
> Shouldn't we abort the migration attempt here?
>
Hmm yes, I think you're right. We shouldn't proceed and should do
error_setg_errno() and return r < 0 instead.
>> + }
>> + } else if (e->type == MIG_EVENT_FAILED) {
>> + r = dev->vhost_ops->vhost_set_owner(dev);
>> + if (r < 0) {
>> + VHOST_OPS_DEBUG(r, "vhost_set_owner failed");
>
> What would be the impact on the guest of this failure?
>
> Shouldn't we *at least* unconditionally print error here?
> Maybe even more if guest would be unsafe in this condition?
Agreed. Should report error as well. The only catch is we might also
get into this branch after unsuccessful qmp_migrate() -> SETUP ->
vhost_reset_owner() -> migration_connect_error_propagate() -> FAILED.
Then we'll attempt set_owner for a device whose owner we didn't reset.
So we should probably add some bool flag which would signalize whether
device has its owner currently set or not, and only attempt set_owner if
reset_owner took place.
But you got me doubtful about what can happen to the guest if on FAILED
we couldn't restore owner. Realistically enough, we can at least get
-ENOMEM during high memory pressure, which is likely to leave vsock dead.
WDYT?
>> + }
>> + }
>> + return 0;
>> +}
>> +
> Thanks,
> Maciej
>
