On Tue, Feb 13, 2024 at 8:49 AM Eugenio Perez Martin <epere...@redhat.com> wrote: > > On Mon, Feb 12, 2024 at 9:20 AM Michael S. Tsirkin <m...@redhat.com> wrote: > > > > On Fri, Feb 09, 2024 at 02:29:59PM -0800, Steve Sistare wrote: > > > Calling suspend or resume requires VIRTIO_CONFIG_S_DRIVER_OK, for all > > > vdpa devices. > > > > > > Suggested-by: Eugenio Perez Martin <epere...@redhat.com>" > > > Signed-off-by: Steve Sistare <steven.sist...@oracle.com> > > > > I don't think failing suspend or resume makes sense though - > > e.g. practically failing suspend will just prevent sleeping I think - > > why should guest not having driver loaded prevent > > system suspend? > > > > In the QEMU case the vhost device has not started, so QEMU should > allow the system suspension. > > I haven't tested the QEMU behavior on suspend (not poweroff) with the > guest driver loaded, but I think QEMU should indeed block the > suspension, as there is no way to recover the device after that > without the guest cooperation? > > > there's also state such as features set which does need to be > > preserved. > > > > That's true if the device does not support resuming. Well, in the > particular case of features, maybe we need to keep it, as userspace > could call VHOST_GET_FEATURES. But maybe we can clean some things, > right. > > > I think the thing to do is to skip invoking suspend/resume callback, and in > > fact checking suspend/resume altogether. > > > > I don't follow this. What should be done in this cases by QEMU? > 1) The device does not support suspend > 2) The device support suspend but not resume > > In my opinion 1) should be forbidden, as we don't support to resume > the device properly, and 2) can be allowed by fetching all the state. >
Ok I missed the whole other thread, everything is clear now. Thanks! > Thanks! > > > > --- > > > drivers/vhost/vdpa.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > > index bc4a51e4638b..ce1882acfc3b 100644 > > > --- a/drivers/vhost/vdpa.c > > > +++ b/drivers/vhost/vdpa.c > > > @@ -598,6 +598,9 @@ static long vhost_vdpa_suspend(struct vhost_vdpa *v) > > > if (!ops->suspend) > > > return -EOPNOTSUPP; > > > > > > + if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK)) > > > + return -EINVAL; > > > + > > > ret = ops->suspend(vdpa); > > > if (!ret) > > > v->suspended = true; > > > @@ -618,6 +621,9 @@ static long vhost_vdpa_resume(struct vhost_vdpa *v) > > > if (!ops->resume) > > > return -EOPNOTSUPP; > > > > > > + if (!(ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK)) > > > + return -EINVAL; > > > + > > > ret = ops->resume(vdpa); > > > if (!ret) > > > v->suspended = false; > > > -- > > > 2.39.3 > > > >
