Hello,
On Tue, Mar 4, 2014 at 7:45 PM, Michael S. Tsirkin <m...@redhat.com> 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?
>
> > Signed-off-by: Antonios Motakis <a.mota...@virtualopensystems.com>
> > Signed-off-by: Nikolay Nikolaev <n.nikol...@virtualopensystems.com>
>
> 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
