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

[Yu, Jin]

> -----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
> >
> >