Re: [Qemu-devel] [PATCH 1/3] virtio: don't exit on guest errors

2011-03-29 Thread Amit Shah
On (Mon) 28 Mar 2011 [23:14:16], Michael S. Tsirkin wrote:
 When guest does something illegal, such as
 programming invalid index values in the virtio
 device, qemu currently tends to crash.
 
 With virtio, a better idea is to log an error,
 and set status to FAIL which stops the device.
 
 Add an API to do this, and fix core, blk and serial
 to use it on error.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/virtio-blk.c|   12 +--
  hw/virtio-serial-bus.c |   13 +--
  hw/virtio.c|   79 
 +++-
  hw/virtio.h|7 +++-
  4 files changed, 73 insertions(+), 38 deletions(-)

ACK

Amit



[Qemu-devel] [PATCH 1/3] virtio: don't exit on guest errors

2011-03-28 Thread Michael S. Tsirkin
When guest does something illegal, such as
programming invalid index values in the virtio
device, qemu currently tends to crash.

With virtio, a better idea is to log an error,
and set status to FAIL which stops the device.

Add an API to do this, and fix core, blk and serial
to use it on error.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/virtio-blk.c|   12 +--
 hw/virtio-serial-bus.c |   13 +--
 hw/virtio.c|   79 +++-
 hw/virtio.h|7 +++-
 4 files changed, 73 insertions(+), 38 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index cff21a9..fdaaf89 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -502,10 +502,14 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int 
version_id)
 req-next = s-rq;
 s-rq = req;
 
-virtqueue_map_sg(req-elem.in_sg, req-elem.in_addr,
-req-elem.in_num, 1);
-virtqueue_map_sg(req-elem.out_sg, req-elem.out_addr,
-req-elem.out_num, 0);
+if (virtqueue_map_sg(s-vq, req-elem.in_sg, req-elem.in_addr,
+ req-elem.in_num, 1)) {
+return -EINVAL;
+}
+if (virtqueue_map_sg(s-vq, req-elem.out_sg, req-elem.out_addr,
+ req-elem.out_num, 0)) {
+return -EINVAL;
+}
 }
 
 return 0;
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7ae2b0d..8807a2f 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -679,10 +679,15 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
int version_id)
 
 qemu_get_buffer(f, (unsigned char *)port-elem,
 sizeof(port-elem));
-virtqueue_map_sg(port-elem.in_sg, port-elem.in_addr,
- port-elem.in_num, 1);
-virtqueue_map_sg(port-elem.out_sg, port-elem.out_addr,
- port-elem.out_num, 1);
+if (virtqueue_map_sg(port-ivq, port-elem.in_sg,
+ port-elem.in_addr,
+ port-elem.in_num, 1)) {
+return -EINVAL;
+}
+if (virtqueue_map_sg(port-ovq, port-elem.out_sg, 
port-elem.out_addr,
+ port-elem.out_num, 1)) {
+return -EINVAL;
+}
 
 /*
  *  Port was throttled on source machine.  Let's
diff --git a/hw/virtio.c b/hw/virtio.c
index d5013b6..d1c8769 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -16,6 +16,7 @@
 #include trace.h
 #include virtio.h
 #include sysemu.h
+#include qemu-error.h
 
 /* The alignment to use between consumer and producer parts of vring.
  * x86 pagesize again. */
@@ -253,15 +254,15 @@ static int virtqueue_num_heads(VirtQueue *vq, unsigned 
int idx)
 
 /* Check it isn't doing very strange things with descriptor numbers. */
 if (num_heads  vq-vring.num) {
-fprintf(stderr, Guest moved used index from %u to %u,
-idx, vring_avail_idx(vq));
-exit(1);
+virtio_error(vq-vdev, Guest moved used index from %u to %u,
+ idx, vring_avail_idx(vq));
+return 0;
 }
 
 return num_heads;
 }
 
-static unsigned int virtqueue_get_head(VirtQueue *vq, unsigned int idx)
+static int virtqueue_get_head(VirtQueue *vq, unsigned int idx)
 {
 unsigned int head;
 
@@ -271,14 +272,14 @@ 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) {
-fprintf(stderr, Guest says index %u is available, head);
-exit(1);
+virtio_error(vq-vdev, Guest says index %u is available, head);
+return -1;
 }
 
 return head;
 }
 
-static unsigned virtqueue_next_desc(target_phys_addr_t desc_pa,
+static unsigned virtqueue_next_desc(VirtQueue *vq, target_phys_addr_t desc_pa,
 unsigned int i, unsigned int max)
 {
 unsigned int next;
@@ -293,8 +294,8 @@ static unsigned virtqueue_next_desc(target_phys_addr_t 
desc_pa,
 wmb();
 
 if (next = max) {
-fprintf(stderr, Desc next is %u, next);
-exit(1);
+virtio_error(vq-vdev, Desc next is %u, next);
+return max;
 }
 
 return next;
@@ -316,18 +317,21 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, 
int out_bytes)
 max = vq-vring.num;
 num_bufs = total_bufs;
 i = virtqueue_get_head(vq, idx++);
+if (i  0) {
+return 0;
+}
 desc_pa = vq-vring.desc;
 
 if (vring_desc_flags(desc_pa, i)  VRING_DESC_F_INDIRECT) {
 if (vring_desc_len(desc_pa, i) % sizeof(VRingDesc)) {
-fprintf(stderr, Invalid size for indirect buffer table\n);
-exit(1);
+