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

[Raphael Norwitz <raphael.s.norwitz@gmail.com>]

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?

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