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@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.");
+ }
+ }
+
/*
* Similar to VFIO, we end up pinning all guest memory and have to
* disable discarding of RAM.
--
2.31.1
在 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@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.
Thanks
> + }
> +
> /*
> * Similar to VFIO, we end up pinning all guest memory and have to
> * disable discarding of RAM.
On Wed, Feb 22, 2023 at 5:05 AM Jason Wang <jasowang@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@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.
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.
>
在 2023/2/22 22:25, Eugenio Perez Martin 写道:
> On Wed, Feb 22, 2023 at 5:05 AM Jason Wang <jasowang@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@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.
On Thu, Feb 23, 2023 at 3:38 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2023/2/22 22:25, Eugenio Perez Martin 写道:
> > On Wed, Feb 22, 2023 at 5:05 AM Jason Wang <jasowang@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@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().
>
That's doable but I'm not sure if it is convenient.
Since we're always offering _F_LOG I thought of the lack of _F_SUSPEND
as the default migration blocker for other kinds of devices like blk.
If we move this code to net_init_vhost_vdpa, all other devices are in
charge of block migration by themselves.
I guess the right action is to use a variable similar to
vhost_vdpa->f_log_all. It defaults to false, and the device can choose
if it should export it or not. This way, the device does not migrate
by default, and the equivalent of net_init_vhost_vdpa could choose
whether to offer _F_LOG with SVQ or not.
OTOH I guess other kinds of devices already must place blockers beyond
_F_LOG, so maybe it makes sense to always offer _F_LOG even if
_F_SUSPEND is not offered? Stefano G., would that break vhost-vdpa-blk
support?
Thanks!
> 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.
>
On Thu, Feb 23, 2023 at 7:07 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Thu, Feb 23, 2023 at 3:38 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > 在 2023/2/22 22:25, Eugenio Perez Martin 写道:
> > > On Wed, Feb 22, 2023 at 5:05 AM Jason Wang <jasowang@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@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().
> >
>
> That's doable but I'm not sure if it is convenient.
So it's a question whether or not we try to let migration work without
suspending. If we don't, there's no need to bother. Looking at the
current vhost-net implementation, it tries to make migration work upon
the error of get_vring_base() so maybe it's worth a try if it doesn't
bother too much. But I'm fine to go either way.
>
> Since we're always offering _F_LOG I thought of the lack of _F_SUSPEND
> as the default migration blocker for other kinds of devices like blk.
Or we can have this by default and allow a specific type of device to clear?
> If we move this code to net_init_vhost_vdpa, all other devices are in
> charge of block migration by themselves.
>
> I guess the right action is to use a variable similar to
> vhost_vdpa->f_log_all. It defaults to false, and the device can choose
> if it should export it or not. This way, the device does not migrate
> by default, and the equivalent of net_init_vhost_vdpa could choose
> whether to offer _F_LOG with SVQ or not.
Looks similar to what I think above.
>
> OTOH I guess other kinds of devices already must place blockers beyond
> _F_LOG, so maybe it makes sense to always offer _F_LOG even if
> _F_SUSPEND is not offered?
I don't see any dependency between the two features. Technically,
there could be devices that have neither _F_LOG nor _F_SUSPEND.
Thanks
> Stefano G., would that break vhost-vdpa-blk
> support?
>
> Thanks!
>
> > 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.
> >
>
© 2016 - 2026 Red Hat, Inc.