From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40080) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZojgQ-0002n2-7m for qemu-devel@nongnu.org; Tue, 20 Oct 2015 23:03:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZojgN-0003It-26 for qemu-devel@nongnu.org; Tue, 20 Oct 2015 23:03:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZojgM-0003I6-QG for qemu-devel@nongnu.org; Tue, 20 Oct 2015 23:03:18 -0400 References: <1445259567-28516-1-git-send-email-thibaut.collet@6wind.com> <1445259567-28516-2-git-send-email-thibaut.collet@6wind.com> <20151020163155-mutt-send-email-mst@redhat.com> From: Jason Wang Message-ID: <5627006E.6000601@redhat.com> Date: Wed, 21 Oct 2015 11:03:10 +0800 MIME-Version: 1.0 In-Reply-To: <20151020163155-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] vhost: set the correct queue index in case of migration with multiqueue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Thibaut Collet Cc: pbonzini@redhat.com, marcandre.lureau@gmail.com, qemu-devel@nongnu.org, haifeng.lin@huawei.com On 10/20/2015 09:33 PM, Michael S. Tsirkin 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 > Thanks! > The code in question was added by: > commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 > Author: Jason Wang > Date: Wed Jan 30 19:12:35 2013 +0800 > > vhost: multiqueue support > > Jason, could you comment on whether this makes sense please? It makes sense. > If yes - this is an old bug, and we need this on stable, > do we not? But not an old bug, only vhost-user has this issue. So no need for stable. > > 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. Yes, this looks cleaner. > >> --- >> 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