From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57271) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSTKg-00023X-Tk for qemu-devel@nongnu.org; Wed, 27 Jul 2016 14:13:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSTKa-0001Jz-KS for qemu-devel@nongnu.org; Wed, 27 Jul 2016 14:13:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35938) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSTKa-0001Jv-C3 for qemu-devel@nongnu.org; Wed, 27 Jul 2016 14:13:20 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 13F1B285A4 for ; Wed, 27 Jul 2016 18:13:20 +0000 (UTC) Date: Wed, 27 Jul 2016 21:13:18 +0300 From: "Michael S. Tsirkin" Message-ID: <20160727211131-mutt-send-email-mst@kernel.org> References: <1467102269-11112-1-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467102269-11112-1-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio: abort on fatal error instead of just exiting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org On Tue, Jun 28, 2016 at 10:24:29AM +0200, Igor Mammedov wrote: > replace mainly useless exit(1) on fatal error path with > abort(), so that it would be possible to generate core > dump, that could be used to analyse cause of problem. > > Signed-off-by: Igor Mammedov I thought about this, and I'd rather we made the jump to stopping the device straight away. Let's leave this alone and focus on the real thing post 2.7. E.g. creating coredumps from guests could cause some disk to fill up. I'd rather not go there. > --- > hw/virtio/virtio.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 7ed06ea..9d3ac72 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -315,7 +315,7 @@ static int virtqueue_num_heads(VirtQueue *vq, unsigned int idx) > if (num_heads > vq->vring.num) { > error_report("Guest moved used index from %u to %u", > idx, vq->shadow_avail_idx); > - exit(1); > + abort(); > } > /* On success, callers read a descriptor at vq->last_avail_idx. > * Make sure descriptor read does not bypass avail index read. */ > @@ -337,7 +337,7 @@ static unsigned int virtqueue_get_head(VirtQueue *vq, unsigned int idx) > /* If their number is silly, that's a fatal mistake. */ > if (head >= vq->vring.num) { > error_report("Guest says index %u is available", head); > - exit(1); > + abort(); > } > > return head; > @@ -360,7 +360,7 @@ static unsigned virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc, > > if (next >= max) { > error_report("Desc next is %u", next); > - exit(1); > + abort(); > } > > vring_desc_read(vdev, desc, desc_pa, next); > @@ -393,13 +393,13 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes, > if (desc.flags & VRING_DESC_F_INDIRECT) { > if (desc.len % sizeof(VRingDesc)) { > error_report("Invalid size for indirect buffer table"); > - exit(1); > + abort(); > } > > /* If we've got too many, that implies a descriptor loop. */ > if (num_bufs >= max) { > error_report("Looped descriptor"); > - exit(1); > + abort(); > } > > /* loop over the indirect descriptor table */ > @@ -414,7 +414,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes, > /* If we've got too many, that implies a descriptor loop. */ > if (++num_bufs > max) { > error_report("Looped descriptor"); > - exit(1); > + abort(); > } > > if (desc.flags & VRING_DESC_F_WRITE) { > @@ -462,7 +462,7 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct iove > > if (num_sg == max_num_sg) { > error_report("virtio: too many write descriptors in indirect table"); > - exit(1); > + abort(); > } > > iov[num_sg].iov_base = cpu_physical_memory_map(pa, &len, is_write); > @@ -500,11 +500,11 @@ static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr, > sg[i].iov_base = cpu_physical_memory_map(addr[i], &len, is_write); > if (!sg[i].iov_base) { > error_report("virtio: error trying to map MMIO memory"); > - exit(1); > + abort(); > } > if (len != sg[i].iov_len) { > error_report("virtio: unexpected memory split"); > - exit(1); > + abort(); > } > } > } > @@ -570,7 +570,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) > if (desc.flags & VRING_DESC_F_INDIRECT) { > if (desc.len % sizeof(VRingDesc)) { > error_report("Invalid size for indirect buffer table"); > - exit(1); > + abort(); > } > > /* loop over the indirect descriptor table */ > @@ -588,7 +588,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) > } else { > if (in_num) { > error_report("Incorrect order for descriptors"); > - exit(1); > + abort(); > } > virtqueue_map_desc(&out_num, addr, iov, > VIRTQUEUE_MAX_SIZE, false, desc.addr, desc.len); > @@ -597,7 +597,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) > /* If we've got too many, that implies a descriptor loop. */ > if ((in_num + out_num) > max) { > error_report("Looped descriptor"); > - exit(1); > + abort(); > } > } while ((i = virtqueue_read_next_desc(vdev, &desc, desc_pa, max)) != max); > > -- > 1.8.3.1