On Mon, 19 Sep 2016 21:27:45 +0200
Laszlo Ersek <ler...@redhat.com> wrote:
> On 09/19/16 19:51, Michael S. Tsirkin wrote:
> > On Mon, Sep 19, 2016 at 06:07:40PM +0200, Cornelia Huck wrote:
> >> On Tue, 12 Apr 2016 14:25:24 +0100
> >> Stefan Hajnoczi <stefa...@redhat.com> wrote:
> >>> v3:
> >>> * Patch 1: Fix typo and clarify commit description [Markus]
> >>> * Use virtio_set_status() instead of open coding assignment [Cornelia]
> >>> * Add live migration
> >>> v2:
> >>> * Add VIRTIO_CONFIG_S_NEEDS_RESET notification for VIRTIO 1.0 [Cornelia]
> >>> (Note I've sent a Linux virtio_config.h patch to get the constant
> >>> added to
> >>> the headers.)
> >>> * Split int -> unsigned int change into separate commit [Fam]
> >>> * Fix double "index" typo in commit description [Fam]
> >>> The virtio code calls exit() when the device enters an invalid state.
> >>> This
> >>> means invalid vring indices and descriptor chains kill the VM. See the
> >>> patch
> >>> descriptions for why this is a bad thing.
> >>> When the virtio device is in the broken state calls to virtqueue_pop() and
> >>> friends will pretend the virtqueue is empty. This means the device will
> >>> become
> >>> isolated from guest activity until it is reset again.
> >>> RFC because two things are missing:
> >>> 1. Live migration support (subsection for broken flag?)
> >>> 2. Auditing devices and replacing exit() calls there too
> >>> Stefan Hajnoczi (10):
> >>> virtio: fix stray tab character
> >>> include: update virtio_config.h Linux header
> >>> virtio: stop virtqueue processing if device is broken
> >>> virtio: migrate vdev->broken flag
> >>> virtio: handle virtqueue_map_desc() errors
> >>> virtio: handle virtqueue_get_avail_bytes() errors
> >>> virtio: use unsigned int for virtqueue_get_avail_bytes() index
> >>> virtio: handle virtqueue_read_next_desc() errors
> >>> virtio: handle virtqueue_num_heads() errors
> >>> virtio: handle virtqueue_get_head() errors
> >>> hw/virtio/virtio.c | 223
> >>> +++++++++++++++++++------
> >>> include/hw/virtio/virtio.h | 3 +
> >>> include/standard-headers/linux/virtio_config.h | 2 +
> >>> 3 files changed, 181 insertions(+), 47 deletions(-)
> >> As the exit-in-virtio question has popped up several times in the
> >> recent past: I think we should go forward with this series, even if we
> >> still need to look at the individual devices. Do you have a version
> >> that fits on current master?
> > I agree.
> NB, Prasad just posted a patch (v3 being the latest) that adds another
> such exit(1), at my suggestion.
> [Qemu-devel] [PATCH v3] virtio: add check for descriptor's mapped
> So a rebase of this series should likely consider that patch as well.
> (But Stefan is aware anyway.)
Stefan's series still applies on the current head, except the virtio_config.h
patch which isn't needed anymore.
And indeed there are a bunch of places where QEMU exits:
[greg@bahia qemu-virtio]$ git grep 'exit(1)' hw/virtio hw/*/virtio*
And also even more places with assert() or BUG_ON(), some of which are
guest errors actually.
For example, in virtio-9p, we have:
static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
len = iov_to_buf(elem->out_sg, elem->out_num, 0,
&out, sizeof out);
BUG_ON(len != sizeof out);
The condition may only be true if the guest sent less than the expected
9P message header which is 7-byte long.
I have a patch for this based on Stefan's series BTW.