Re: [Qemu-devel] [PATCH 1/1] vhost: set the correct queue index in case of migration with multiqueue

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 02:59:27PM +0200, Thibaut Collet wrote:
> When a live migration is started the log address to mark dirty pages is 
> provided
> to the vhost backend through the vhost_dev_set_log function.
> This function is called for each queue pairs but the queue index is wrongly 
> set:
> always set to the first queue pair. Then vhost backend lost descriptor 
> addresses
> of the queue pairs greater than 1 and behaviour of the vhost backend is
> unpredictable.
> 
> The queue index is computed by taking account of the vq_index (to retrieve the
> queue pair index) and calling the vhost_get_vq_index method of the backend.
> 
> Signed-off-by: Thibaut Collet <address@hidden>

Thanks!
The code in question was added by:
    commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8
    Author: Jason Wang <address@hidden>
    Date:   Wed Jan 30 19:12:35 2013 +0800

        vhost: multiqueue support

Jason, could you comment on whether this makes sense please?
If yes - this is an old bug, and we need this on stable,
do we not?

Maybe we should refactor vhost_virtqueue_set_addr to
make it call vhost_get_vq_index internally automatically.
All callers do this anyway.
This can be a patch on top.

> ---
>  hw/virtio/vhost.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index feeaaa4..de29968 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -656,13 +656,14 @@ static int vhost_dev_set_features(struct vhost_dev 
> *dev, bool enable_log)
>  
>  static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
>  {
> -    int r, t, i;
> +    int r, t, i, idx;
>      r = vhost_dev_set_features(dev, enable_log);
>      if (r < 0) {
>          goto err_features;
>      }
>      for (i = 0; i < dev->nvqs; ++i) {
> -        r = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
> +        idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
> +        r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
>                                       enable_log);
>          if (r < 0) {
>              goto err_vq;
> @@ -671,7 +672,8 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool 
> enable_log)
>      return 0;
>  err_vq:
>      for (; i >= 0; --i) {
> -        t = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
> +        idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
> +        t = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
>                                       dev->log_enabled);
>          assert(t >= 0);
>      }
> -- 
> 2.1.4