Virtualization - Feb 2023 - [PATCH v2 11/13] vdpa: block migration if dev does not have _F_SUSPEND

? 2023/2/22 22:25, Eugenio Perez Martin ??:
> On Wed, Feb 22, 2023 at 5:05 AM Jason Wang <jasowang at redhat.com>
wrote:
>>
>> ? 2023/2/8 17:42, Eugenio P?rez ??:
>>> Next patches enable devices to be migrated even if vdpa netdev has
not
>>> been started with x-svq. However, not all devices are migratable,
so we
>>> need to block migration if we detect that.
>>>
>>> Block vhost-vdpa device migration if it does not offer _F_SUSPEND
and it
>>> has not been started with x-svq.
>>>
>>> Signed-off-by: Eugenio P?rez <eperezma at redhat.com>
>>> ---
>>>    hw/virtio/vhost-vdpa.c | 21 +++++++++++++++++++++
>>>    1 file changed, 21 insertions(+)
>>>
>>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>>> index 84a6b9690b..9d30cf9b3c 100644
>>> --- a/hw/virtio/vhost-vdpa.c
>>> +++ b/hw/virtio/vhost-vdpa.c
>>> @@ -442,6 +442,27 @@ static int vhost_vdpa_init(struct vhost_dev
*dev, void *opaque, Error **errp)
>>>            return 0;
>>>        }
>>>
>>> +    /*
>>> +     * If dev->shadow_vqs_enabled at initialization that means
the device has
>>> +     * been started with x-svq=on, so don't block migration
>>> +     */
>>> +    if (dev->migration_blocker == NULL &&
!v->shadow_vqs_enabled) {
>>> +        uint64_t backend_features;
>>> +
>>> +        /* We don't have dev->backend_features yet */
>>> +        ret = vhost_vdpa_call(dev, VHOST_GET_BACKEND_FEATURES,
>>> +                              &backend_features);
>>> +        if (unlikely(ret)) {
>>> +            error_setg_errno(errp, -ret, "Could not get
backend features");
>>> +            return ret;
>>> +        }
>>> +
>>> +        if (!(backend_features &
BIT_ULL(VHOST_BACKEND_F_SUSPEND))) {
>>> +            error_setg(&dev->migration_blocker,
>>> +                "vhost-vdpa backend lacks
VHOST_BACKEND_F_SUSPEND feature.");
>>> +        }
>>
>> I wonder why not let the device to decide? For networking device, we
can
>> live without suspend probably.
>>
> Right, but how can we know if this is a net device in init? I don't
> think a switch (vhost_vdpa_get_device_id(dev)) is elegant.

I meant the caller of vhost_vdpa_init() which is net_init_vhost_vdpa().

Thanks

>
> If the parent device does not need to be suspended i'd go with
> exposing a suspend ioctl but do nothing in the parent device. After
> that, it could even choose to return an error for GET_VRING_BASE.
>
> If we want to implement it as a fallback in qemu, I'd go for
> implementing it on top of this series. There are a few operations we
> could move to a device-kind specific ops.
>
> Would it make sense to you?
>
> Thanks!
>
>
>> Thanks
>>
>>
>>> +    }
>>> +
>>>        /*
>>>         * Similar to VFIO, we end up pinning all guest memory and
have to
>>>         * disable discarding of RAM.