Patchwork [08/10] monitor: QError support

login
register
mail settings
Submitter Luiz Capitulino
Date Nov. 17, 2009, 7:43 p.m.
Message ID <1258487037-24950-9-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/38706/
State New
Headers show

Comments

Luiz Capitulino - Nov. 17, 2009, 7:43 p.m.
This commit adds QError support in the Monitor.

A QError member is added to the Monitor struct. This new member
stores error information and is also used to check if an error
has occurred when the called handler returns.

Additionally, a new macro called qemu_error_new() is introduced.
It builds on top of the QemuErrorSink API and should be used in
place of qemu_error().

When all conversion to qemu_error_new() is done, qemu_error() can
be turned private.

Basically, Monitor's error flow is something like this:

1. An error occurs in the handler, it calls qemu_error_new()
2. qemu_error_new() builds a new QError object and stores it in
   the Monitor struct
3. The handler returns
4. Top level Monitor code checks the Monitor struct and calls
   qerror_print() to print the error

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   42 +++++++++++++++++++++++++++++++++++++++++-
 sysemu.h  |    6 ++++++
 2 files changed, 47 insertions(+), 1 deletions(-)
Markus Armbruster - Nov. 18, 2009, 3:16 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> This commit adds QError support in the Monitor.
>
> A QError member is added to the Monitor struct. This new member
> stores error information and is also used to check if an error
> has occurred when the called handler returns.
>
> Additionally, a new macro called qemu_error_new() is introduced.
> It builds on top of the QemuErrorSink API and should be used in
> place of qemu_error().
>
> When all conversion to qemu_error_new() is done, qemu_error() can
> be turned private.
>
> Basically, Monitor's error flow is something like this:
>
> 1. An error occurs in the handler, it calls qemu_error_new()
> 2. qemu_error_new() builds a new QError object and stores it in
>    the Monitor struct
> 3. The handler returns
> 4. Top level Monitor code checks the Monitor struct and calls
>    qerror_print() to print the error
[...]
> diff --git a/sysemu.h b/sysemu.h
> index b1887ef..656f6a4 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -7,6 +7,7 @@
>  #include "qemu-queue.h"
>  #include "qemu-timer.h"
>  #include "qdict.h"
> +#include "qerror.h"
>  
>  #ifdef _WIN32
>  #include <windows.h>
> @@ -71,6 +72,11 @@ void qemu_errors_to_file(FILE *fp);
>  void qemu_errors_to_mon(Monitor *mon);
>  void qemu_errors_to_previous(void);
>  void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
> +void qemu_error_full(const char *file, int linenr, const char *fmt, ...)
> +                     __attribute__ ((format(printf, 3, 4)));
> +
> +#define qemu_error_new(fmt, ...) \
> +    qemu_error_full(__FILE__, __LINE__, fmt, ## __VA_ARGS__)
>  
>  #ifdef _WIN32
>  /* Polling handling */

The relationship between qemu_error_new() and qemu_error_full() is not
obvious from their names.  What about calling the latter
qemu_error_new_internal()?
Luiz Capitulino - Nov. 18, 2009, 5:29 p.m.
On Wed, 18 Nov 2009 16:16:40 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > This commit adds QError support in the Monitor.
> >
> > A QError member is added to the Monitor struct. This new member
> > stores error information and is also used to check if an error
> > has occurred when the called handler returns.
> >
> > Additionally, a new macro called qemu_error_new() is introduced.
> > It builds on top of the QemuErrorSink API and should be used in
> > place of qemu_error().
> >
> > When all conversion to qemu_error_new() is done, qemu_error() can
> > be turned private.
> >
> > Basically, Monitor's error flow is something like this:
> >
> > 1. An error occurs in the handler, it calls qemu_error_new()
> > 2. qemu_error_new() builds a new QError object and stores it in
> >    the Monitor struct
> > 3. The handler returns
> > 4. Top level Monitor code checks the Monitor struct and calls
> >    qerror_print() to print the error
> [...]
> > diff --git a/sysemu.h b/sysemu.h
> > index b1887ef..656f6a4 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -7,6 +7,7 @@
> >  #include "qemu-queue.h"
> >  #include "qemu-timer.h"
> >  #include "qdict.h"
> > +#include "qerror.h"
> >  
> >  #ifdef _WIN32
> >  #include <windows.h>
> > @@ -71,6 +72,11 @@ void qemu_errors_to_file(FILE *fp);
> >  void qemu_errors_to_mon(Monitor *mon);
> >  void qemu_errors_to_previous(void);
> >  void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
> > +void qemu_error_full(const char *file, int linenr, const char *fmt, ...)
> > +                     __attribute__ ((format(printf, 3, 4)));
> > +
> > +#define qemu_error_new(fmt, ...) \
> > +    qemu_error_full(__FILE__, __LINE__, fmt, ## __VA_ARGS__)
> >  
> >  #ifdef _WIN32
> >  /* Polling handling */
> 
> The relationship between qemu_error_new() and qemu_error_full() is not
> obvious from their names.  What about calling the latter
> qemu_error_new_internal()?

 Okay, as I'm going to post a new version anyway.
Daniel P. Berrange - Nov. 18, 2009, 6:16 p.m.
On Tue, Nov 17, 2009 at 05:43:55PM -0200, Luiz Capitulino wrote:
> This commit adds QError support in the Monitor.
> 
> A QError member is added to the Monitor struct. This new member
> stores error information and is also used to check if an error
> has occurred when the called handler returns.
> 
> Additionally, a new macro called qemu_error_new() is introduced.
> It builds on top of the QemuErrorSink API and should be used in
> place of qemu_error().
> 
> When all conversion to qemu_error_new() is done, qemu_error() can
> be turned private.

> diff --git a/sysemu.h b/sysemu.h
> index b1887ef..656f6a4 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -7,6 +7,7 @@
>  #include "qemu-queue.h"
>  #include "qemu-timer.h"
>  #include "qdict.h"
> +#include "qerror.h"
>  
>  #ifdef _WIN32
>  #include <windows.h>
> @@ -71,6 +72,11 @@ void qemu_errors_to_file(FILE *fp);
>  void qemu_errors_to_mon(Monitor *mon);
>  void qemu_errors_to_previous(void);
>  void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
> +void qemu_error_full(const char *file, int linenr, const char *fmt, ...)
> +                     __attribute__ ((format(printf, 3, 4)));

Add in a 3rd param, 'const char *func'

> +
> +#define qemu_error_new(fmt, ...) \
> +    qemu_error_full(__FILE__, __LINE__, fmt, ## __VA_ARGS__)

And use  __func__  here

Regards,
Daniel

Patch

diff --git a/monitor.c b/monitor.c
index 3286ba2..74abef9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -49,6 +49,7 @@ 
 #include "qlist.h"
 #include "qdict.h"
 #include "qstring.h"
+#include "qerror.h"
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
@@ -103,6 +104,7 @@  struct Monitor {
     CPUState *mon_cpu;
     BlockDriverCompletionFunc *password_completion_cb;
     void *password_opaque;
+    QError *error;
     QLIST_HEAD(,mon_fd_t) fds;
     QLIST_ENTRY(Monitor) entry;
 };
@@ -224,6 +226,11 @@  static inline int monitor_handler_ported(const mon_cmd_t *cmd)
     return cmd->user_print != NULL;
 }
 
+static inline int monitor_has_error(const Monitor *mon)
+{
+    return mon->error != NULL;
+}
+
 static void monitor_print_qobject(Monitor *mon, const QObject *data)
 {
     switch (qobject_type(data)) {
@@ -3168,6 +3175,13 @@  fail:
     return NULL;
 }
 
+static void monitor_print_error(Monitor *mon)
+{
+    qerror_print(mon->error);
+    QDECREF(mon->error);
+    mon->error = NULL;
+}
+
 static void monitor_handle_command(Monitor *mon, const char *cmdline)
 {
     QDict *qdict;
@@ -3193,7 +3207,10 @@  static void monitor_handle_command(Monitor *mon, const char *cmdline)
         cmd->mhandler.cmd(mon, qdict);
     }
 
-   qemu_errors_to_previous();
+    if (monitor_has_error(mon))
+        monitor_print_error(mon);
+
+    qemu_errors_to_previous();
 
 out:
     QDECREF(qdict);
@@ -3644,3 +3661,26 @@  void qemu_error(const char *fmt, ...)
         break;
     }
 }
+
+void qemu_error_full(const char *file, int linenr, const char *fmt, ...)
+{
+    va_list va;
+    QError *qerror;
+
+    assert(qemu_error_sink != NULL);
+
+    va_start(va, fmt);
+    qerror = qerror_from_info(file, linenr, fmt, &va);
+    va_end(va);
+
+    switch (qemu_error_sink->dest) {
+    case ERR_SINK_FILE:
+        qerror_print(qerror);
+        QDECREF(qerror);
+        break;
+    case ERR_SINK_MONITOR:
+        assert(qemu_error_sink->mon->error == NULL);
+        qemu_error_sink->mon->error = qerror;
+        break;
+    }
+}
diff --git a/sysemu.h b/sysemu.h
index b1887ef..656f6a4 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -7,6 +7,7 @@ 
 #include "qemu-queue.h"
 #include "qemu-timer.h"
 #include "qdict.h"
+#include "qerror.h"
 
 #ifdef _WIN32
 #include <windows.h>
@@ -71,6 +72,11 @@  void qemu_errors_to_file(FILE *fp);
 void qemu_errors_to_mon(Monitor *mon);
 void qemu_errors_to_previous(void);
 void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
+void qemu_error_full(const char *file, int linenr, const char *fmt, ...)
+                     __attribute__ ((format(printf, 3, 4)));
+
+#define qemu_error_new(fmt, ...) \
+    qemu_error_full(__FILE__, __LINE__, fmt, ## __VA_ARGS__)
 
 #ifdef _WIN32
 /* Polling handling */