Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option

[Ian Campbell]

On Wed, 2011-05-25 at 10:23 +0200, Jan Kiszka wrote:
> On 2011-05-25 10:13, Ian Campbell wrote:
> > On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote:
> >> On 2011-05-24 18:06, Ian Campbell wrote:
> >> Well, first of all I think this revealed a Xen bug because it crashes
> >> when you try to run xenfv with an inappropriate accelerator, no? What is
> >> the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv
> >> -machine accel=tcg?
> > 
> > Unsurprisingly it crashed...
> > 
> > I'm not sure if this is a bug in the xen side of simply a case of
> > providing enough rope. I didn't really follow the threads which resulted
> > in the accel stuff all that closely so I'm not sure what the intention
> > was, it seems to me that the accel option is invalid with certain
> > machine types though.
> > 
> > I suppose in theory -machine xenpv,accel=kvm might result in xenner or
> > something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?).
> > For -machine xenfv I don't expect anything other than accel=xen makes
> > much sense.
> 
> The point is that crashing is always a very poor way of reporting a
> misconfiguration to the user. I bet you are able to detect and report
> that more gracefully, e.g. during xenfv machine init.

ACK. How about:

8<------------------------------------

>From b23cadda8da72da480fe7cc8e31ffccd449f4b47 Mon Sep 17 00:00:00 2001
From: Ian Campbell <address@hidden>
Date: Wed, 25 May 2011 09:32:07 +0100
Subject: [PATCH] Fail to initialise Xen HVM support gracefully if Xen 
acceleration not enabled.

This is preferable to the existing segmentation fault due to use of
uninitialised xen_xc context, this is normally inialisted in xen_init which is
not called if xen acceleration is not enabled.

Signed-off-by: Ian Campbell <address@hidden>
Cc: Jan Kiszka <address@hidden>
Cc: Anthony PERARD <address@hidden>
---
 xen-all.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 377aff7..c04bcbf 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -783,6 +783,12 @@ int xen_hvm_init(void)
     unsigned long ioreq_pfn;
     XenIOState *state;
 
+    if (!xen_xc) {
+        errno = ENOTSUP;
+        perror("xen: xen hvm requires accel=xen");
+        return -errno;
+    }
+
     state = qemu_mallocz(sizeof (XenIOState));
 
     state->xce_handle = xen_xc_evtchn_open(NULL, 0);
-- 
1.7.2.5



-- 
Ian Campbell
Current Noise: Blind Melon - No Rain

Who are you?