[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 10/20] Gracefully handle ioctl failure in vho
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v9 10/20] Gracefully handle ioctl failure in vhost_virtqueue_stop |
Date: |
Wed, 5 Mar 2014 15:47:31 +0200 |
On Wed, Mar 05, 2014 at 02:38:34PM +0100, Antonios Motakis wrote:
> Hello,
>
>
> On Tue, Mar 4, 2014 at 7:45 PM, Michael S. Tsirkin <address@hidden> wrote:
>
> 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.
>
>
>
> Reconnect would be a really useful feature for us, so we tried to keep it in a
> reasonable way.
>
> However we didn't take into account that some guests might crash under those
> assumptions. Looks like we have no option but to remove reconnect altogether
> for now; maybe a future extension to the virtio-net spec will allow us to do
> it
> cleanly, but I don't see an obvious workaround to keep this in now.
>
> Thanks for pointing this out.
>
> Btw, since it looks like we are closing a final version of the patches, what
> kind of timeframe should we aim for inclusion? Should we already rebase on top
> of Paolo's NUMA patch series?
>
I think it should be possible to merge after Paolo's
patches go in, yes. I haven't followed them closely
so I don't know when will that be.
I wish someone else would ack the char dev changes - anyone?
But it's not a blocker requirement.
> >
> > Signed-off-by: Antonios Motakis <address@hidden>
> > Signed-off-by: Nikolay Nikolaev <address@hidden>
>
> 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
>
>
>
>
> --
> Antonios Motakis
> Virtual Open Systems
- [Qemu-devel] [PATCH v9 07/20] vhost_net should call the poll callback only when it is set, (continued)
- [Qemu-devel] [PATCH v9 07/20] vhost_net should call the poll callback only when it is set, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 01/20] Convert -mem-path to QemuOpts and add share property, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 06/20] Add G_IO_HUP handler for socket chardev, Antonios Motakis, 2014/03/04
- Re: [Qemu-devel] [PATCH v9 00/20] Vhost and vhost-net support for userspace based backends, Paolo Bonzini, 2014/03/04
- [Qemu-devel] [PATCH v9 12/20] Add vhost_ops to vhost_dev struct and replace all relevant ioctls, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 10/20] Gracefully handle ioctl failure in vhost_virtqueue_stop, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 05/20] Add chardev API qemu_chr_fe_get_msgfds, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 03/20] Add chardev API qemu_chr_fe_read_all, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 17/20] Add the vhost-user netdev backend to the command line, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 19/20] libqemustub: add stubs to be able to use qemu-char.c, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 20/20] Add qtest for vhost-user, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 02/20] Add kvm_eventfds_enabled function, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 09/20] Add new virtio API virtio_queue_get_avail_idx, Antonios Motakis, 2014/03/04
- [Qemu-devel] [PATCH v9 16/20] Add new vhost-user netdev backend, Antonios Motakis, 2014/03/04