[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] vhost: set the correct queue index in case
From: |
Thibaut Collet |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] vhost: set the correct queue index in case of migration with multiqueue |
Date: |
Mon, 19 Oct 2015 18:41:38 +0200 |
On Mon, Oct 19, 2015 at 5:41 PM, Michael S. Tsirkin <address@hidden> 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 <address@hidden>
>
> 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