Patchwork [PATCHv3] qxl: abort on panic instead of exit

login
register
mail settings
Submitter Alon Levy
Date June 29, 2011, 11:57 a.m.
Message ID <1309348641-20061-7-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/102593/
State New
Headers show

Comments

Alon Levy - June 29, 2011, 11:57 a.m.
---
 hw/qxl.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Gerd Hoffmann - June 29, 2011, 12:44 p.m.
On 06/29/11 13:57, Alon Levy wrote:
> ---
>   hw/qxl.h |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/qxl.h b/hw/qxl.h
> index 7df594e..bf875a0 100644
> --- a/hw/qxl.h
> +++ b/hw/qxl.h
> @@ -89,7 +89,7 @@ typedef struct PCIQXLDevice {
>
>   #define PANIC_ON(x) if ((x)) {                         \
>       printf("%s: PANIC %s failed\n", __FUNCTION__, #x); \
> -    exit(-1);                                          \
> +    abort();                                          \
>   }

Not urgent, but I think this should change.  A bunch of the PANIC_ON 
checks are guest-triggerable.  Long-term I want invalid requests from 
the guest just be ignored by default.  Keeping an option to do something 
else is fine (log a message, abort, write out guest dump, whatever), I 
see that this can be quite useful for guest driver debugging.

cheers,
   Gerd
Alon Levy - June 29, 2011, 2:24 p.m.
On Wed, Jun 29, 2011 at 02:44:11PM +0200, Gerd Hoffmann wrote:
> On 06/29/11 13:57, Alon Levy wrote:
> >---
> >  hw/qxl.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/hw/qxl.h b/hw/qxl.h
> >index 7df594e..bf875a0 100644
> >--- a/hw/qxl.h
> >+++ b/hw/qxl.h
> >@@ -89,7 +89,7 @@ typedef struct PCIQXLDevice {
> >
> >  #define PANIC_ON(x) if ((x)) {                         \
> >      printf("%s: PANIC %s failed\n", __FUNCTION__, #x); \
> >-    exit(-1);                                          \
> >+    abort();                                          \
> >  }
> 
> Not urgent, but I think this should change.  A bunch of the PANIC_ON
> checks are guest-triggerable.  Long-term I want invalid requests
> from the guest just be ignored by default.  Keeping an option to do
> something else is fine (log a message, abort, write out guest dump,
> whatever), I see that this can be quite useful for guest driver
> debugging.
Agreed. So keep PANIC_ON but remove a lot of it's users, or add some wrapper
that depends on a "guest_panic" parameter that defaults off.

> 
> cheers,
>   Gerd
Gerd Hoffmann - June 29, 2011, 2:34 p.m.
Hi,

> Agreed. So keep PANIC_ON but remove a lot of it's users, or add some wrapper
> that depends on a "guest_panic" parameter that defaults off.

Maybe keep PANIC_ON in case there are any which can trigger on qemu bugs 
only.  Add a new one where the name makes clear this triggers on case 
the guest driver does something wrong, maybe qxl_guest_bug() or so. 
Note this isn't a simple search/replace as qemu continues execution, but 
still has to ignore the invalid request from the guest (and maybe raise 
an error irq)?

cheers,
   Gerd

Patch

diff --git a/hw/qxl.h b/hw/qxl.h
index 7df594e..bf875a0 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -89,7 +89,7 @@  typedef struct PCIQXLDevice {
 
 #define PANIC_ON(x) if ((x)) {                         \
     printf("%s: PANIC %s failed\n", __FUNCTION__, #x); \
-    exit(-1);                                          \
+    abort();                                          \
 }
 
 #define dprint(_qxl, _level, _fmt, ...)                                 \