From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57465) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKuLE-0004Bk-5E for qemu-devel@nongnu.org; Tue, 04 Mar 2014 13:45:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKuL5-0003lt-Fn for qemu-devel@nongnu.org; Tue, 04 Mar 2014 13:45:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:61985) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKuL5-0003lU-1A for qemu-devel@nongnu.org; Tue, 04 Mar 2014 13:45:15 -0500 Date: Tue, 4 Mar 2014 20:45:07 +0200 From: "Michael S. Tsirkin" Message-ID: <20140304184507.GC21171@redhat.com> References: <1393957383-16685-1-git-send-email-a.motakis@virtualopensystems.com> <1393957383-16685-11-git-send-email-a.motakis@virtualopensystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1393957383-16685-11-git-send-email-a.motakis@virtualopensystems.com> Subject: Re: [Qemu-devel] [PATCH v9 10/20] Gracefully handle ioctl failure in vhost_virtqueue_stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Antonios Motakis Cc: lukego@gmail.com, snabb-devel@googlegroups.com, n.nikolaev@virtualopensystems.com, qemu-devel@nongnu.org, tech@virtualopensystems.com On Tue, Mar 04, 2014 at 07:22:53PM +0100, Antonios Motakis wrote: > On stopping the vhost, a call to VHOST_GET_VRING_BASE is issued. The > received value is stored as last_avail_idx, so the virtqueue can continue > operating if the connection is resumed. Handle the failure of this call > and use the current avail_idx. Some packets from the avail ring may be > omitted but still we keep a sane value and can continue on reconnect. omitted how? some guests crash if we never complete handling buffers, or networking breaks, etc ... This would be a big problem for reconnect, some robust way to communicate avail ring state would need to be found. Is reconnect really a mandatory feature for you? I'd suggest you drop it from v1, focus on basic functionality. > > Signed-off-by: Antonios Motakis > Signed-off-by: Nikolay Nikolaev Problem is, a bunch of stuff breaks if vhost keeps going when we ask it to stop. In particular it will keep looking at the ring state when guest asked it to stop doing this, this will corrupt guest memory. > --- > hw/virtio/vhost.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 9e336ad..322e2c0 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -758,12 +758,13 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev, > assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs); > r = ioctl(dev->control, VHOST_GET_VRING_BASE, &state); > if (r < 0) { > + state.num = virtio_queue_get_avail_idx(vdev, idx); > fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r); > fflush(stderr); > } > virtio_queue_set_last_avail_idx(vdev, idx, state.num); > virtio_queue_invalidate_signalled_used(vdev, idx); > - assert (r >= 0); > + > cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx), > 0, virtio_queue_get_ring_size(vdev, idx)); > cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vdev, idx), > -- > 1.8.3.2