Re: [PATCH] vhost-blk: set features before setting inflight feature

[Raphael Norwitz]

I see your point - all the open source backends I could find which
support VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD rely on knowing the vq
type to allocate the fd.

That said, it looks like dpdk supports both
VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD and packed vqs without needing
such an API 
https://github.com/DPDK/dpdk/blob/main/lib/librte_vhost/vhost_user.c#L1509.
I'm not sure exactly how the VQ state is sent to DPDK before the
inflight fd negotiation, but ideally vhost-user-blk could be made to
work the same way. Maybe someone with more vhost-net and dpdk
knowledge could chime in on how vhost-net backends do it?

On Mon, Sep 14, 2020 at 10:52 PM Yu, Jin <jin.yu@intel.com> wrote:
>
> > -----Original Message-----
> > From: Raphael Norwitz <raphael.s.norwitz@gmail.com>
> > Sent: Tuesday, September 15, 2020 9:25 AM
> > To: Yu, Jin <jin.yu@intel.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>; Raphael Norwitz
> > <raphael.norwitz@nutanix.com>; Kevin Wolf <kwolf@redhat.com>; Max
> > Reitz <mreitz@redhat.com>; QEMU <qemu-devel@nongnu.org>
> > Subject: Re: [PATCH] vhost-blk: set features before setting inflight feature
> >
> > Backends already receive the format in vhost_dev_start before the memory
> > tables are set or any of the virtqueues are started. Can you elaborate on 
> > why
> > you need to know the virtqueue format before setting the inflight FD?
> >
> First, when the backend receives the get_inflight_fd sent by QEMU, it needs 
> to allocate vq's inflight memory,
> and it needs to know the format of vq.
> Second, when the backend reconnects to QEMU, QEMU sends set_inflight_fd, and 
> the backend restores the inflight memory of vq.
> It also needs to know the format of vq.
> Thanks.
> > On Thu, Sep 10, 2020 at 2:15 AM Jin Yu <jin.yu@intel.com> wrote:
> > >
> > > Virtqueue has split and packed, so before setting inflight, you need
> > > to inform the back-end virtqueue format.
> > >
> > > Signed-off-by: Jin Yu <jin.yu@intel.com>
> > > ---
> > >  hw/block/vhost-user-blk.c |  6 ++++++
> > >  hw/virtio/vhost.c         | 18 ++++++++++++++++++
> > >  include/hw/virtio/vhost.h |  1 +
> > >  3 files changed, 25 insertions(+)
> > >
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index 39aec42dae..9e0e9ebec0 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -131,6 +131,12 @@ static int vhost_user_blk_start(VirtIODevice
> > > *vdev)
> > >
> > >      s->dev.acked_features = vdev->guest_features;
> > >
> > > +    ret = vhost_dev_prepare_inflight(&s->dev);
> > > +    if (ret < 0) {
> > > +        error_report("Error set inflight format: %d", -ret);
> > > +        goto err_guest_notifiers;
> > > +    }
> > > +
> > >      if (!s->inflight->addr) {
> > >          ret = vhost_dev_get_inflight(&s->dev, s->queue_size, 
> > > s->inflight);
> > >          if (ret < 0) {
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index
> > > 1a1384e7a6..4027c11886 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -1616,6 +1616,24 @@ int vhost_dev_load_inflight(struct vhost_inflight
> > *inflight, QEMUFile *f)
> > >      return 0;
> > >  }
> > >
> > > +int vhost_dev_prepare_inflight(struct vhost_dev *hdev) {
> > > +    int r;
> > > +
> > > +    if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
> > > +        hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
> > > +        return 0;
> > > +    }
> > > +
> > > +    r = vhost_dev_set_features(hdev, hdev->log_enabled);
> > > +    if (r < 0) {
> > > +        VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
> > > +        return r;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >  int vhost_dev_set_inflight(struct vhost_dev *dev,
> > >                             struct vhost_inflight *inflight)  { diff
> > > --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index
> > > 767a95ec0b..4e2fc75528 100644
> > > --- a/include/hw/virtio/vhost.h
> > > +++ b/include/hw/virtio/vhost.h
> > > @@ -140,6 +140,7 @@ void vhost_dev_reset_inflight(struct
> > > vhost_inflight *inflight);  void vhost_dev_free_inflight(struct
> > > vhost_inflight *inflight);  void vhost_dev_save_inflight(struct
> > > vhost_inflight *inflight, QEMUFile *f);  int
> > > vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f);
> > > +int vhost_dev_prepare_inflight(struct vhost_dev *hdev);
> > >  int vhost_dev_set_inflight(struct vhost_dev *dev,
> > >                             struct vhost_inflight *inflight);  int
> > > vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
> > > --
> > > 2.17.2
> > >
> > >