On Mon, Oct 19, 2015 at 5:41 PM, Michael S. Tsirkin <m...@redhat.com> wrote: > 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 <thibaut.col...@6wind.com> > > This needs some thought to make sure we don't break the kernel vhost.
For kernel vhost my patch does nothing has vhost_get_vq_index method for vhost kernel subtract dev->vq_index (that was just added before) and idx is still equal to i. > > I queued this temporarily to enable your testing but I think it would be > preferable to make vhost_virtqueue_set_addr for vhost_user call > vhost_get_vq_index internally. > If I call the vhost_get_vq_index internally by vhost_user when vhost_virtqueue_set_addr is called I will break the vhost_virtqueue_start: this function calls the vhost_get_vq_index function for vhost user and vhost kernel to initializes the queue. > > >> --- >> 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