[Qemu-devel] [RFC 00/27]: add new error format

[Qemu-devel] [RFC 00/27]: add new error format

[Luiz Capitulino]

[Please, read below why this is an RFC]

This series implements the 'Plan for error handling in QMP' as described
by Anthony in this email:

    http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html

Basically, this replaces almost all error classes by GenericError (the
exception are a few error classes used by libvirt) and drops the error
data memeber. This also adds a free form string to error_set().

On the wire, we go from:

    { "error": { "class": "DeviceNotRemovable",
                 "data": { "device": "virtio0" },
                 "desc": "Device 'virtio0' is not removable" } }

to:

     { "error": { "class": "GenericError",
                  "desc": "Device 'virtio0' is not removable" } }

Internally, we go from:

  void error_set(Error **err, const char *fmt, ...);

to:

  void error_set(Error **err, ErrorClass err_class, const char *fmt, ...);

This series is in RFC state because when I was almost done I've found that
I've broke error_is_type() and qemu-ga (both should be easy to fix). Besides,
the resulting code is missing some simplifications and more testing...

But I wanted to drop this before going out for the weekend :)

 QMP/qmp-spec.txt      |  10 +-
 block.c               |   1 +
 block_int.h           |   1 +
 configure             |  10 -
 error.c               |  46 +----
 error.h               |  15 +-
 error_int.h           |   1 -
 hmp.c                 |  80 +++++---
 hmp.h                 |   1 +
 monitor.c             |  80 ++------
 qapi-schema.json      |  33 +++-
 qerror.c              | 514 ++------------------------------------------------
 qerror.h              | 164 ++++++++--------
 qmp-commands.hx       |   3 +-
 scripts/qapi-types.py |  17 +-
 scripts/qapi.py       |   4 +-
 16 files changed, 226 insertions(+), 754 deletions(-)

[Qemu-devel] [PATCH 01/27] monitor: drop unused monitor debug code

[Luiz Capitulino]

In the old QMP days, this code was used to find out QMP commands that
might be calling monitor_printf() down its call chain.

This is almost impossible to happen today, because the qapi converted
commands don't even have a monitor object. Besides, it's been more than
a year since I used this last time.

Let's just drop it.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 configure | 10 ----------
 monitor.c | 62 --------------------------------------------------------------
 2 files changed, 72 deletions(-)

diff --git a/configure b/configure
index cef0a71..a5e43fd 100755
--- a/configure
+++ b/configure
@@ -147,7 +147,6 @@ vhost_net="no"
 kvm="no"
 gprof="no"
 debug_tcg="no"
-debug_mon="no"
 debug="no"
 strip_opt="yes"
 tcg_interpreter="no"
@@ -633,14 +632,9 @@ for opt do
   ;;
   --disable-debug-tcg) debug_tcg="no"
   ;;
-  --enable-debug-mon) debug_mon="yes"
-  ;;
-  --disable-debug-mon) debug_mon="no"
-  ;;
   --enable-debug)
       # Enable debugging options that aren't excessively noisy
       debug_tcg="yes"
-      debug_mon="yes"
       debug="yes"
       strip_opt="no"
   ;;
@@ -3007,7 +3001,6 @@ echo "host CPU          $cpu"
 echo "host big endian   $bigendian"
 echo "target list       $target_list"
 echo "tcg debug enabled $debug_tcg"
-echo "Mon debug enabled $debug_mon"
 echo "gprof enabled     $gprof"
 echo "sparse enabled    $sparse"
 echo "strip binaries    $strip_opt"
@@ -3100,9 +3093,6 @@ echo "ARCH=$ARCH" >> $config_host_mak
 if test "$debug_tcg" = "yes" ; then
   echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
 fi
-if test "$debug_mon" = "yes" ; then
-  echo "CONFIG_DEBUG_MONITOR=y" >> $config_host_mak
-fi
 if test "$debug" = "yes" ; then
   echo "CONFIG_DEBUG_EXEC=y" >> $config_host_mak
 fi
diff --git a/monitor.c b/monitor.c
index 09aa3cd..4eeb0c5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -180,33 +180,6 @@ struct Monitor {
     QLIST_ENTRY(Monitor) entry;
 };
 
-#ifdef CONFIG_DEBUG_MONITOR
-#define MON_DEBUG(fmt, ...) do {    \
-    fprintf(stderr, "Monitor: ");       \
-    fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-
-static inline void mon_print_count_inc(Monitor *mon)
-{
-    mon->print_calls_nr++;
-}
-
-static inline void mon_print_count_init(Monitor *mon)
-{
-    mon->print_calls_nr = 0;
-}
-
-static inline int mon_print_count_get(const Monitor *mon)
-{
-    return mon->print_calls_nr;
-}
-
-#else /* !CONFIG_DEBUG_MONITOR */
-#define MON_DEBUG(fmt, ...) do { } while (0)
-static inline void mon_print_count_inc(Monitor *mon) { }
-static inline void mon_print_count_init(Monitor *mon) { }
-static inline int mon_print_count_get(const Monitor *mon) { return 0; }
-#endif /* CONFIG_DEBUG_MONITOR */
-
 /* QMP checker flags */
 #define QMP_ACCEPT_UNKNOWNS 1
 
@@ -299,8 +272,6 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
     if (!mon)
         return;
 
-    mon_print_count_inc(mon);
-
     if (monitor_ctrl_mode(mon)) {
         return;
     }
@@ -3876,8 +3847,6 @@ void monitor_set_error(Monitor *mon, QError *qerror)
     if (!mon->error) {
         mon->error = qerror;
     } else {
-        MON_DEBUG("Additional error report at %s:%d\n",
-                  qerror->file, qerror->linenr);
         QDECREF(qerror);
     }
 }
@@ -3891,36 +3860,7 @@ static void handler_audit(Monitor *mon, const mon_cmd_t *cmd, int ret)
          * Action: Report an internal error to the client if in QMP.
          */
         qerror_report(QERR_UNDEFINED_ERROR);
-        MON_DEBUG("command '%s' returned failure but did not pass an error\n",
-                  cmd->name);
     }
-
-#ifdef CONFIG_DEBUG_MONITOR
-    if (!ret && monitor_has_error(mon)) {
-        /*
-         * If it returns success, it must not have passed an error.
-         *
-         * Action: Report the passed error to the client.
-         */
-        MON_DEBUG("command '%s' returned success but passed an error\n",
-                  cmd->name);
-    }
-
-    if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
-        /*
-         * Handlers should not call Monitor print functions.
-         *
-         * Action: Ignore them in QMP.
-         *
-         * (XXX: we don't check any 'info' or 'query' command here
-         * because the user print function _is_ called by do_info(), hence
-         * we will trigger this check. This problem will go away when we
-         * make 'query' commands real and kill do_info())
-         */
-        MON_DEBUG("command '%s' called print functions %d time(s)\n",
-                  cmd->name, mon_print_count_get(mon));
-    }
-#endif
 }
 
 static void handle_user_command(Monitor *mon, const char *cmdline)
@@ -4449,8 +4389,6 @@ static void qmp_call_cmd(Monitor *mon, const mon_cmd_t *cmd,
     int ret;
     QObject *data = NULL;
 
-    mon_print_count_init(mon);
-
     ret = cmd->mhandler.cmd_new(mon, params, &data);
     handler_audit(mon, cmd, ret);
     monitor_protocol_emitter(mon, data);
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 02/27] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg

[Luiz Capitulino]

Actually, renames it to 'object'. This must be what the original author
meant to write, as there's no 'object' in the error's data member.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qerror.c b/qerror.c
index 92c4eff..082de98 100644
--- a/qerror.c
+++ b/qerror.c
@@ -49,7 +49,7 @@ static const QErrorStringTable qerror_table[] = {
     },
     {
         .error_fmt = QERR_AMBIGUOUS_PATH,
-        .desc      = "Path '%(path)' does not uniquely identify a %(object)"
+        .desc      = "Path '%(path)' does not uniquely identify an object"
     },
     {
         .error_fmt = QERR_BAD_BUS_FOR_DEVICE,
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 03/27] qerror: QERR_DEVICE_ENCRYPTED: add filename info to human msg

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qerror.c b/qerror.c
index 082de98..b2ed0e3 100644
--- a/qerror.c
+++ b/qerror.c
@@ -81,7 +81,7 @@ static const QErrorStringTable qerror_table[] = {
     },
     {
         .error_fmt = QERR_DEVICE_ENCRYPTED,
-        .desc      = "Device '%(device)' is encrypted",
+        .desc      = "Device '%(device)' is encrypted (filename is '%(filename)')",
     },
     {
         .error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 04/27] qerror: reduce public exposure

[Luiz Capitulino]

Make functions only used in qerror.c static.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 10 +++++-----
 qerror.h |  5 -----
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/qerror.c b/qerror.c
index b2ed0e3..ce0499b 100644
--- a/qerror.c
+++ b/qerror.c
@@ -336,7 +336,7 @@ static const QErrorStringTable qerror_table[] = {
  *
  * Return strong reference.
  */
-QError *qerror_new(void)
+static QError *qerror_new(void)
 {
     QError *qerr;
 
@@ -424,8 +424,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
  *
  * Return strong reference.
  */
-QError *qerror_from_info(const char *file, int linenr, const char *func,
-                         const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *file, int linenr, const char *func,
+                                const char *fmt, va_list *va)
 {
     QError *qerr;
 
@@ -549,7 +549,7 @@ QString *qerror_human(const QError *qerror)
  * it uses error_report() for this, so that the output is routed to the right
  * place (ie. stderr or Monitor's device).
  */
-void qerror_print(QError *qerror)
+static void qerror_print(QError *qerror)
 {
     QString *qstring = qerror_human(qerror);
     loc_push_restore(&qerror->loc);
@@ -620,7 +620,7 @@ void assert_no_error(Error *err)
 /**
  * qobject_to_qerror(): Convert a QObject into a QError
  */
-QError *qobject_to_qerror(const QObject *obj)
+static QError *qobject_to_qerror(const QObject *obj)
 {
     if (qobject_type(obj) != QTYPE_QERROR) {
         return NULL;
diff --git a/qerror.h b/qerror.h
index b4c8758..fe8870c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -33,11 +33,7 @@ typedef struct QError {
     const QErrorStringTable *entry;
 } QError;
 
-QError *qerror_new(void);
-QError *qerror_from_info(const char *file, int linenr, const char *func,
-                         const char *fmt, va_list *va) GCC_FMT_ATTR(4, 0);
 QString *qerror_human(const QError *qerror);
-void qerror_print(QError *qerror);
 void qerror_report_internal(const char *file, int linenr, const char *func,
                             const char *fmt, ...) GCC_FMT_ATTR(4, 5);
 void qerror_report_err(Error *err);
@@ -45,7 +41,6 @@ void assert_no_error(Error *err);
 QString *qerror_format(const char *fmt, QDict *error);
 #define qerror_report(fmt, ...) \
     qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
-QError *qobject_to_qerror(const QObject *obj);
 
 /*
  * QError class list
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 05/27] qerror: drop qerror_abort()

[Luiz Capitulino]

qerror_abort() depends on the 'file', 'func' and 'linenr' members of
QError. However, these members are going to be dropped by the next
commit, so let's drop qerror_abort() in favor of printing an error
message to stderr plus an abort() call.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 65 ++++++++++++++++++++++++++--------------------------------------
 1 file changed, 26 insertions(+), 39 deletions(-)

diff --git a/qerror.c b/qerror.c
index ce0499b..59025ea 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,55 +346,46 @@ static QError *qerror_new(void)
     return qerr;
 }
 
-static void GCC_FMT_ATTR(2, 3) qerror_abort(const QError *qerr,
-                                            const char *fmt, ...)
-{
-    va_list ap;
-
-    fprintf(stderr, "qerror: bad call in function '%s':\n", qerr->func);
-    fprintf(stderr, "qerror: -> ");
-
-    va_start(ap, fmt);
-    vfprintf(stderr, fmt, ap);
-    va_end(ap);
-
-    fprintf(stderr, "\nqerror: call at %s:%d\n", qerr->file, qerr->linenr);
-    abort();
-}
-
-static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
-                                               const char *fmt, va_list *va)
+static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
 {
     QObject *obj;
+    QDict *ret;
 
     obj = qobject_from_jsonv(fmt, va);
     if (!obj) {
-        qerror_abort(qerr, "invalid format '%s'", fmt);
+        fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
+        abort();
     }
     if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_abort(qerr, "error format is not a QDict '%s'", fmt);
+        fprintf(stderr, "error is not a dict '%s'\n", fmt);
+        abort();
     }
 
-    qerr->error = qobject_to_qdict(obj);
-
-    obj = qdict_get(qerr->error, "class");
+    ret = qobject_to_qdict(obj);
+    obj = qdict_get(ret, "class");
     if (!obj) {
-        qerror_abort(qerr, "missing 'class' key in '%s'", fmt);
+        fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
+        abort();
     }
     if (qobject_type(obj) != QTYPE_QSTRING) {
-        qerror_abort(qerr, "'class' key value should be a QString");
+        fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
+        abort();
     }
-    
-    obj = qdict_get(qerr->error, "data");
+
+    obj = qdict_get(ret, "data");
     if (!obj) {
-        qerror_abort(qerr, "missing 'data' key in '%s'", fmt);
+        fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
+        abort();
     }
     if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_abort(qerr, "'data' key value should be a QDICT");
+        fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
+        abort();
     }
+
+    return ret;
 }
 
-static void qerror_set_desc(QError *qerr, const char *fmt)
+static const QErrorStringTable *get_desc_no_fail(const char *fmt)
 {
     int i;
 
@@ -402,12 +393,12 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
 
     for (i = 0; qerror_table[i].error_fmt; i++) {
         if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-            qerr->entry = &qerror_table[i];
-            return;
+            return &qerror_table[i];
         }
     }
 
-    qerror_abort(qerr, "error format '%s' not found", fmt);
+    fprintf(stderr, "error format '%s' not found\n", fmt);
+    abort();
 }
 
 /**
@@ -435,12 +426,8 @@ static QError *qerror_from_info(const char *file, int linenr, const char *func,
     qerr->file = file;
     qerr->func = func;
 
-    if (!fmt) {
-        qerror_abort(qerr, "QDict not specified");
-    }
-
-    qerror_set_data(qerr, fmt, va);
-    qerror_set_desc(qerr, fmt);
+    qerr->error = error_obj_from_fmt_no_fail(fmt, va);
+    qerr->entry = get_desc_no_fail(fmt);
 
     return qerr;
 }
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 06/27] qerror: QError: drop file, linenr, func

[Luiz Capitulino]

They have never been fully used and conflict with future error related
improvements.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 20 +++-----------------
 qerror.h |  8 +-------
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/qerror.c b/qerror.c
index 59025ea..f55a652 100644
--- a/qerror.c
+++ b/qerror.c
@@ -404,27 +404,14 @@ static const QErrorStringTable *get_desc_no_fail(const char *fmt)
 /**
  * qerror_from_info(): Create a new QError from error information
  *
- * The information consists of:
- *
- * - file   the file name of where the error occurred
- * - linenr the line number of where the error occurred
- * - func   the function name of where the error occurred
- * - fmt    JSON printf-like dictionary, there must exist keys 'class' and
- *          'data'
- * - va     va_list of all arguments specified by fmt
- *
  * Return strong reference.
  */
-static QError *qerror_from_info(const char *file, int linenr, const char *func,
-                                const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *fmt, va_list *va)
 {
     QError *qerr;
 
     qerr = qerror_new();
     loc_save(&qerr->loc);
-    qerr->linenr = linenr;
-    qerr->file = file;
-    qerr->func = func;
 
     qerr->error = error_obj_from_fmt_no_fail(fmt, va);
     qerr->entry = get_desc_no_fail(fmt);
@@ -545,14 +532,13 @@ static void qerror_print(QError *qerror)
     QDECREF(qstring);
 }
 
-void qerror_report_internal(const char *file, int linenr, const char *func,
-                            const char *fmt, ...)
+void qerror_report(const char *fmt, ...)
 {
     va_list va;
     QError *qerror;
 
     va_start(va, fmt);
-    qerror = qerror_from_info(file, linenr, func, fmt, &va);
+    qerror = qerror_from_info(fmt, &va);
     va_end(va);
 
     if (monitor_cur_is_qmp()) {
diff --git a/qerror.h b/qerror.h
index fe8870c..3c0b14c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,20 +27,14 @@ typedef struct QError {
     QObject_HEAD;
     QDict *error;
     Location loc;
-    int linenr;
-    const char *file;
-    const char *func;
     const QErrorStringTable *entry;
 } QError;
 
 QString *qerror_human(const QError *qerror);
-void qerror_report_internal(const char *file, int linenr, const char *func,
-                            const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void qerror_report_err(Error *err);
 void assert_no_error(Error *err);
 QString *qerror_format(const char *fmt, QDict *error);
-#define qerror_report(fmt, ...) \
-    qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
 
 /*
  * QError class list
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 07/27] qerror: qerror_format(): return an allocated string

[Luiz Capitulino]

Simplifies current and future users.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.c  |  5 +----
 qerror.c | 10 ++++++++--
 qerror.h |  2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/error.c b/error.c
index a52b771..b630b05 100644
--- a/error.c
+++ b/error.c
@@ -64,10 +64,7 @@ bool error_is_set(Error **errp)
 const char *error_get_pretty(Error *err)
 {
     if (err->msg == NULL) {
-        QString *str;
-        str = qerror_format(err->fmt, err->obj);
-        err->msg = g_strdup(qstring_get_str(str));
-        QDECREF(str);
+        err->msg = qerror_format(err->fmt, err->obj);
     }
 
     return err->msg;
diff --git a/qerror.c b/qerror.c
index f55a652..5b4266c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -493,9 +493,11 @@ static QString *qerror_format_desc(QDict *error,
     return qstring;
 }
 
-QString *qerror_format(const char *fmt, QDict *error)
+char *qerror_format(const char *fmt, QDict *error)
 {
     const QErrorStringTable *entry = NULL;
+    QString *qstr;
+    char *ret;
     int i;
 
     for (i = 0; qerror_table[i].error_fmt; i++) {
@@ -505,7 +507,11 @@ QString *qerror_format(const char *fmt, QDict *error)
         }
     }
 
-    return qerror_format_desc(error, entry);
+    qstr = qerror_format_desc(error, entry);
+    ret = g_strdup(qstring_get_str(qstr));
+    QDECREF(qstr);
+
+    return ret;
 }
 
 /**
diff --git a/qerror.h b/qerror.h
index 3c0b14c..aec76b2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -34,7 +34,7 @@ QString *qerror_human(const QError *qerror);
 void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void qerror_report_err(Error *err);
 void assert_no_error(Error *err);
-QString *qerror_format(const char *fmt, QDict *error);
+char *qerror_format(const char *fmt, QDict *error);
 
 /*
  * QError class list
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 08/27] qerror: don't delay error message construction

[Luiz Capitulino]

Today, the error message is only constructed when it's used. This commit
changes that to construct the error message when the error object is
built (ie. when the error is reported).

This eliminates the need of storing a pointer to qerror_table[], which
will be dropped soon, and also simplifies the code.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 29 ++++-------------------------
 qerror.h |  2 +-
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/qerror.c b/qerror.c
index 5b4266c..5b7d67d 100644
--- a/qerror.c
+++ b/qerror.c
@@ -385,22 +385,6 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
     return ret;
 }
 
-static const QErrorStringTable *get_desc_no_fail(const char *fmt)
-{
-    int i;
-
-    // FIXME: inefficient loop
-
-    for (i = 0; qerror_table[i].error_fmt; i++) {
-        if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-            return &qerror_table[i];
-        }
-    }
-
-    fprintf(stderr, "error format '%s' not found\n", fmt);
-    abort();
-}
-
 /**
  * qerror_from_info(): Create a new QError from error information
  *
@@ -414,7 +398,7 @@ static QError *qerror_from_info(const char *fmt, va_list *va)
     loc_save(&qerr->loc);
 
     qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-    qerr->entry = get_desc_no_fail(fmt);
+    qerr->err_msg = qerror_format(fmt, qerr->error);
 
     return qerr;
 }
@@ -519,7 +503,7 @@ char *qerror_format(const char *fmt, QDict *error)
  */
 QString *qerror_human(const QError *qerror)
 {
-    return qerror_format_desc(qerror->error, qerror->entry);
+    return qstring_from_str(qerror->err_msg);
 }
 
 /**
@@ -566,19 +550,13 @@ struct Error
 void qerror_report_err(Error *err)
 {
     QError *qerr;
-    int i;
 
     qerr = qerror_new();
     loc_save(&qerr->loc);
     QINCREF(err->obj);
     qerr->error = err->obj;
 
-    for (i = 0; qerror_table[i].error_fmt; i++) {
-        if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
-            qerr->entry = &qerror_table[i];
-            break;
-        }
-    }
+    qerr->err_msg = qerror_format(err->fmt, qerr->error);
 
     if (monitor_cur_is_qmp()) {
         monitor_set_error(cur_mon, qerr);
@@ -619,5 +597,6 @@ static void qerror_destroy_obj(QObject *obj)
     qerr = qobject_to_qerror(obj);
 
     QDECREF(qerr->error);
+    g_free(qerr->err_msg);
     g_free(qerr);
 }
diff --git a/qerror.h b/qerror.h
index aec76b2..de8497d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,7 +27,7 @@ typedef struct QError {
     QObject_HEAD;
     QDict *error;
     Location loc;
-    const QErrorStringTable *entry;
+    char *err_msg;
 } QError;
 
 QString *qerror_human(const QError *qerror);
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 09/27] error: don't delay error message construction

[Luiz Capitulino]

Today, the error message is only constructed when it's used. This commit
changes that to construct the error message when the error object is
built (ie. when the error is reported).

This simplifies the Error object.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.c  | 8 +-------
 qerror.c | 4 +---
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/error.c b/error.c
index b630b05..acb10a2 100644
--- a/error.c
+++ b/error.c
@@ -20,7 +20,6 @@
 struct Error
 {
     QDict *obj;
-    const char *fmt;
     char *msg;
 };
 
@@ -38,7 +37,7 @@ void error_set(Error **errp, const char *fmt, ...)
     va_start(ap, fmt);
     err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
     va_end(ap);
-    err->fmt = fmt;
+    err->msg = qerror_format(fmt, err->obj);
 
     *errp = err;
 }
@@ -49,7 +48,6 @@ Error *error_copy(const Error *err)
 
     err_new = g_malloc0(sizeof(*err));
     err_new->msg = g_strdup(err->msg);
-    err_new->fmt = err->fmt;
     err_new->obj = err->obj;
     QINCREF(err_new->obj);
 
@@ -63,10 +61,6 @@ bool error_is_set(Error **errp)
 
 const char *error_get_pretty(Error *err)
 {
-    if (err->msg == NULL) {
-        err->msg = qerror_format(err->fmt, err->obj);
-    }
-
     return err->msg;
 }
 
diff --git a/qerror.c b/qerror.c
index 5b7d67d..691d8a8 100644
--- a/qerror.c
+++ b/qerror.c
@@ -543,7 +543,6 @@ void qerror_report(const char *fmt, ...)
 struct Error
 {
     QDict *obj;
-    const char *fmt;
     char *msg;
 };
 
@@ -555,8 +554,7 @@ void qerror_report_err(Error *err)
     loc_save(&qerr->loc);
     QINCREF(err->obj);
     qerr->error = err->obj;
-
-    qerr->err_msg = qerror_format(err->fmt, qerr->error);
+    qerr->err_msg = g_strdup(err->msg);
 
     if (monitor_cur_is_qmp()) {
         monitor_set_error(cur_mon, qerr);
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 10/27] qmp: query-block: add 'valid_encryption_key' field

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 block.c          | 1 +
 qapi-schema.json | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index ce7eb8f..b230f16 100644
--- a/block.c
+++ b/block.c
@@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
             info->value->inserted->ro = bs->read_only;
             info->value->inserted->drv = g_strdup(bs->drv->format_name);
             info->value->inserted->encrypted = bs->encrypted;
+            info->value->inserted->valid_encryption_key = bs->valid_key;
             if (bs->backing_file[0]) {
                 info->value->inserted->has_backing_file = true;
                 info->value->inserted->backing_file = g_strdup(bs->backing_file);
diff --git a/qapi-schema.json b/qapi-schema.json
index a92adb1..7500754 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -400,6 +400,8 @@
 #
 # @encrypted: true if the backing device is encrypted
 #
+# @valid_encryption_key: true if a valid encryption key has been set
+#
 # @bps: total throughput limit in bytes per second is specified
 #
 # @bps_rd: read throughput limit in bytes per second is specified
@@ -419,8 +421,9 @@
 { 'type': 'BlockDeviceInfo',
   'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
             '*backing_file': 'str', 'encrypted': 'bool',
-            'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
-            'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
+            'valid_encryption_key': 'bool', 'bps': 'int',
+            'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
+            'iops_rd': 'int', 'iops_wr': 'int'} }
 
 ##
 # @BlockDeviceIoStatus:
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 11/27] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED

[Luiz Capitulino]

Today, hmp_cont() checks for QERR_DEVICE_ENCRYPTED in order to know
if qmp_cont() failed due to an encrypted device. If it did,
hmp_cont() accesses QERR_DEVICE_ENCRYPTED's data member 'device' to
precisely know for which device an encryption key must be set.

However, all errors data members are going to be dropped by a later
commit, so hmp_cont() can't do this anymore.

This commit changes hmp_cont() to loop through all block devices
and proactively set an encryption key for any encrypted device
without a valid one.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/hmp.c b/hmp.c
index 6b72a64..a906f8a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -610,34 +610,41 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
 
 static void hmp_cont_cb(void *opaque, int err)
 {
-    Monitor *mon = opaque;
-
     if (!err) {
-        hmp_cont(mon, NULL);
+        qmp_cont(NULL);
     }
 }
 
-void hmp_cont(Monitor *mon, const QDict *qdict)
+static bool block_dev_is_encrypted(const BlockInfo *bdev)
 {
-    Error *errp = NULL;
-
-    qmp_cont(&errp);
-    if (error_is_set(&errp)) {
-        if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
-            const char *device;
+    return (bdev->inserted && bdev->inserted->encrypted);
+}
 
-            /* The device is encrypted. Ask the user for the password
-               and retry */
+static bool block_dev_key_is_set(const BlockInfo *bdev)
+{
+    return (bdev->inserted && bdev->inserted->valid_encryption_key);
+}
 
-            device = error_get_field(errp, "device");
-            assert(device != NULL);
+void hmp_cont(Monitor *mon, const QDict *qdict)
+{
+    BlockInfoList *bdev_list, *bdev;
+    Error *errp = NULL;
 
-            monitor_read_block_device_key(mon, device, hmp_cont_cb, mon);
-            error_free(errp);
-            return;
+    bdev_list = qmp_query_block(NULL);
+    for (bdev = bdev_list; bdev; bdev = bdev->next) {
+        if (block_dev_is_encrypted(bdev->value) &&
+            !block_dev_key_is_set(bdev->value)) {
+                monitor_read_block_device_key(mon, bdev->value->device,
+                                              hmp_cont_cb, NULL);
+                goto out;
         }
-        hmp_handle_error(mon, &errp);
     }
+
+    qmp_cont(&errp);
+    hmp_handle_error(mon, &errp);
+
+out:
+    qapi_free_BlockInfoList(bdev_list);
 }
 
 void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 12/27] hmp: hmp_change(): don't use error_get_field()

[Luiz Capitulino]

Use the 'device' passed by the user and call qmp_query_block() to
get the 'filename' info.

error_get_field() is going to be dropped by a future commit.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/hmp.c b/hmp.c
index a906f8a..435c9cd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -783,17 +783,35 @@ static void hmp_change_read_arg(Monitor *mon, const char *password,
 static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
                                    void *opaque)
 {
-    Error *encryption_err = opaque;
+    char *device = opaque;
     Error *err = NULL;
-    const char *device;
-
-    device = error_get_field(encryption_err, "device");
 
     qmp_block_passwd(device, password, &err);
     hmp_handle_error(mon, &err);
-    error_free(encryption_err);
 
     monitor_read_command(mon, 1);
+    g_free(device);
+}
+
+static char *get_device_file(const char *device)
+{
+    BlockInfoList *bdev_list, *bdev;
+    char *ret;
+
+    bdev_list = qmp_query_block(NULL);
+    for (bdev = bdev_list; bdev; bdev = bdev->next) {
+        if (!strcmp(bdev->value->device, device)) {
+            break;
+        }
+    }
+
+    assert(bdev);
+    assert(bdev->value->has_inserted);
+
+    ret = g_strdup(bdev->value->inserted->file);
+    qapi_free_BlockInfoList(bdev_list);
+
+    return ret;
 }
 
 void hmp_change(Monitor *mon, const QDict *qdict)
@@ -814,9 +832,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 
     qmp_change(device, target, !!arg, arg, &err);
     if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
-        monitor_printf(mon, "%s (%s) is encrypted.\n",
-                       error_get_field(err, "device"),
-                       error_get_field(err, "filename"));
+        char *filename = get_device_file(device);
+        monitor_printf(mon, "%s (%s) is encrypted.\n", device, filename);
+        g_free(filename);
         if (!monitor_get_rs(mon)) {
             monitor_printf(mon,
                     "terminal does not support password prompting\n");
@@ -824,9 +842,10 @@ void hmp_change(Monitor *mon, const QDict *qdict)
             return;
         }
         readline_start(monitor_get_rs(mon), "Password: ", 1,
-                       cb_hmp_change_bdrv_pwd, err);
+                       cb_hmp_change_bdrv_pwd, (void *) g_strdup(device));
         return;
     }
+
     hmp_handle_error(mon, &err);
 }
 
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 13/27] error: error_is_type(): don't use error_get_field()

[Luiz Capitulino]

It's going to be dropped.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/error.c b/error.c
index acb10a2..cdeeafe 100644
--- a/error.c
+++ b/error.c
@@ -113,7 +113,7 @@ bool error_is_type(Error *err, const char *fmt)
     end = strchr(ptr, '\'');
     assert(end != NULL);
 
-    error_class = error_get_field(err, "class");
+    error_class = qdict_get_str(err->obj, "class");
     if (strlen(error_class) != end - ptr) {
         return false;
     }
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 14/27] error: drop functions used to get error data

[Luiz Capitulino]

They are not used anymore and all current errors' data are going to
be dropped.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.c     | 23 -----------------------
 error.h     | 10 ----------
 error_int.h |  1 -
 3 files changed, 34 deletions(-)

diff --git a/error.c b/error.c
index cdeeafe..b9d9b64 100644
--- a/error.c
+++ b/error.c
@@ -64,29 +64,6 @@ const char *error_get_pretty(Error *err)
     return err->msg;
 }
 
-const char *error_get_field(Error *err, const char *field)
-{
-    if (strcmp(field, "class") == 0) {
-        return qdict_get_str(err->obj, field);
-    } else {
-        QDict *dict = qdict_get_qdict(err->obj, "data");
-        return qdict_get_str(dict, field);
-    }
-}
-
-QDict *error_get_data(Error *err)
-{
-    QDict *data = qdict_get_qdict(err->obj, "data");
-    QINCREF(data);
-    return data;
-}
-
-void error_set_field(Error *err, const char *field, const char *value)
-{
-    QDict *dict = qdict_get_qdict(err->obj, "data");
-    qdict_put(dict, field, qstring_from_str(value));
-}
-
 void error_free(Error *err)
 {
     if (err) {
diff --git a/error.h b/error.h
index 45ff6c1..592a734 100644
--- a/error.h
+++ b/error.h
@@ -45,16 +45,6 @@ Error *error_copy(const Error *err);
 const char *error_get_pretty(Error *err);
 
 /**
- * Get an individual named error field.
- */
-const char *error_get_field(Error *err, const char *field);
-
-/**
- * Get an individual named error field.
- */
-void error_set_field(Error *err, const char *field, const char *value);
-
-/**
  * Propagate an error to an indirect pointer to an error.  This function will
  * always transfer ownership of the error reference and handles the case where
  * dst_err is NULL correctly.
diff --git a/error_int.h b/error_int.h
index 5e39424..4b00d08 100644
--- a/error_int.h
+++ b/error_int.h
@@ -22,7 +22,6 @@
  *
  * These are used to convert QErrors to Errors
  */
-QDict *error_get_data(Error *err);
 QObject *error_get_qobject(Error *err);
 void error_set_qobject(Error **errp, QObject *obj);
   
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 15/27] block: block_int: include qerror.h

[Luiz Capitulino]

Several block/ files are relying on qerror.h being provided by
qapi-types.h. However, qapi-types.h won't provide it anymore.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 block_int.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block_int.h b/block_int.h
index d72317f..00892e8 100644
--- a/block_int.h
+++ b/block_int.h
@@ -30,6 +30,7 @@
 #include "qemu-coroutine.h"
 #include "qemu-timer.h"
 #include "qapi-types.h"
+#include "qerror.h"
 
 #define BLOCK_FLAG_ENCRYPT	1
 #define BLOCK_FLAG_COMPAT6	4
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 16/27] hmp: hmp.h: include qdict.h

[Luiz Capitulino]

hmp.h is relying on qdict.h being provided by qapi-types.h. However,
qapi-types.h won't provide it anymore.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hmp.h b/hmp.h
index 8d2b0d7..3275522 100644
--- a/hmp.h
+++ b/hmp.h
@@ -16,6 +16,7 @@
 
 #include "qemu-common.h"
 #include "qapi-types.h"
+#include "qdict.h"
 
 void hmp_info_name(Monitor *mon);
 void hmp_info_version(Monitor *mon);
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 17/27] qapi: qapi-types.h: don't include qapi/qapi-types-core.h

[Luiz Capitulino]

qapi-types.h needs only qemu-common.h. Including qapi-types-core.h
causes problems when qerror.h or error.h includes qapi-types.h.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi-types.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a734f5..3ed9f04 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -253,7 +253,8 @@ fdecl.write(mcgen('''
 #ifndef %(guard)s
 #define %(guard)s
 
-#include "qapi/qapi-types-core.h"
+#include "qemu-common.h"
+
 ''',
                   guard=guardname(h_file)))
 
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 18/27] qapi: generate correct enum names for camel case enums

[Luiz Capitulino]

That's, generate ERROR_CLASS_GENERIC_ERROR instead of
ERROR_CLASS_GENERICERROR.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi-types.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 3ed9f04..9b7da96 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -79,6 +79,16 @@ const char *%(name)s_lookup[] = {
 ''')
     return ret
 
+def generate_enum_name(name):
+    if name.isupper():
+        return c_fun(name)
+    new_name = ''
+    for c in c_fun(name):
+        if c.isupper():
+            new_name += '_'
+        new_name += c
+    return new_name.lstrip('_').upper()
+
 def generate_enum(name, values):
     lookup_decl = mcgen('''
 extern const char *%(name)s_lookup[];
@@ -100,7 +110,7 @@ typedef enum %(name)s
     %(abbrev)s_%(value)s = %(i)d,
 ''',
                      abbrev=de_camel_case(name).upper(),
-                     value=c_fun(value).upper(),
+                     value=generate_enum_name(value),
                      i=i)
         i += 1
 
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 19/27] qapi: don't convert enum strings to lowercase

[Luiz Capitulino]

Next commit will introduce enum strings in camel case.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi-types.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9b7da96..cf601ae 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -70,7 +70,7 @@ const char *%(name)s_lookup[] = {
         ret += mcgen('''
     "%(value)s",
 ''',
-                     value=value.lower())
+                     value=value)
 
     ret += mcgen('''
     NULL,
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 20/27] qapi-schema: add ErrorClass enum

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi-schema.json | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 7500754..b1325c4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3,6 +3,32 @@
 # QAPI Schema
 
 ##
+# @ErrorClass
+#
+# QEMU error classes
+#
+# @GenericError: this is used for errors that don't require a specific error
+#                class. This should be the default case for most errors
+#
+# @CommandNotFound: the request command has not been found
+#
+# @DeviceNotActive: a device has failed to be become active
+#
+# @DeviceNotFound: the requested device has not been found
+#
+# @KVMMissingCap: the requested operation can't be fulfilled because a
+#                 required KVM capability is missing
+#
+# @MigrationExpected: the requested operation can't be fulfilled because a
+#                     migration process is expected
+#
+# Since: 1.2
+##
+{ 'enum': 'ErrorClass',
+  'data': [ 'GenericError', 'CommandNotFound', 'DeviceNotActive',
+            'KVMMissingCap', 'DeviceNotFound', 'MigrationExpected' ] }
+
+##
 # @NameInfo:
 #
 # Guest name information.
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 21/27] qerror: qerror_table: don't use C99 struct initializers

[Luiz Capitulino]

This allows for changing QERR_ macros to initialize two struct members
at the same time. See next commit for more details.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 280 +++++++++++++++++++++++++++++++--------------------------------
 qerror.h |   2 +-
 2 files changed, 141 insertions(+), 141 deletions(-)

diff --git a/qerror.c b/qerror.c
index 691d8a8..cdaaf17 100644
--- a/qerror.c
+++ b/qerror.c
@@ -44,289 +44,289 @@ static const QType qerror_type = {
  */
 static const QErrorStringTable qerror_table[] = {
     {
-        .error_fmt = QERR_ADD_CLIENT_FAILED,
-        .desc      = "Could not add client",
+         QERR_ADD_CLIENT_FAILED,
+         "Could not add client",
     },
     {
-        .error_fmt = QERR_AMBIGUOUS_PATH,
-        .desc      = "Path '%(path)' does not uniquely identify an object"
+         QERR_AMBIGUOUS_PATH,
+         "Path '%(path)' does not uniquely identify an object"
     },
     {
-        .error_fmt = QERR_BAD_BUS_FOR_DEVICE,
-        .desc      = "Device '%(device)' can't go on a %(bad_bus_type) bus",
+         QERR_BAD_BUS_FOR_DEVICE,
+         "Device '%(device)' can't go on a %(bad_bus_type) bus",
     },
     {
-        .error_fmt = QERR_BASE_NOT_FOUND,
-        .desc      = "Base '%(base)' not found",
+         QERR_BASE_NOT_FOUND,
+         "Base '%(base)' not found",
     },
     {
-        .error_fmt = QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-        .desc      = "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
+         QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+         "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
     },
     {
-        .error_fmt = QERR_BUS_NO_HOTPLUG,
-        .desc      = "Bus '%(bus)' does not support hotplugging",
+         QERR_BUS_NO_HOTPLUG,
+         "Bus '%(bus)' does not support hotplugging",
     },
     {
-        .error_fmt = QERR_BUS_NOT_FOUND,
-        .desc      = "Bus '%(bus)' not found",
+         QERR_BUS_NOT_FOUND,
+         "Bus '%(bus)' not found",
     },
     {
-        .error_fmt = QERR_COMMAND_DISABLED,
-        .desc      = "The command %(name) has been disabled for this instance",
+         QERR_COMMAND_DISABLED,
+         "The command %(name) has been disabled for this instance",
     },
     {
-        .error_fmt = QERR_COMMAND_NOT_FOUND,
-        .desc      = "The command %(name) has not been found",
+         QERR_COMMAND_NOT_FOUND,
+         "The command %(name) has not been found",
     },
     {
-        .error_fmt = QERR_DEVICE_ENCRYPTED,
-        .desc      = "Device '%(device)' is encrypted (filename is '%(filename)')",
+         QERR_DEVICE_ENCRYPTED,
+         "Device '%(device)' is encrypted (filename is '%(filename)')",
     },
     {
-        .error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-        .desc      = "Migration is disabled when using feature '%(feature)' in device '%(device)'",
+         QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
+         "Migration is disabled when using feature '%(feature)' in device '%(device)'",
     },
     {
-        .error_fmt = QERR_DEVICE_HAS_NO_MEDIUM,
-        .desc      = "Device '%(device)' has no medium",
+         QERR_DEVICE_HAS_NO_MEDIUM,
+         "Device '%(device)' has no medium",
     },
     {
-        .error_fmt = QERR_DEVICE_INIT_FAILED,
-        .desc      = "Device '%(device)' could not be initialized",
+         QERR_DEVICE_INIT_FAILED,
+         "Device '%(device)' could not be initialized",
     },
     {
-        .error_fmt = QERR_DEVICE_IN_USE,
-        .desc      = "Device '%(device)' is in use",
+         QERR_DEVICE_IN_USE,
+         "Device '%(device)' is in use",
     },
     {
-        .error_fmt = QERR_DEVICE_IS_READ_ONLY,
-        .desc      = "Device '%(device)' is read only",
+         QERR_DEVICE_IS_READ_ONLY,
+         "Device '%(device)' is read only",
     },
     {
-        .error_fmt = QERR_DEVICE_LOCKED,
-        .desc      = "Device '%(device)' is locked",
+         QERR_DEVICE_LOCKED,
+         "Device '%(device)' is locked",
     },
     {
-        .error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
-        .desc      = "Device '%(device)' has multiple child busses",
+         QERR_DEVICE_MULTIPLE_BUSSES,
+         "Device '%(device)' has multiple child busses",
     },
     {
-        .error_fmt = QERR_DEVICE_NO_BUS,
-        .desc      = "Device '%(device)' has no child bus",
+         QERR_DEVICE_NO_BUS,
+         "Device '%(device)' has no child bus",
     },
     {
-        .error_fmt = QERR_DEVICE_NO_HOTPLUG,
-        .desc      = "Device '%(device)' does not support hotplugging",
+         QERR_DEVICE_NO_HOTPLUG,
+         "Device '%(device)' does not support hotplugging",
     },
     {
-        .error_fmt = QERR_DEVICE_NOT_ACTIVE,
-        .desc      = "Device '%(device)' has not been activated",
+         QERR_DEVICE_NOT_ACTIVE,
+         "Device '%(device)' has not been activated",
     },
     {
-        .error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
-        .desc      = "Device '%(device)' is not encrypted",
+         QERR_DEVICE_NOT_ENCRYPTED,
+         "Device '%(device)' is not encrypted",
     },
     {
-        .error_fmt = QERR_DEVICE_NOT_FOUND,
-        .desc      = "Device '%(device)' not found",
+         QERR_DEVICE_NOT_FOUND,
+         "Device '%(device)' not found",
     },
     {
-        .error_fmt = QERR_DEVICE_NOT_REMOVABLE,
-        .desc      = "Device '%(device)' is not removable",
+         QERR_DEVICE_NOT_REMOVABLE,
+         "Device '%(device)' is not removable",
     },
     {
-        .error_fmt = QERR_DUPLICATE_ID,
-        .desc      = "Duplicate ID '%(id)' for %(object)",
+         QERR_DUPLICATE_ID,
+         "Duplicate ID '%(id)' for %(object)",
     },
     {
-        .error_fmt = QERR_FD_NOT_FOUND,
-        .desc      = "File descriptor named '%(name)' not found",
+         QERR_FD_NOT_FOUND,
+         "File descriptor named '%(name)' not found",
     },
     {
-        .error_fmt = QERR_FD_NOT_SUPPLIED,
-        .desc      = "No file descriptor supplied via SCM_RIGHTS",
+         QERR_FD_NOT_SUPPLIED,
+         "No file descriptor supplied via SCM_RIGHTS",
     },
     {
-        .error_fmt = QERR_FEATURE_DISABLED,
-        .desc      = "The feature '%(name)' is not enabled",
+         QERR_FEATURE_DISABLED,
+         "The feature '%(name)' is not enabled",
     },
     {
-        .error_fmt = QERR_INVALID_BLOCK_FORMAT,
-        .desc      = "Invalid block format '%(name)'",
+         QERR_INVALID_BLOCK_FORMAT,
+         "Invalid block format '%(name)'",
     },
     {
-        .error_fmt = QERR_INVALID_OPTION_GROUP,
-        .desc      = "There is no option group '%(group)'",
+         QERR_INVALID_OPTION_GROUP,
+         "There is no option group '%(group)'",
     },
     {
-        .error_fmt = QERR_INVALID_PARAMETER,
-        .desc      = "Invalid parameter '%(name)'",
+         QERR_INVALID_PARAMETER,
+         "Invalid parameter '%(name)'",
     },
     {
-        .error_fmt = QERR_INVALID_PARAMETER_COMBINATION,
-        .desc      = "Invalid parameter combination",
+         QERR_INVALID_PARAMETER_COMBINATION,
+         "Invalid parameter combination",
     },
     {
-        .error_fmt = QERR_INVALID_PARAMETER_TYPE,
-        .desc      = "Invalid parameter type for '%(name)', expected: %(expected)",
+         QERR_INVALID_PARAMETER_TYPE,
+         "Invalid parameter type for '%(name)', expected: %(expected)",
     },
     {
-        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
-        .desc      = "Parameter '%(name)' expects %(expected)",
+         QERR_INVALID_PARAMETER_VALUE,
+         "Parameter '%(name)' expects %(expected)",
     },
     {
-        .error_fmt = QERR_INVALID_PASSWORD,
-        .desc      = "Password incorrect",
+         QERR_INVALID_PASSWORD,
+         "Password incorrect",
     },
     {
-        .error_fmt = QERR_IO_ERROR,
-        .desc      = "An IO error has occurred",
+         QERR_IO_ERROR,
+         "An IO error has occurred",
     },
     {
-        .error_fmt = QERR_JSON_PARSE_ERROR,
-        .desc      = "JSON parse error, %(message)",
+         QERR_JSON_PARSE_ERROR,
+         "JSON parse error, %(message)",
 
     },
     {
-        .error_fmt = QERR_JSON_PARSING,
-        .desc      = "Invalid JSON syntax",
+         QERR_JSON_PARSING,
+         "Invalid JSON syntax",
     },
     {
-        .error_fmt = QERR_KVM_MISSING_CAP,
-        .desc      = "Using KVM without %(capability), %(feature) unavailable",
+         QERR_KVM_MISSING_CAP,
+         "Using KVM without %(capability), %(feature) unavailable",
     },
     {
-        .error_fmt = QERR_MIGRATION_ACTIVE,
-        .desc      = "There's a migration process in progress",
+         QERR_MIGRATION_ACTIVE,
+         "There's a migration process in progress",
     },
     {
-        .error_fmt = QERR_MIGRATION_NOT_SUPPORTED,
-        .desc      = "State blocked by non-migratable device '%(device)'",
+         QERR_MIGRATION_NOT_SUPPORTED,
+         "State blocked by non-migratable device '%(device)'",
     },
     {
-        .error_fmt = QERR_MIGRATION_EXPECTED,
-        .desc      = "An incoming migration is expected before this command can be executed",
+         QERR_MIGRATION_EXPECTED,
+         "An incoming migration is expected before this command can be executed",
     },
     {
-        .error_fmt = QERR_MISSING_PARAMETER,
-        .desc      = "Parameter '%(name)' is missing",
+         QERR_MISSING_PARAMETER,
+         "Parameter '%(name)' is missing",
     },
     {
-        .error_fmt = QERR_NO_BUS_FOR_DEVICE,
-        .desc      = "No '%(bus)' bus found for device '%(device)'",
+         QERR_NO_BUS_FOR_DEVICE,
+         "No '%(bus)' bus found for device '%(device)'",
     },
     {
-        .error_fmt = QERR_NOT_SUPPORTED,
-        .desc      = "Not supported",
+         QERR_NOT_SUPPORTED,
+         "Not supported",
     },
     {
-        .error_fmt = QERR_OPEN_FILE_FAILED,
-        .desc      = "Could not open '%(filename)'",
+         QERR_OPEN_FILE_FAILED,
+         "Could not open '%(filename)'",
     },
     {
-        .error_fmt = QERR_PERMISSION_DENIED,
-        .desc      = "Insufficient permission to perform this operation",
+         QERR_PERMISSION_DENIED,
+         "Insufficient permission to perform this operation",
     },
     {
-        .error_fmt = QERR_PROPERTY_NOT_FOUND,
-        .desc      = "Property '%(device).%(property)' not found",
+         QERR_PROPERTY_NOT_FOUND,
+         "Property '%(device).%(property)' not found",
     },
     {
-        .error_fmt = QERR_PROPERTY_VALUE_BAD,
-        .desc      = "Property '%(device).%(property)' doesn't take value '%(value)'",
+         QERR_PROPERTY_VALUE_BAD,
+         "Property '%(device).%(property)' doesn't take value '%(value)'",
     },
     {
-        .error_fmt = QERR_PROPERTY_VALUE_IN_USE,
-        .desc      = "Property '%(device).%(property)' can't take value '%(value)', it's in use",
+         QERR_PROPERTY_VALUE_IN_USE,
+         "Property '%(device).%(property)' can't take value '%(value)', it's in use",
     },
     {
-        .error_fmt = QERR_PROPERTY_VALUE_NOT_FOUND,
-        .desc      = "Property '%(device).%(property)' can't find value '%(value)'",
+         QERR_PROPERTY_VALUE_NOT_FOUND,
+         "Property '%(device).%(property)' can't find value '%(value)'",
     },
     {
-        .error_fmt = QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
-        .desc      = "Property '%(device).%(property)' doesn't take "
+         QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
+         "Property '%(device).%(property)' doesn't take "
                      "value '%(value)', it's not a power of 2",
     },
     {
-        .error_fmt = QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-        .desc      = "Property '%(device).%(property)' doesn't take "
+         QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+         "Property '%(device).%(property)' doesn't take "
                      "value %(value) (minimum: %(min), maximum: %(max))",
     },
     {
-        .error_fmt = QERR_QGA_COMMAND_FAILED,
-        .desc      = "Guest agent command failed, error was '%(message)'",
+         QERR_QGA_COMMAND_FAILED,
+         "Guest agent command failed, error was '%(message)'",
     },
     {
-        .error_fmt = QERR_QGA_LOGGING_FAILED,
-        .desc      = "Guest agent failed to log non-optional log statement",
+         QERR_QGA_LOGGING_FAILED,
+         "Guest agent failed to log non-optional log statement",
     },
     {
-        .error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
-        .desc      = "Expected '%(expected)' in QMP input",
+         QERR_QMP_BAD_INPUT_OBJECT,
+         "Expected '%(expected)' in QMP input",
     },
     {
-        .error_fmt = QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
-        .desc      = "QMP input object member '%(member)' expects '%(expected)'",
+         QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+         "QMP input object member '%(member)' expects '%(expected)'",
     },
     {
-        .error_fmt = QERR_QMP_EXTRA_MEMBER,
-        .desc      = "QMP input object member '%(member)' is unexpected",
+         QERR_QMP_EXTRA_MEMBER,
+         "QMP input object member '%(member)' is unexpected",
     },
     {
-        .error_fmt = QERR_RESET_REQUIRED,
-        .desc      = "Resetting the Virtual Machine is required",
+         QERR_RESET_REQUIRED,
+         "Resetting the Virtual Machine is required",
     },
     {
-        .error_fmt = QERR_SET_PASSWD_FAILED,
-        .desc      = "Could not set password",
+         QERR_SET_PASSWD_FAILED,
+         "Could not set password",
     },
     {
-        .error_fmt = QERR_TOO_MANY_FILES,
-        .desc      = "Too many open files",
+         QERR_TOO_MANY_FILES,
+         "Too many open files",
     },
     {
-        .error_fmt = QERR_UNDEFINED_ERROR,
-        .desc      = "An undefined error has occurred",
+         QERR_UNDEFINED_ERROR,
+         "An undefined error has occurred",
     },
     {
-        .error_fmt = QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-        .desc      = "'%(device)' uses a %(format) feature which is not "
+         QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+         "'%(device)' uses a %(format) feature which is not "
                      "supported by this qemu version: %(feature)",
     },
     {
-        .error_fmt = QERR_UNSUPPORTED,
-        .desc      = "this feature or command is not currently supported",
+         QERR_UNSUPPORTED,
+         "this feature or command is not currently supported",
     },
     {
-        .error_fmt = QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
-        .desc      = "Migration is disabled when VirtFS export path '%(path)' "
+         QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
+         "Migration is disabled when VirtFS export path '%(path)' "
                      "is mounted in the guest using mount_tag '%(tag)'",
     },
     {
-        .error_fmt = QERR_VNC_SERVER_FAILED,
-        .desc      = "Could not start VNC server on %(target)",
+         QERR_VNC_SERVER_FAILED,
+         "Could not start VNC server on %(target)",
     },
     {
-        .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
-        .desc      = "Connection can not be completed immediately",
+         QERR_SOCKET_CONNECT_IN_PROGRESS,
+         "Connection can not be completed immediately",
     },
     {
-        .error_fmt = QERR_SOCKET_CONNECT_FAILED,
-        .desc      = "Failed to connect to socket",
+         QERR_SOCKET_CONNECT_FAILED,
+         "Failed to connect to socket",
     },
     {
-        .error_fmt = QERR_SOCKET_LISTEN_FAILED,
-        .desc      = "Failed to set socket to listening mode",
+         QERR_SOCKET_LISTEN_FAILED,
+         "Failed to set socket to listening mode",
     },
     {
-        .error_fmt = QERR_SOCKET_BIND_FAILED,
-        .desc      = "Failed to bind socket",
+         QERR_SOCKET_BIND_FAILED,
+         "Failed to bind socket",
     },
     {
-        .error_fmt = QERR_SOCKET_CREATE_FAILED,
-        .desc      = "Failed to create socket",
+         QERR_SOCKET_CREATE_FAILED,
+         "Failed to create socket",
     },
     {}
 };
diff --git a/qerror.h b/qerror.h
index de8497d..f1c4917 100644
--- a/qerror.h
+++ b/qerror.h
@@ -19,8 +19,8 @@
 #include <stdarg.h>
 
 typedef struct QErrorStringTable {
-    const char *desc;
     const char *error_fmt;
+    const char *desc;
 } QErrorStringTable;
 
 typedef struct QError {
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 22/27] error, qerror: add ErrorClass argument to error functions

[Luiz Capitulino]

The new argument is added to functions qerror_report(), error_set()
and error_is_type(). qerror_report_err() is also updated to take
care of it.

However, the new argument is not used yet. It's only stored in
Error and QError.

The QERR_ macros are also changed to contain an ErrorClass value.
Thus, the value is used in all current calls to the functions
mentioned above and to also initialize qerror_table[]. Right now
that value is just a place holder.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.c  |   7 ++-
 error.h  |   5 ++-
 qerror.c |  10 +++--
 qerror.h | 147 ++++++++++++++++++++++++++++++++-------------------------------
 4 files changed, 90 insertions(+), 79 deletions(-)

diff --git a/error.c b/error.c
index b9d9b64..1c37092 100644
--- a/error.c
+++ b/error.c
@@ -14,6 +14,7 @@
 #include "error.h"
 #include "qjson.h"
 #include "qdict.h"
+#include "qapi-types.h"
 #include "error_int.h"
 #include "qerror.h"
 
@@ -21,9 +22,10 @@ struct Error
 {
     QDict *obj;
     char *msg;
+    ErrorClass err_class;
 };
 
-void error_set(Error **errp, const char *fmt, ...)
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
 {
     Error *err;
     va_list ap;
@@ -38,6 +40,7 @@ void error_set(Error **errp, const char *fmt, ...)
     err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
     va_end(ap);
     err->msg = qerror_format(fmt, err->obj);
+    err->err_class = err_class;
 
     *errp = err;
 }
@@ -73,7 +76,7 @@ void error_free(Error *err)
     }
 }
 
-bool error_is_type(Error *err, const char *fmt)
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
 {
     const char *error_class;
     char *ptr;
diff --git a/error.h b/error.h
index 592a734..28de85a 100644
--- a/error.h
+++ b/error.h
@@ -13,6 +13,7 @@
 #define ERROR_H
 
 #include "compiler.h"
+#include "qapi-types.h"
 #include <stdbool.h>
 
 /**
@@ -26,7 +27,7 @@ typedef struct Error Error;
  * Currently, qerror.h defines these error formats.  This function is not
  * meant to be used outside of QEMU.
  */
-void error_set(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4);
 
 /**
  * Returns true if an indirect pointer to an error is pointing to a valid
@@ -60,6 +61,6 @@ void error_free(Error *err);
  * Determine if an error is of a speific type (based on the qerror format).
  * Non-QEMU users should get the `class' field to identify the error type.
  */
-bool error_is_type(Error *err, const char *fmt);
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
 
 #endif
diff --git a/qerror.c b/qerror.c
index cdaaf17..85e65b8 100644
--- a/qerror.c
+++ b/qerror.c
@@ -390,13 +390,15 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
  *
  * Return strong reference.
  */
-static QError *qerror_from_info(const char *fmt, va_list *va)
+static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
+                                va_list *va)
 {
     QError *qerr;
 
     qerr = qerror_new();
     loc_save(&qerr->loc);
 
+    qerr->err_class = err_class;
     qerr->error = error_obj_from_fmt_no_fail(fmt, va);
     qerr->err_msg = qerror_format(fmt, qerr->error);
 
@@ -522,13 +524,13 @@ static void qerror_print(QError *qerror)
     QDECREF(qstring);
 }
 
-void qerror_report(const char *fmt, ...)
+void qerror_report(ErrorClass eclass, const char *fmt, ...)
 {
     va_list va;
     QError *qerror;
 
     va_start(va, fmt);
-    qerror = qerror_from_info(fmt, &va);
+    qerror = qerror_from_info(eclass, fmt, &va);
     va_end(va);
 
     if (monitor_cur_is_qmp()) {
@@ -544,6 +546,7 @@ struct Error
 {
     QDict *obj;
     char *msg;
+    ErrorClass err_class;
 };
 
 void qerror_report_err(Error *err)
@@ -555,6 +558,7 @@ void qerror_report_err(Error *err)
     QINCREF(err->obj);
     qerr->error = err->obj;
     qerr->err_msg = g_strdup(err->msg);
+    qerr->err_class = err->err_class;
 
     if (monitor_cur_is_qmp()) {
         monitor_set_error(cur_mon, qerr);
diff --git a/qerror.h b/qerror.h
index f1c4917..704a556 100644
--- a/qerror.h
+++ b/qerror.h
@@ -16,9 +16,11 @@
 #include "qstring.h"
 #include "qemu-error.h"
 #include "error.h"
+#include "qapi-types.h"
 #include <stdarg.h>
 
 typedef struct QErrorStringTable {
+    ErrorClass err_class;
     const char *error_fmt;
     const char *desc;
 } QErrorStringTable;
@@ -28,10 +30,11 @@ typedef struct QError {
     QDict *error;
     Location loc;
     char *err_msg;
+    ErrorClass err_class;
 } QError;
 
 QString *qerror_human(const QError *qerror);
-void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+void qerror_report(ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void qerror_report_err(Error *err);
 void assert_no_error(Error *err);
 char *qerror_format(const char *fmt, QDict *error);
@@ -42,217 +45,217 @@ char *qerror_format(const char *fmt, QDict *error);
  * Use scripts/check-qerror.sh to check.
  */
 #define QERR_ADD_CLIENT_FAILED \
-    "{ 'class': 'AddClientFailed', 'data': {} }"
+    -1, "{ 'class': 'AddClientFailed', 'data': {} }"
 
 #define QERR_AMBIGUOUS_PATH \
-    "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
+    -1, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
 
 #define QERR_BAD_BUS_FOR_DEVICE \
-    "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
+    -1, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
 
 #define QERR_BASE_NOT_FOUND \
-    "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
+    -1, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
 
 #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
-    "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
+    -1, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
 
 #define QERR_BUFFER_OVERRUN \
-    "{ 'class': 'BufferOverrun', 'data': {} }"
+    -1, "{ 'class': 'BufferOverrun', 'data': {} }"
 
 #define QERR_BUS_NO_HOTPLUG \
-    "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+    -1, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
 
 #define QERR_BUS_NOT_FOUND \
-    "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+    -1, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
 
 #define QERR_COMMAND_DISABLED \
-    "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
 
 #define QERR_COMMAND_NOT_FOUND \
-    "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
 
 #define QERR_DEVICE_ENCRYPTED \
-    "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
+    -1, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
 
 #define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
-    "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
+    -1, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
 
 #define QERR_DEVICE_HAS_NO_MEDIUM \
-    "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_INIT_FAILED \
-    "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_IN_USE \
-    "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_IS_READ_ONLY \
-    "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_LOCKED \
-    "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_MULTIPLE_BUSSES \
-    "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NO_BUS \
-    "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NO_HOTPLUG \
-    "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_ACTIVE \
-    "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_ENCRYPTED \
-    "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_FOUND \
-    "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_REMOVABLE \
-    "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
+    -1, "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
 
 #define QERR_DUPLICATE_ID \
-    "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
+    -1, "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
 
 #define QERR_FD_NOT_FOUND \
-    "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
 
 #define QERR_FD_NOT_SUPPLIED \
-    "{ 'class': 'FdNotSupplied', 'data': {} }"
+    -1, "{ 'class': 'FdNotSupplied', 'data': {} }"
 
 #define QERR_FEATURE_DISABLED \
-    "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
 
 #define QERR_INVALID_BLOCK_FORMAT \
-    "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
 
 #define QERR_INVALID_OPTION_GROUP \
-    "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
+    -1, "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
 
 #define QERR_INVALID_PARAMETER \
-    "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
 
 #define QERR_INVALID_PARAMETER_COMBINATION \
-    "{ 'class': 'InvalidParameterCombination', 'data': {} }"
+    -1, "{ 'class': 'InvalidParameterCombination', 'data': {} }"
 
 #define QERR_INVALID_PARAMETER_TYPE \
-    "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
+    -1, "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
 
 #define QERR_INVALID_PARAMETER_VALUE \
-    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
+    -1, "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
 
 #define QERR_INVALID_PASSWORD \
-    "{ 'class': 'InvalidPassword', 'data': {} }"
+    -1, "{ 'class': 'InvalidPassword', 'data': {} }"
 
 #define QERR_IO_ERROR \
-    "{ 'class': 'IOError', 'data': {} }"
+    -1, "{ 'class': 'IOError', 'data': {} }"
 
 #define QERR_JSON_PARSE_ERROR \
-    "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
+    -1, "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
 
 #define QERR_JSON_PARSING \
-    "{ 'class': 'JSONParsing', 'data': {} }"
+    -1, "{ 'class': 'JSONParsing', 'data': {} }"
 
 #define QERR_KVM_MISSING_CAP \
-    "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
+    -1, "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
 
 #define QERR_MIGRATION_ACTIVE \
-    "{ 'class': 'MigrationActive', 'data': {} }"
+    -1, "{ 'class': 'MigrationActive', 'data': {} }"
 
 #define QERR_MIGRATION_NOT_SUPPORTED \
-    "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
+    -1, "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
 
 #define QERR_MIGRATION_EXPECTED \
-    "{ 'class': 'MigrationExpected', 'data': {} }"
+    -1, "{ 'class': 'MigrationExpected', 'data': {} }"
 
 #define QERR_MISSING_PARAMETER \
-    "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
+    -1, "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
 
 #define QERR_NO_BUS_FOR_DEVICE \
-    "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
+    -1, "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
 
 #define QERR_NOT_SUPPORTED \
-    "{ 'class': 'NotSupported', 'data': {} }"
+    -1, "{ 'class': 'NotSupported', 'data': {} }"
 
 #define QERR_OPEN_FILE_FAILED \
-    "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
+    -1, "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
 
 #define QERR_PERMISSION_DENIED \
-    "{ 'class': 'PermissionDenied', 'data': {} }"
+    -1, "{ 'class': 'PermissionDenied', 'data': {} }"
 
 #define QERR_PROPERTY_NOT_FOUND \
-    "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
+    -1, "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
 
 #define QERR_PROPERTY_VALUE_BAD \
-    "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    -1, "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
 #define QERR_PROPERTY_VALUE_IN_USE \
-    "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    -1, "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
 #define QERR_PROPERTY_VALUE_NOT_FOUND \
-    "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    -1, "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
 #define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \
-    "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
+    -1, "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
     "'device': %s, 'property': %s, 'value': %"PRId64" } }"
 
 #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
-    "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
+    -1, "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
 
 #define QERR_QGA_COMMAND_FAILED \
-    "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
+    -1, "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
 
 #define QERR_QGA_LOGGING_FAILED \
-    "{ 'class': 'QgaLoggingFailed', 'data': {} }"
+    -1, "{ 'class': 'QgaLoggingFailed', 'data': {} }"
 
 #define QERR_QMP_BAD_INPUT_OBJECT \
-    "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
+    -1, "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
 
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
-    "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
+    -1, "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
 
 #define QERR_QMP_EXTRA_MEMBER \
-    "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
+    -1, "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
 
 #define QERR_RESET_REQUIRED \
-    "{ 'class': 'ResetRequired', 'data': {} }"
+    -1, "{ 'class': 'ResetRequired', 'data': {} }"
 
 #define QERR_SET_PASSWD_FAILED \
-    "{ 'class': 'SetPasswdFailed', 'data': {} }"
+    -1, "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
 #define QERR_TOO_MANY_FILES \
-    "{ 'class': 'TooManyFiles', 'data': {} }"
+    -1, "{ 'class': 'TooManyFiles', 'data': {} }"
 
 #define QERR_UNDEFINED_ERROR \
-    "{ 'class': 'UndefinedError', 'data': {} }"
+    -1, "{ 'class': 'UndefinedError', 'data': {} }"
 
 #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
-    "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
+    -1, "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
 
 #define QERR_UNSUPPORTED \
-    "{ 'class': 'Unsupported', 'data': {} }"
+    -1, "{ 'class': 'Unsupported', 'data': {} }"
 
 #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
-    "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
+    -1, "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
 
 #define QERR_VNC_SERVER_FAILED \
-    "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
+    -1, "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
 
 #define QERR_SOCKET_CONNECT_IN_PROGRESS \
-    "{ 'class': 'SockConnectInprogress', 'data': {} }"
+    -1, "{ 'class': 'SockConnectInprogress', 'data': {} }"
 
 #define QERR_SOCKET_CONNECT_FAILED \
-    "{ 'class': 'SockConnectFailed', 'data': {} }"
+    -1, "{ 'class': 'SockConnectFailed', 'data': {} }"
 
 #define QERR_SOCKET_LISTEN_FAILED \
-    "{ 'class': 'SockListenFailed', 'data': {} }"
+    -1, "{ 'class': 'SockListenFailed', 'data': {} }"
 
 #define QERR_SOCKET_BIND_FAILED \
-    "{ 'class': 'SockBindFailed', 'data': {} }"
+    -1, "{ 'class': 'SockBindFailed', 'data': {} }"
 
 #define QERR_SOCKET_CREATE_FAILED \
-    "{ 'class': 'SockCreateFailed', 'data': {} }"
+    -1, "{ 'class': 'SockCreateFailed', 'data': {} }"
 
 #endif /* QERROR_H */
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 23/27] qerror: use ErrorClass for QERR_ macro

[Luiz Capitulino]

This commit replaces the place holder value for the ErrorClass
argument being passed to all current error calls to the real
ErrorClass value.

That is, all current errors are mapped to GenericError, except the
following errors: CommandNotFound, DeviceNotActive, DeviceNotFound,
KVMMissingCap, MigrationExpected; which are maintained as they are.

Note that the ErrorClass value is still unused.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.h | 142 +++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/qerror.h b/qerror.h
index 704a556..14b9bfc 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,217 +45,217 @@ char *qerror_format(const char *fmt, QDict *error);
  * Use scripts/check-qerror.sh to check.
  */
 #define QERR_ADD_CLIENT_FAILED \
-    -1, "{ 'class': 'AddClientFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
 
 #define QERR_AMBIGUOUS_PATH \
-    -1, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
 
 #define QERR_BAD_BUS_FOR_DEVICE \
-    -1, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
 
 #define QERR_BASE_NOT_FOUND \
-    -1, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
 
 #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
-    -1, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
 
 #define QERR_BUFFER_OVERRUN \
-    -1, "{ 'class': 'BufferOverrun', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
 
 #define QERR_BUS_NO_HOTPLUG \
-    -1, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
 
 #define QERR_BUS_NOT_FOUND \
-    -1, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
 
 #define QERR_COMMAND_DISABLED \
-    -1, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
 
 #define QERR_COMMAND_NOT_FOUND \
-    -1, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
+    ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
 
 #define QERR_DEVICE_ENCRYPTED \
-    -1, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
 
 #define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
-    -1, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
 
 #define QERR_DEVICE_HAS_NO_MEDIUM \
-    -1, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_INIT_FAILED \
-    -1, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_IN_USE \
-    -1, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_IS_READ_ONLY \
-    -1, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_LOCKED \
-    -1, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_MULTIPLE_BUSSES \
-    -1, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NO_BUS \
-    -1, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NO_HOTPLUG \
-    -1, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_ACTIVE \
-    -1, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
+    ERROR_CLASS_DEVICE_NOT_ACTIVE, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_ENCRYPTED \
-    -1, "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_FOUND \
-    -1, "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
+    ERROR_CLASS_DEVICE_NOT_FOUND, "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
 
 #define QERR_DEVICE_NOT_REMOVABLE \
-    -1, "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
 
 #define QERR_DUPLICATE_ID \
-    -1, "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
 
 #define QERR_FD_NOT_FOUND \
-    -1, "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
 
 #define QERR_FD_NOT_SUPPLIED \
-    -1, "{ 'class': 'FdNotSupplied', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotSupplied', 'data': {} }"
 
 #define QERR_FEATURE_DISABLED \
-    -1, "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
 
 #define QERR_INVALID_BLOCK_FORMAT \
-    -1, "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
 
 #define QERR_INVALID_OPTION_GROUP \
-    -1, "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
 
 #define QERR_INVALID_PARAMETER \
-    -1, "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
 
 #define QERR_INVALID_PARAMETER_COMBINATION \
-    -1, "{ 'class': 'InvalidParameterCombination', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterCombination', 'data': {} }"
 
 #define QERR_INVALID_PARAMETER_TYPE \
-    -1, "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
 
 #define QERR_INVALID_PARAMETER_VALUE \
-    -1, "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
 
 #define QERR_INVALID_PASSWORD \
-    -1, "{ 'class': 'InvalidPassword', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidPassword', 'data': {} }"
 
 #define QERR_IO_ERROR \
-    -1, "{ 'class': 'IOError', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'IOError', 'data': {} }"
 
 #define QERR_JSON_PARSE_ERROR \
-    -1, "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
 
 #define QERR_JSON_PARSING \
-    -1, "{ 'class': 'JSONParsing', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParsing', 'data': {} }"
 
 #define QERR_KVM_MISSING_CAP \
-    -1, "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
+    ERROR_CLASS_K_V_M_MISSING_CAP, "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
 
 #define QERR_MIGRATION_ACTIVE \
-    -1, "{ 'class': 'MigrationActive', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationActive', 'data': {} }"
 
 #define QERR_MIGRATION_NOT_SUPPORTED \
-    -1, "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
 
 #define QERR_MIGRATION_EXPECTED \
-    -1, "{ 'class': 'MigrationExpected', 'data': {} }"
+    ERROR_CLASS_MIGRATION_EXPECTED, "{ 'class': 'MigrationExpected', 'data': {} }"
 
 #define QERR_MISSING_PARAMETER \
-    -1, "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
 
 #define QERR_NO_BUS_FOR_DEVICE \
-    -1, "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
 
 #define QERR_NOT_SUPPORTED \
-    -1, "{ 'class': 'NotSupported', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NotSupported', 'data': {} }"
 
 #define QERR_OPEN_FILE_FAILED \
-    -1, "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
 
 #define QERR_PERMISSION_DENIED \
-    -1, "{ 'class': 'PermissionDenied', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PermissionDenied', 'data': {} }"
 
 #define QERR_PROPERTY_NOT_FOUND \
-    -1, "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
 
 #define QERR_PROPERTY_VALUE_BAD \
-    -1, "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
 #define QERR_PROPERTY_VALUE_IN_USE \
-    -1, "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
 #define QERR_PROPERTY_VALUE_NOT_FOUND \
-    -1, "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
 #define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \
-    -1, "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
     "'device': %s, 'property': %s, 'value': %"PRId64" } }"
 
 #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
-    -1, "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
 
 #define QERR_QGA_COMMAND_FAILED \
-    -1, "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
 
 #define QERR_QGA_LOGGING_FAILED \
-    -1, "{ 'class': 'QgaLoggingFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaLoggingFailed', 'data': {} }"
 
 #define QERR_QMP_BAD_INPUT_OBJECT \
-    -1, "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
 
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
-    -1, "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
 
 #define QERR_QMP_EXTRA_MEMBER \
-    -1, "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
 
 #define QERR_RESET_REQUIRED \
-    -1, "{ 'class': 'ResetRequired', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'ResetRequired', 'data': {} }"
 
 #define QERR_SET_PASSWD_FAILED \
-    -1, "{ 'class': 'SetPasswdFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
 #define QERR_TOO_MANY_FILES \
-    -1, "{ 'class': 'TooManyFiles', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'TooManyFiles', 'data': {} }"
 
 #define QERR_UNDEFINED_ERROR \
-    -1, "{ 'class': 'UndefinedError', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UndefinedError', 'data': {} }"
 
 #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
-    -1, "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
 
 #define QERR_UNSUPPORTED \
-    -1, "{ 'class': 'Unsupported', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'Unsupported', 'data': {} }"
 
 #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
-    -1, "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
 
 #define QERR_VNC_SERVER_FAILED \
-    -1, "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
 
 #define QERR_SOCKET_CONNECT_IN_PROGRESS \
-    -1, "{ 'class': 'SockConnectInprogress', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectInprogress', 'data': {} }"
 
 #define QERR_SOCKET_CONNECT_FAILED \
-    -1, "{ 'class': 'SockConnectFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectFailed', 'data': {} }"
 
 #define QERR_SOCKET_LISTEN_FAILED \
-    -1, "{ 'class': 'SockListenFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockListenFailed', 'data': {} }"
 
 #define QERR_SOCKET_BIND_FAILED \
-    -1, "{ 'class': 'SockBindFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockBindFailed', 'data': {} }"
 
 #define QERR_SOCKET_CREATE_FAILED \
-    -1, "{ 'class': 'SockCreateFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockCreateFailed', 'data': {} }"
 
 #endif /* QERROR_H */
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 24/27] qmp: switch to the new error format on the wire

[Luiz Capitulino]

IMPORTANT: this commit BREAKS QMP's compatibility for the error response.

This commit changes QMP's wire protocol to make use of the simpler
error format introduced by previous commits.

There are two important (and mostly incompatible) changes:

 1. Almost all error classes have been replaced by GenericError. The
    only classes that are still supported for compatibility with
    libvirt are: CommandNotFound, DeviceNotActive, KVMMissingCap,
    DeviceNotFound and MigrationExpected

 2. The 'data' field of the error dictionary is gone

As an example, an error response like:

  { "error": { "class": "DeviceNotRemovable",
               "data": { "device": "virtio0" },
               "desc": "Device 'virtio0' is not removable" } }

Will now be emitted as:

  { "error": { "class": "GenericError",
               "desc": "Device 'virtio0' is not removable" } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-spec.txt | 10 +++-------
 monitor.c        | 18 +++++++++++++-----
 qmp-commands.hx  |  3 +--
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 1ba916c..a277896 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -106,14 +106,11 @@ completed because of an error condition.
 
 The format is:
 
-{ "error": { "class": json-string, "data": json-object, "desc": json-string },
-  "id": json-value }
+{ "error": { "class": json-string, "desc": json-string }, "id": json-value }
 
  Where,
 
-- The "class" member contains the error class name (eg. "ServiceUnavailable")
-- The "data" member contains specific error data and is defined in a
-  per-command basis, it will be an empty json-object if the error has no data
+- The "class" member contains the error class name (eg. "GenericError")
 - The "desc" member is a human-readable error message. Clients should
   not attempt to parse this message.
 - The "id" member contains the transaction identification associated with
@@ -173,8 +170,7 @@ S: {"return": {"enabled": true, "present": true}, "id": "example"}
 ------------------
 
 C: { "execute": }
-S: {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data":
-{}}}
+S: {"error": {"class": "GenericError", "desc": "Invalid JSON syntax" } }
 
 3.5 Powerdown event
 -------------------
diff --git a/monitor.c b/monitor.c
index 4eeb0c5..8b00b3b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -356,16 +356,26 @@ static void monitor_json_emitter(Monitor *mon, const QObject *data)
     QDECREF(json);
 }
 
+static QDict *build_qmp_error_dict(const QError *err)
+{
+    QObject *obj;
+
+    obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }",
+                             ErrorClass_lookup[err->err_class],
+                             qerror_human(err));
+
+    return qobject_to_qdict(obj);
+}
+
 static void monitor_protocol_emitter(Monitor *mon, QObject *data)
 {
     QDict *qmp;
 
     trace_monitor_protocol_emitter(mon);
 
-    qmp = qdict_new();
-
     if (!monitor_has_error(mon)) {
         /* success response */
+        qmp = qdict_new();
         if (data) {
             qobject_incref(data);
             qdict_put_obj(qmp, "return", data);
@@ -375,9 +385,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data)
         }
     } else {
         /* error response */
-        qdict_put(mon->error->error, "desc", qerror_human(mon->error));
-        qdict_put(qmp, "error", mon->error->error);
-        QINCREF(mon->error->error);
+        qmp = build_qmp_error_dict(mon->error);
         QDECREF(mon->error);
         mon->error = NULL;
     }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e3cf3c5..e520b51 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -435,8 +435,7 @@ Example:
 -> { "execute": "inject-nmi" }
 <- { "return": {} }
 
-Note: inject-nmi is only supported for x86 guest currently, it will
-      returns "Unsupported" error for non-x86 guest.
+Note: inject-nmi is only supported for x86 guests.
 
 EQMP
 
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 25/27] qapi: qapi.py: allow the "'" character be escaped

[Luiz Capitulino]

A future commit will add a new qapi script which escapes that character.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e062336..9aa518f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -21,7 +21,9 @@ def tokenize(data):
         elif data[0] == "'":
             data = data[1:]
             string = ''
-            while data[0] != "'":
+            while True:
+                if data[0] == "'" and string[len(string)-1] != "\\":
+                    break
                 string += data[0]
                 data = data[1:]
             data = data[1:]
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 26/27] error, qerror: pass desc string to error calls

[Luiz Capitulino]

FIXME: broke error_is_type() and qemu-ga.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.c  |   5 ++-
 qerror.c |  44 ++------------------
 qerror.h | 143 +++++++++++++++++++++++++++++++--------------------------------
 3 files changed, 77 insertions(+), 115 deletions(-)

diff --git a/error.c b/error.c
index 1c37092..dd20ab8 100644
--- a/error.c
+++ b/error.c
@@ -29,6 +29,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
 {
     Error *err;
     va_list ap;
+    char msg[2048];
 
     if (errp == NULL) {
         return;
@@ -37,9 +38,9 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     err = g_malloc0(sizeof(*err));
 
     va_start(ap, fmt);
-    err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
+    vsnprintf(msg, sizeof(msg), fmt, ap);
     va_end(ap);
-    err->msg = qerror_format(fmt, err->obj);
+    err->msg = g_strdup(msg);
     err->err_class = err_class;
 
     *errp = err;
diff --git a/qerror.c b/qerror.c
index 85e65b8..f443261 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,45 +346,6 @@ static QError *qerror_new(void)
     return qerr;
 }
 
-static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
-{
-    QObject *obj;
-    QDict *ret;
-
-    obj = qobject_from_jsonv(fmt, va);
-    if (!obj) {
-        fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
-        abort();
-    }
-    if (qobject_type(obj) != QTYPE_QDICT) {
-        fprintf(stderr, "error is not a dict '%s'\n", fmt);
-        abort();
-    }
-
-    ret = qobject_to_qdict(obj);
-    obj = qdict_get(ret, "class");
-    if (!obj) {
-        fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
-        abort();
-    }
-    if (qobject_type(obj) != QTYPE_QSTRING) {
-        fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
-        abort();
-    }
-
-    obj = qdict_get(ret, "data");
-    if (!obj) {
-        fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
-        abort();
-    }
-    if (qobject_type(obj) != QTYPE_QDICT) {
-        fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
-        abort();
-    }
-
-    return ret;
-}
-
 /**
  * qerror_from_info(): Create a new QError from error information
  *
@@ -394,13 +355,14 @@ static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
                                 va_list *va)
 {
     QError *qerr;
+    char msg[2048];
 
     qerr = qerror_new();
     loc_save(&qerr->loc);
 
     qerr->err_class = err_class;
-    qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-    qerr->err_msg = qerror_format(fmt, qerr->error);
+    vsnprintf(msg, sizeof(msg), fmt, *va);
+    qerr->err_msg = g_strdup(msg);
 
     return qerr;
 }
diff --git a/qerror.h b/qerror.h
index 14b9bfc..92a4b83 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,217 +45,216 @@ char *qerror_format(const char *fmt, QDict *error);
  * Use scripts/check-qerror.sh to check.
  */
 #define QERR_ADD_CLIENT_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Could not add client"
 
 #define QERR_AMBIGUOUS_PATH \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"
 
 #define QERR_BAD_BUS_FOR_DEVICE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus"
 
 #define QERR_BASE_NOT_FOUND \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
 
 #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
 
 #define QERR_BUFFER_OVERRUN \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "An internal buffer overran"
 
 #define QERR_BUS_NO_HOTPLUG \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Bus '%s' does not support hotplugging"
 
 #define QERR_BUS_NOT_FOUND \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Bus '%s' not found"
 
 #define QERR_COMMAND_DISABLED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "The command %s has been disabled for this instance"
 
 #define QERR_COMMAND_NOT_FOUND \
-    ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
+    ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found"
 
 #define QERR_DEVICE_ENCRYPTED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is encrypted (filename=%s)"
 
 #define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when using feature '%s' in device '%s'"
 
 #define QERR_DEVICE_HAS_NO_MEDIUM \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no medium"
 
 #define QERR_DEVICE_INIT_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' could not be initialized"
 
 #define QERR_DEVICE_IN_USE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
 
 #define QERR_DEVICE_IS_READ_ONLY \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
 
 #define QERR_DEVICE_LOCKED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is locked"
 
 #define QERR_DEVICE_MULTIPLE_BUSSES \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has multiple child busses"
 
 #define QERR_DEVICE_NO_BUS \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no child bus"
 
 #define QERR_DEVICE_NO_HOTPLUG \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
 
 #define QERR_DEVICE_NOT_ACTIVE \
-    ERROR_CLASS_DEVICE_NOT_ACTIVE, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
+    ERROR_CLASS_DEVICE_NOT_ACTIVE, "Device '%s' has not been activated"
 
 #define QERR_DEVICE_NOT_ENCRYPTED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not encrypted"
 
 #define QERR_DEVICE_NOT_FOUND \
-    ERROR_CLASS_DEVICE_NOT_FOUND, "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
+    ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found"
 
 #define QERR_DEVICE_NOT_REMOVABLE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not removable"
 
 #define QERR_DUPLICATE_ID \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Duplicate ID '%s' for %s"
 
 #define QERR_FD_NOT_FOUND \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "File descriptor named '%s' not found"
 
 #define QERR_FD_NOT_SUPPLIED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotSupplied', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "No file descriptor supplied via SCM_RIGHTS"
 
 #define QERR_FEATURE_DISABLED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
 
 #define QERR_INVALID_BLOCK_FORMAT \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Invalid block format '%s'"
 
 #define QERR_INVALID_OPTION_GROUP \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "There is no option group '%s'"
 
 #define QERR_INVALID_PARAMETER \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter '%s'"
 
 #define QERR_INVALID_PARAMETER_COMBINATION \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterCombination', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter combination"
 
 #define QERR_INVALID_PARAMETER_TYPE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter type for '%s', expected: %s"
 
 #define QERR_INVALID_PARAMETER_VALUE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' expects %s"
 
 #define QERR_INVALID_PASSWORD \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidPassword', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Password incorrect"
 
 #define QERR_IO_ERROR \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'IOError', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "An IO error has occurred"
 
 #define QERR_JSON_PARSE_ERROR \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "JSON parse error, %s"
 
 #define QERR_JSON_PARSING \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParsing', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
 
 #define QERR_KVM_MISSING_CAP \
-    ERROR_CLASS_K_V_M_MISSING_CAP, "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
+    ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
 
 #define QERR_MIGRATION_ACTIVE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationActive', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
 
 #define QERR_MIGRATION_NOT_SUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
+    ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device '%s'"
 
 #define QERR_MIGRATION_EXPECTED \
-    ERROR_CLASS_MIGRATION_EXPECTED, "{ 'class': 'MigrationExpected', 'data': {} }"
+    ERROR_CLASS_MIGRATION_EXPECTED, "An incoming migration is expected before this command can be executed"
 
 #define QERR_MISSING_PARAMETER \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
 
 #define QERR_NO_BUS_FOR_DEVICE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "No '%s' bus found for device '%s'"
 
 #define QERR_NOT_SUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NotSupported', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Not supported"
 
 #define QERR_OPEN_FILE_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'"
 
 #define QERR_PERMISSION_DENIED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PermissionDenied', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"
 
 #define QERR_PROPERTY_NOT_FOUND \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' not found"
 
 #define QERR_PROPERTY_VALUE_BAD \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' doesn't take value '%s'"
 
 #define QERR_PROPERTY_VALUE_IN_USE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' can't take value '%s', it's in use"
 
 #define QERR_PROPERTY_VALUE_NOT_FOUND \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' can't find value '%s'"
 
 #define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
-    "'device': %s, 'property': %s, 'value': %"PRId64" } }"
+    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value '%" PRId64 "', it's not a power of 2"
 
 #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
+    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")"
 
 #define QERR_QGA_COMMAND_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Guest agent command failed, error was '%s'"
 
 #define QERR_QGA_LOGGING_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaLoggingFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Guest agent failed to log non-optional log statement"
 
 #define QERR_QMP_BAD_INPUT_OBJECT \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Expected '%s' in QMP input"
 
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' expects '%s'"
 
 #define QERR_QMP_EXTRA_MEMBER \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' is unexpected"
 
 #define QERR_RESET_REQUIRED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'ResetRequired', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Resetting the Virtual Machine is required"
 
 #define QERR_SET_PASSWD_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SetPasswdFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Could not set password"
 
 #define QERR_TOO_MANY_FILES \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'TooManyFiles', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Too many open files"
 
 #define QERR_UNDEFINED_ERROR \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UndefinedError', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "An undefined error has occurred"
 
 #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "'%s' uses a %s feature which is not supported by this qemu version: %s"
 
 #define QERR_UNSUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'Unsupported', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "this feature or command is not currently supported"
 
 #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'"
 
 #define QERR_VNC_SERVER_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
+    ERROR_CLASS_GENERIC_ERROR, "Could not start VNC server on %s"
 
 #define QERR_SOCKET_CONNECT_IN_PROGRESS \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectInprogress', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Connection can not be completed immediately"
 
 #define QERR_SOCKET_CONNECT_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
 
 #define QERR_SOCKET_LISTEN_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockListenFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Failed to set socket to listening mode"
 
 #define QERR_SOCKET_BIND_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockBindFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Failed to bind socket"
 
 #define QERR_SOCKET_CREATE_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockCreateFailed', 'data': {} }"
+    ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
 
 #endif /* QERROR_H */
-- 
1.7.11.2.249.g31c7954.dirty

[Qemu-devel] [PATCH 27/27] qerror: drop qerror_table and qerror_format()

[Luiz Capitulino]

They not used anymore.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c | 404 ---------------------------------------------------------------
 qerror.h |   1 -
 2 files changed, 405 deletions(-)

diff --git a/qerror.c b/qerror.c
index f443261..e1606d3 100644
--- a/qerror.c
+++ b/qerror.c
@@ -23,315 +23,6 @@ static const QType qerror_type = {
 };
 
 /**
- * The 'desc' parameter is a printf-like string, the format of the format
- * string is:
- *
- * %(KEY)
- *
- * Where KEY is a QDict key, which has to be passed to qerror_from_info().
- *
- * Example:
- *
- * "foo error on device: %(device) slot: %(slot_nr)"
- *
- * A single percent sign can be printed if followed by a second one,
- * for example:
- *
- * "running out of foo: %(foo)%%"
- *
- * Please keep the entries in alphabetical order.
- * Use scripts/check-qerror.sh to check.
- */
-static const QErrorStringTable qerror_table[] = {
-    {
-         QERR_ADD_CLIENT_FAILED,
-         "Could not add client",
-    },
-    {
-         QERR_AMBIGUOUS_PATH,
-         "Path '%(path)' does not uniquely identify an object"
-    },
-    {
-         QERR_BAD_BUS_FOR_DEVICE,
-         "Device '%(device)' can't go on a %(bad_bus_type) bus",
-    },
-    {
-         QERR_BASE_NOT_FOUND,
-         "Base '%(base)' not found",
-    },
-    {
-         QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-         "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
-    },
-    {
-         QERR_BUS_NO_HOTPLUG,
-         "Bus '%(bus)' does not support hotplugging",
-    },
-    {
-         QERR_BUS_NOT_FOUND,
-         "Bus '%(bus)' not found",
-    },
-    {
-         QERR_COMMAND_DISABLED,
-         "The command %(name) has been disabled for this instance",
-    },
-    {
-         QERR_COMMAND_NOT_FOUND,
-         "The command %(name) has not been found",
-    },
-    {
-         QERR_DEVICE_ENCRYPTED,
-         "Device '%(device)' is encrypted (filename is '%(filename)')",
-    },
-    {
-         QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-         "Migration is disabled when using feature '%(feature)' in device '%(device)'",
-    },
-    {
-         QERR_DEVICE_HAS_NO_MEDIUM,
-         "Device '%(device)' has no medium",
-    },
-    {
-         QERR_DEVICE_INIT_FAILED,
-         "Device '%(device)' could not be initialized",
-    },
-    {
-         QERR_DEVICE_IN_USE,
-         "Device '%(device)' is in use",
-    },
-    {
-         QERR_DEVICE_IS_READ_ONLY,
-         "Device '%(device)' is read only",
-    },
-    {
-         QERR_DEVICE_LOCKED,
-         "Device '%(device)' is locked",
-    },
-    {
-         QERR_DEVICE_MULTIPLE_BUSSES,
-         "Device '%(device)' has multiple child busses",
-    },
-    {
-         QERR_DEVICE_NO_BUS,
-         "Device '%(device)' has no child bus",
-    },
-    {
-         QERR_DEVICE_NO_HOTPLUG,
-         "Device '%(device)' does not support hotplugging",
-    },
-    {
-         QERR_DEVICE_NOT_ACTIVE,
-         "Device '%(device)' has not been activated",
-    },
-    {
-         QERR_DEVICE_NOT_ENCRYPTED,
-         "Device '%(device)' is not encrypted",
-    },
-    {
-         QERR_DEVICE_NOT_FOUND,
-         "Device '%(device)' not found",
-    },
-    {
-         QERR_DEVICE_NOT_REMOVABLE,
-         "Device '%(device)' is not removable",
-    },
-    {
-         QERR_DUPLICATE_ID,
-         "Duplicate ID '%(id)' for %(object)",
-    },
-    {
-         QERR_FD_NOT_FOUND,
-         "File descriptor named '%(name)' not found",
-    },
-    {
-         QERR_FD_NOT_SUPPLIED,
-         "No file descriptor supplied via SCM_RIGHTS",
-    },
-    {
-         QERR_FEATURE_DISABLED,
-         "The feature '%(name)' is not enabled",
-    },
-    {
-         QERR_INVALID_BLOCK_FORMAT,
-         "Invalid block format '%(name)'",
-    },
-    {
-         QERR_INVALID_OPTION_GROUP,
-         "There is no option group '%(group)'",
-    },
-    {
-         QERR_INVALID_PARAMETER,
-         "Invalid parameter '%(name)'",
-    },
-    {
-         QERR_INVALID_PARAMETER_COMBINATION,
-         "Invalid parameter combination",
-    },
-    {
-         QERR_INVALID_PARAMETER_TYPE,
-         "Invalid parameter type for '%(name)', expected: %(expected)",
-    },
-    {
-         QERR_INVALID_PARAMETER_VALUE,
-         "Parameter '%(name)' expects %(expected)",
-    },
-    {
-         QERR_INVALID_PASSWORD,
-         "Password incorrect",
-    },
-    {
-         QERR_IO_ERROR,
-         "An IO error has occurred",
-    },
-    {
-         QERR_JSON_PARSE_ERROR,
-         "JSON parse error, %(message)",
-
-    },
-    {
-         QERR_JSON_PARSING,
-         "Invalid JSON syntax",
-    },
-    {
-         QERR_KVM_MISSING_CAP,
-         "Using KVM without %(capability), %(feature) unavailable",
-    },
-    {
-         QERR_MIGRATION_ACTIVE,
-         "There's a migration process in progress",
-    },
-    {
-         QERR_MIGRATION_NOT_SUPPORTED,
-         "State blocked by non-migratable device '%(device)'",
-    },
-    {
-         QERR_MIGRATION_EXPECTED,
-         "An incoming migration is expected before this command can be executed",
-    },
-    {
-         QERR_MISSING_PARAMETER,
-         "Parameter '%(name)' is missing",
-    },
-    {
-         QERR_NO_BUS_FOR_DEVICE,
-         "No '%(bus)' bus found for device '%(device)'",
-    },
-    {
-         QERR_NOT_SUPPORTED,
-         "Not supported",
-    },
-    {
-         QERR_OPEN_FILE_FAILED,
-         "Could not open '%(filename)'",
-    },
-    {
-         QERR_PERMISSION_DENIED,
-         "Insufficient permission to perform this operation",
-    },
-    {
-         QERR_PROPERTY_NOT_FOUND,
-         "Property '%(device).%(property)' not found",
-    },
-    {
-         QERR_PROPERTY_VALUE_BAD,
-         "Property '%(device).%(property)' doesn't take value '%(value)'",
-    },
-    {
-         QERR_PROPERTY_VALUE_IN_USE,
-         "Property '%(device).%(property)' can't take value '%(value)', it's in use",
-    },
-    {
-         QERR_PROPERTY_VALUE_NOT_FOUND,
-         "Property '%(device).%(property)' can't find value '%(value)'",
-    },
-    {
-         QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
-         "Property '%(device).%(property)' doesn't take "
-                     "value '%(value)', it's not a power of 2",
-    },
-    {
-         QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-         "Property '%(device).%(property)' doesn't take "
-                     "value %(value) (minimum: %(min), maximum: %(max))",
-    },
-    {
-         QERR_QGA_COMMAND_FAILED,
-         "Guest agent command failed, error was '%(message)'",
-    },
-    {
-         QERR_QGA_LOGGING_FAILED,
-         "Guest agent failed to log non-optional log statement",
-    },
-    {
-         QERR_QMP_BAD_INPUT_OBJECT,
-         "Expected '%(expected)' in QMP input",
-    },
-    {
-         QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
-         "QMP input object member '%(member)' expects '%(expected)'",
-    },
-    {
-         QERR_QMP_EXTRA_MEMBER,
-         "QMP input object member '%(member)' is unexpected",
-    },
-    {
-         QERR_RESET_REQUIRED,
-         "Resetting the Virtual Machine is required",
-    },
-    {
-         QERR_SET_PASSWD_FAILED,
-         "Could not set password",
-    },
-    {
-         QERR_TOO_MANY_FILES,
-         "Too many open files",
-    },
-    {
-         QERR_UNDEFINED_ERROR,
-         "An undefined error has occurred",
-    },
-    {
-         QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-         "'%(device)' uses a %(format) feature which is not "
-                     "supported by this qemu version: %(feature)",
-    },
-    {
-         QERR_UNSUPPORTED,
-         "this feature or command is not currently supported",
-    },
-    {
-         QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
-         "Migration is disabled when VirtFS export path '%(path)' "
-                     "is mounted in the guest using mount_tag '%(tag)'",
-    },
-    {
-         QERR_VNC_SERVER_FAILED,
-         "Could not start VNC server on %(target)",
-    },
-    {
-         QERR_SOCKET_CONNECT_IN_PROGRESS,
-         "Connection can not be completed immediately",
-    },
-    {
-         QERR_SOCKET_CONNECT_FAILED,
-         "Failed to connect to socket",
-    },
-    {
-         QERR_SOCKET_LISTEN_FAILED,
-         "Failed to set socket to listening mode",
-    },
-    {
-         QERR_SOCKET_BIND_FAILED,
-         "Failed to bind socket",
-    },
-    {
-         QERR_SOCKET_CREATE_FAILED,
-         "Failed to create socket",
-    },
-    {}
-};
-
-/**
  * qerror_new(): Create a new QError
  *
  * Return strong reference.
@@ -367,101 +58,6 @@ static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
     return qerr;
 }
 
-static void parse_error(const QErrorStringTable *entry, int c)
-{
-    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
-    abort();
-}
-
-static const char *append_field(QDict *error, QString *outstr,
-                                const QErrorStringTable *entry,
-                                const char *start)
-{
-    QObject *obj;
-    QDict *qdict;
-    QString *key_qs;
-    const char *end, *key;
-
-    if (*start != '%')
-        parse_error(entry, '%');
-    start++;
-    if (*start != '(')
-        parse_error(entry, '(');
-    start++;
-
-    end = strchr(start, ')');
-    if (!end)
-        parse_error(entry, ')');
-
-    key_qs = qstring_from_substr(start, 0, end - start - 1);
-    key = qstring_get_str(key_qs);
-
-    qdict = qobject_to_qdict(qdict_get(error, "data"));
-    obj = qdict_get(qdict, key);
-    if (!obj) {
-        abort();
-    }
-
-    switch (qobject_type(obj)) {
-        case QTYPE_QSTRING:
-            qstring_append(outstr, qdict_get_str(qdict, key));
-            break;
-        case QTYPE_QINT:
-            qstring_append_int(outstr, qdict_get_int(qdict, key));
-            break;
-        default:
-            abort();
-    }
-
-    QDECREF(key_qs);
-    return ++end;
-}
-
-static QString *qerror_format_desc(QDict *error,
-                                   const QErrorStringTable *entry)
-{
-    QString *qstring;
-    const char *p;
-
-    assert(entry != NULL);
-
-    qstring = qstring_new();
-
-    for (p = entry->desc; *p != '\0';) {
-        if (*p != '%') {
-            qstring_append_chr(qstring, *p++);
-        } else if (*(p + 1) == '%') {
-            qstring_append_chr(qstring, '%');
-            p += 2;
-        } else {
-            p = append_field(error, qstring, entry, p);
-        }
-    }
-
-    return qstring;
-}
-
-char *qerror_format(const char *fmt, QDict *error)
-{
-    const QErrorStringTable *entry = NULL;
-    QString *qstr;
-    char *ret;
-    int i;
-
-    for (i = 0; qerror_table[i].error_fmt; i++) {
-        if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-            entry = &qerror_table[i];
-            break;
-        }
-    }
-
-    qstr = qerror_format_desc(error, entry);
-    ret = g_strdup(qstring_get_str(qstr));
-    QDECREF(qstr);
-
-    return ret;
-}
-
 /**
  * qerror_human(): Format QError data into human-readable string.
  */
diff --git a/qerror.h b/qerror.h
index 92a4b83..f84147b 100644
--- a/qerror.h
+++ b/qerror.h
@@ -37,7 +37,6 @@ QString *qerror_human(const QError *qerror);
 void qerror_report(ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void qerror_report_err(Error *err);
 void assert_no_error(Error *err);
-char *qerror_format(const char *fmt, QDict *error);
 
 /*
  * QError class list
-- 
1.7.11.2.249.g31c7954.dirty

Re: [Qemu-devel] [PATCH 20/27] qapi-schema: add ErrorClass enum

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

On 07/27/2012 03:32 PM, Luiz Capitulino wrote:
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  qapi-schema.json | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7500754..b1325c4 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3,6 +3,32 @@
>  # QAPI Schema
>  
>  ##
> +# @ErrorClass
> +#
> +# QEMU error classes
> +#
> +# @GenericError: this is used for errors that don't require a specific error
> +#                class. This should be the default case for most errors
> +#
> +# @CommandNotFound: the request command has not been found

s/request/requested/

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Luiz Capitulino]

On Fri, 27 Jul 2012 18:31:41 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> [Please, read below why this is an RFC]
> 
> This series implements the 'Plan for error handling in QMP' as described
> by Anthony in this email:

For anyone willing to try this series, it applies on top of dfe1ce5d80
on master. My local mirror broke a few days ago and I only noticed
yesterday.

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Amos Kong]

On 31/07/12 22:44, Luiz Capitulino wrote:
> On Fri, 27 Jul 2012 18:31:41 -0300
> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>
>> [Please, read below why this is an RFC]
>>
>> This series implements the 'Plan for error handling in QMP' as described
>> by Anthony in this email:



Tested with 
http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1


| bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
| {
|     const char *error_class;
|     char *ptr;
|     char *end;
|
|     if (!err) {
|         return false;
|     }
|
|     ptr = strstr(fmt, "'class': '");

                           ^^^ "'class: '" doesn't exist in *fmt

|     assert(ptr != NULL);
|



(gdb) r
Starting program: /home/devel/qemu/x86_64-softmmu/qemu-system-x86_64 
--enable-kvm -monitor stdio -boot n -vnc :1
[Thread debugging using libthread_db enabled]
[New Thread 0x7ffff1f3f700 (LWP 16427)]
[New Thread 0x7fffe3dfe700 (LWP 16429)]
QEMU 1.1.50 monitor - type 'help' for more information
(qemu) migrate -d tcp:0:1234
qemu-system-x86_64: error.c:92: error_is_type: Assertion `ptr != ((void 
*)0)' failed.

Program received signal SIGABRT, Aborted.
0x00007ffff4ed48a5 in raise () from /lib64/libc.so.6

(gdb) bt
#0  0x00007ffff4ed48a5 in raise () from /lib64/libc.so.6
#1  0x00007ffff4ed6085 in abort () from /lib64/libc.so.6
#2  0x00007ffff4ecda1e in __assert_fail_base () from /lib64/libc.so.6
#3  0x00007ffff4ecdae0 in __assert_fail () from /lib64/libc.so.6
#4  0x00007ffff7ca86b9 in error_is_type (err=0x7ffff8bf0a90, 
err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x7ffff7f63030 "Connection can 
not be completed immediately") at error.c:92
#5  0x00007ffff7db645f in tcp_start_outgoing_migration 
(s=0x7ffff82ef6c0, host_port=0x7ffff8c1a064 "0:1234", 
errp=0x7fffffffc8f8) at migration-tcp.c:93
#6  0x00007ffff7db7d70 in qmp_migrate (uri=0x7ffff8c1a060 "tcp:0:1234", 
has_blk=false, blk=false, has_inc=false, inc=false, has_detach=false, 
detach=false, errp=0x7fffffffc8f8)
     at migration.c:431
#7  0x00007ffff7cabbf8 in hmp_migrate (mon=0x7ffff8bb4760, 
qdict=0x7ffff8c16de0) at hmp.c:948
#8  0x00007ffff7eb60e8 in handle_user_command (mon=0x7ffff8bb4760, 
cmdline=0x7ffff8bb4bd0 "migrate -d tcp:0:1234") at 
/home/devel/qemu/monitor.c:3882
#9  0x00007ffff7eb7ad4 in monitor_command_cb (mon=0x7ffff8bb4760, 
cmdline=0x7ffff8bb4bd0 "migrate -d tcp:0:1234", opaque=0x0) at 
/home/devel/qemu/monitor.c:4499
#10 0x00007ffff7df9d35 in readline_handle_byte (rs=0x7ffff8bb4bd0, 
ch=13) at readline.c:373
#11 0x00007ffff7eb7a0d in monitor_read (opaque=0x7ffff8bb4760, 
buf=0x7fffffffcae0 "\r", size=1) at /home/devel/qemu/monitor.c:4485
#12 0x00007ffff7ddcbf1 in qemu_chr_be_write (s=0x7ffff8baf580, 
buf=0x7fffffffcae0 "\r", len=1) at qemu-char.c:164
#13 0x00007ffff7dde022 in fd_chr_read (opaque=0x7ffff8baf580) at 
qemu-char.c:588
#14 0x00007ffff7ce8392 in qemu_iohandler_poll (readfds=0x7ffff87042e0, 
writefds=0x7ffff8704360, xfds=0x7ffff87043e0, ret=1) at iohandler.c:121
#15 0x00007ffff7db5aca in main_loop_wait (nonblocking=0) at main-loop.c:497
#16 0x00007ffff7dadb70 in main_loop () at /home/devel/qemu/vl.c:1560
#17 0x00007ffff7db463d in main (argc=8, argv=0x7fffffffdfe8, 
envp=0x7fffffffe030) at /home/devel/qemu/vl.c:3658
(gdb) frame 4
#4  0x00007ffff7ca86b9 in error_is_type (err=0x7ffff8bf0a90, 
err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x7ffff7f63030 "Connection can 
not be completed immediately") at error.c:92
92          assert(ptr != NULL);
(gdb) l
87          if (!err) {
88              return false;
89          }
90
91          ptr = strstr(fmt, "'class': '");
92          assert(ptr != NULL);
93          ptr += strlen("'class': '");
94
95          end = strchr(ptr, '\'');
96          assert(end != NULL);





-- 
			Amos.

Re: [Qemu-devel] [PATCH 26/27] error, qerror: pass desc string to error calls

[Amos Kong]

On 28/07/12 05:32, Luiz Capitulino wrote:
> FIXME: broke error_is_type() and qemu-ga.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   error.c  |   5 ++-
>   qerror.c |  44 ++------------------
>   qerror.h | 143 +++++++++++++++++++++++++++++++--------------------------------
>   3 files changed, 77 insertions(+), 115 deletions(-)
>
> diff --git a/error.c b/error.c
> index 1c37092..dd20ab8 100644
> --- a/error.c
> +++ b/error.c
> @@ -29,6 +29,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>   {
>       Error *err;
>       va_list ap;
> +    char msg[2048];
>
>       if (errp == NULL) {
>           return;
> @@ -37,9 +38,9 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>       err = g_malloc0(sizeof(*err));
>
>       va_start(ap, fmt);
> -    err->obj = qobject_to_qdict(qobject_from_jsonv(fmt,&ap));

err->obj is not set in latest error_set(), it would be used in 
error_is_type()

    >> error_class = qdict_get_str(err->obj, "class");



> +    vsnprintf(msg, sizeof(msg), fmt, ap);
>       va_end(ap);
> -    err->msg = qerror_format(fmt, err->obj);
> +    err->msg = g_strdup(msg);
>       err->err_class = err_class;
>
>       *errp = err;
> diff --git a/qerror.c b/qerror.c
> index 85e65b8..f443261 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -346,45 +346,6 @@ static QError *qerror_new(void)
>       return qerr;
>   }
>
> -static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
> -{
> -    QObject *obj;
> -    QDict *ret;
> -
> -    obj = qobject_from_jsonv(fmt, va);
> -    if (!obj) {
> -        fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
> -        abort();
> -    }
> -    if (qobject_type(obj) != QTYPE_QDICT) {
> -        fprintf(stderr, "error is not a dict '%s'\n", fmt);
> -        abort();
> -    }
> -
> -    ret = qobject_to_qdict(obj);
> -    obj = qdict_get(ret, "class");
> -    if (!obj) {
> -        fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
> -        abort();
> -    }
> -    if (qobject_type(obj) != QTYPE_QSTRING) {
> -        fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
> -        abort();
> -    }
> -
> -    obj = qdict_get(ret, "data");
> -    if (!obj) {
> -        fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
> -        abort();
> -    }
> -    if (qobject_type(obj) != QTYPE_QDICT) {
> -        fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
> -        abort();
> -    }
> -
> -    return ret;
> -}
> -
>   /**
>    * qerror_from_info(): Create a new QError from error information
>    *
> @@ -394,13 +355,14 @@ static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
>                                   va_list *va)
>   {
>       QError *qerr;
> +    char msg[2048];
>
>       qerr = qerror_new();
>       loc_save(&qerr->loc);
>
>       qerr->err_class = err_class;
> -    qerr->error = error_obj_from_fmt_no_fail(fmt, va);
> -    qerr->err_msg = qerror_format(fmt, qerr->error);
> +    vsnprintf(msg, sizeof(msg), fmt, *va);
> +    qerr->err_msg = g_strdup(msg);
>
>       return qerr;
>   }
> diff --git a/qerror.h b/qerror.h
> index 14b9bfc..92a4b83 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -45,217 +45,216 @@ char *qerror_format(const char *fmt, QDict *error);
>    * Use scripts/check-qerror.sh to check.
>    */
>   #define QERR_ADD_CLIENT_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Could not add client"
>
>   #define QERR_AMBIGUOUS_PATH \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"
>
>   #define QERR_BAD_BUS_FOR_DEVICE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus"
>
>   #define QERR_BASE_NOT_FOUND \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
>
>   #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
>
>   #define QERR_BUFFER_OVERRUN \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "An internal buffer overran"
>
>   #define QERR_BUS_NO_HOTPLUG \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Bus '%s' does not support hotplugging"
>
>   #define QERR_BUS_NOT_FOUND \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Bus '%s' not found"
>
>   #define QERR_COMMAND_DISABLED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "The command %s has been disabled for this instance"
>
>   #define QERR_COMMAND_NOT_FOUND \
> -    ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
> +    ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found"
>
>   #define QERR_DEVICE_ENCRYPTED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is encrypted (filename=%s)"
>
>   #define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when using feature '%s' in device '%s'"
>
>   #define QERR_DEVICE_HAS_NO_MEDIUM \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no medium"
>
>   #define QERR_DEVICE_INIT_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' could not be initialized"
>
>   #define QERR_DEVICE_IN_USE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
>
>   #define QERR_DEVICE_IS_READ_ONLY \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
>
>   #define QERR_DEVICE_LOCKED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is locked"
>
>   #define QERR_DEVICE_MULTIPLE_BUSSES \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has multiple child busses"
>
>   #define QERR_DEVICE_NO_BUS \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no child bus"
>
>   #define QERR_DEVICE_NO_HOTPLUG \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
>
>   #define QERR_DEVICE_NOT_ACTIVE \
> -    ERROR_CLASS_DEVICE_NOT_ACTIVE, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
> +    ERROR_CLASS_DEVICE_NOT_ACTIVE, "Device '%s' has not been activated"
>
>   #define QERR_DEVICE_NOT_ENCRYPTED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not encrypted"
>
>   #define QERR_DEVICE_NOT_FOUND \
> -    ERROR_CLASS_DEVICE_NOT_FOUND, "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
> +    ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found"
>
>   #define QERR_DEVICE_NOT_REMOVABLE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not removable"
>
>   #define QERR_DUPLICATE_ID \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Duplicate ID '%s' for %s"
>
>   #define QERR_FD_NOT_FOUND \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "File descriptor named '%s' not found"
>
>   #define QERR_FD_NOT_SUPPLIED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotSupplied', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "No file descriptor supplied via SCM_RIGHTS"
>
>   #define QERR_FEATURE_DISABLED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
>
>   #define QERR_INVALID_BLOCK_FORMAT \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Invalid block format '%s'"
>
>   #define QERR_INVALID_OPTION_GROUP \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "There is no option group '%s'"
>
>   #define QERR_INVALID_PARAMETER \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter '%s'"
>
>   #define QERR_INVALID_PARAMETER_COMBINATION \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterCombination', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter combination"
>
>   #define QERR_INVALID_PARAMETER_TYPE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter type for '%s', expected: %s"
>
>   #define QERR_INVALID_PARAMETER_VALUE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' expects %s"
>
>   #define QERR_INVALID_PASSWORD \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidPassword', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Password incorrect"
>
>   #define QERR_IO_ERROR \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'IOError', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "An IO error has occurred"
>
>   #define QERR_JSON_PARSE_ERROR \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "JSON parse error, %s"
>
>   #define QERR_JSON_PARSING \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParsing', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
>
>   #define QERR_KVM_MISSING_CAP \
> -    ERROR_CLASS_K_V_M_MISSING_CAP, "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
> +    ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
>
>   #define QERR_MIGRATION_ACTIVE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationActive', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
>
>   #define QERR_MIGRATION_NOT_SUPPORTED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
> +    ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device '%s'"
>
>   #define QERR_MIGRATION_EXPECTED \
> -    ERROR_CLASS_MIGRATION_EXPECTED, "{ 'class': 'MigrationExpected', 'data': {} }"
> +    ERROR_CLASS_MIGRATION_EXPECTED, "An incoming migration is expected before this command can be executed"
>
>   #define QERR_MISSING_PARAMETER \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
>
>   #define QERR_NO_BUS_FOR_DEVICE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "No '%s' bus found for device '%s'"
>
>   #define QERR_NOT_SUPPORTED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NotSupported', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Not supported"
>
>   #define QERR_OPEN_FILE_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'"
>
>   #define QERR_PERMISSION_DENIED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PermissionDenied', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"
>
>   #define QERR_PROPERTY_NOT_FOUND \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' not found"
>
>   #define QERR_PROPERTY_VALUE_BAD \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' doesn't take value '%s'"
>
>   #define QERR_PROPERTY_VALUE_IN_USE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' can't take value '%s', it's in use"
>
>   #define QERR_PROPERTY_VALUE_NOT_FOUND \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' can't find value '%s'"
>
>   #define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
> -    "'device': %s, 'property': %s, 'value': %"PRId64" } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value '%" PRId64 "', it's not a power of 2"
>
>   #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")"
>
>   #define QERR_QGA_COMMAND_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Guest agent command failed, error was '%s'"
>
>   #define QERR_QGA_LOGGING_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaLoggingFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Guest agent failed to log non-optional log statement"
>
>   #define QERR_QMP_BAD_INPUT_OBJECT \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Expected '%s' in QMP input"
>
>   #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' expects '%s'"
>
>   #define QERR_QMP_EXTRA_MEMBER \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' is unexpected"
>
>   #define QERR_RESET_REQUIRED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'ResetRequired', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Resetting the Virtual Machine is required"
>
>   #define QERR_SET_PASSWD_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SetPasswdFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Could not set password"
>
>   #define QERR_TOO_MANY_FILES \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'TooManyFiles', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Too many open files"
>
>   #define QERR_UNDEFINED_ERROR \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UndefinedError', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "An undefined error has occurred"
>
>   #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "'%s' uses a %s feature which is not supported by this qemu version: %s"
>
>   #define QERR_UNSUPPORTED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'Unsupported', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "this feature or command is not currently supported"
>
>   #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'"
>
>   #define QERR_VNC_SERVER_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
> +    ERROR_CLASS_GENERIC_ERROR, "Could not start VNC server on %s"
>
>   #define QERR_SOCKET_CONNECT_IN_PROGRESS \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectInprogress', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Connection can not be completed immediately"
>
>   #define QERR_SOCKET_CONNECT_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
>
>   #define QERR_SOCKET_LISTEN_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockListenFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Failed to set socket to listening mode"
>
>   #define QERR_SOCKET_BIND_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockBindFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Failed to bind socket"
>
>   #define QERR_SOCKET_CREATE_FAILED \
> -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockCreateFailed', 'data': {} }"
> +    ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
>
>   #endif /* QERROR_H */

-- 
			Amos.

Re: [Qemu-devel] [PATCH 11/27] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Today, hmp_cont() checks for QERR_DEVICE_ENCRYPTED in order to know
> if qmp_cont() failed due to an encrypted device. If it did,
> hmp_cont() accesses QERR_DEVICE_ENCRYPTED's data member 'device' to
> precisely know for which device an encryption key must be set.
>
> However, all errors data members are going to be dropped by a later
> commit, so hmp_cont() can't do this anymore.
>
> This commit changes hmp_cont() to loop through all block devices
> and proactively set an encryption key for any encrypted device
> without a valid one.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hmp.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 6b72a64..a906f8a 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -610,34 +610,41 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
>  
>  static void hmp_cont_cb(void *opaque, int err)
>  {
> -    Monitor *mon = opaque;
> -
>      if (!err) {
> -        hmp_cont(mon, NULL);
> +        qmp_cont(NULL);
>      }
>  }
>  
> -void hmp_cont(Monitor *mon, const QDict *qdict)
> +static bool block_dev_is_encrypted(const BlockInfo *bdev)
>  {
> -    Error *errp = NULL;
> -
> -    qmp_cont(&errp);
> -    if (error_is_set(&errp)) {
> -        if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
> -            const char *device;
> +    return (bdev->inserted && bdev->inserted->encrypted);
> +}
>  
> -            /* The device is encrypted. Ask the user for the password
> -               and retry */
> +static bool block_dev_key_is_set(const BlockInfo *bdev)
> +{
> +    return (bdev->inserted && bdev->inserted->valid_encryption_key);
> +}

New static helpers block_dev_is_encrypted(), block_dev_key_is_set().
They work on BlockInfo.  Call them blockinfo_{is_encrypted,key_is_set}()?

>  
> -            device = error_get_field(errp, "device");
> -            assert(device != NULL);
> +void hmp_cont(Monitor *mon, const QDict *qdict)
> +{
> +    BlockInfoList *bdev_list, *bdev;
> +    Error *errp = NULL;
>  
> -            monitor_read_block_device_key(mon, device, hmp_cont_cb, mon);
> -            error_free(errp);
> -            return;
> +    bdev_list = qmp_query_block(NULL);
> +    for (bdev = bdev_list; bdev; bdev = bdev->next) {
> +        if (block_dev_is_encrypted(bdev->value) &&
> +            !block_dev_key_is_set(bdev->value)) {
> +                monitor_read_block_device_key(mon, bdev->value->device,
> +                                              hmp_cont_cb, NULL);
> +                goto out;

Any particular reason for creating BlockInfos just to find out whether
we lack the key?  Why not bdrv_key_required()?

>          }
> -        hmp_handle_error(mon, &errp);
>      }
> +
> +    qmp_cont(&errp);
> +    hmp_handle_error(mon, &errp);
> +
> +out:
> +    qapi_free_BlockInfoList(bdev_list);
>  }
>  
>  void hmp_system_wakeup(Monitor *mon, const QDict *qdict)

Diff makes this change look worse than it is.  Odd: M-x ediff gets it
right.  Anyway, here's how I think it works:

Unchanged qmp_cont(): search the bdrv_states for the first encrypted one
without a key.  If found, set err argument to QERR_DEVICE_ENCRYPTED.
Other errors unrelated to encrypted devices are also possible.

hmp_cont() before: try qmp_cont().  If we get QERR_DEVICE_ENCRYPTED,
extract the device from the error object, and prompt for its key, with a
callback that retries hmp_cont() if the key was provided.

After: search the bdrv_states for an encrypted one without a key.  If
there is none, qmp_cont(), no special error handling.  If there is one,
prompt for its key, with a callback that runs qmp_cont() if the key was
provided.

Have you tested this with multiple devices lacking their key?

Re: [Qemu-devel] [PATCH 12/27] hmp: hmp_change(): don't use error_get_field()

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Use the 'device' passed by the user and call qmp_query_block() to
> get the 'filename' info.
>
> error_get_field() is going to be dropped by a future commit.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hmp.c | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index a906f8a..435c9cd 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -783,17 +783,35 @@ static void hmp_change_read_arg(Monitor *mon, const char *password,
>  static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
>                                     void *opaque)
>  {
> -    Error *encryption_err = opaque;
> +    char *device = opaque;
>      Error *err = NULL;
> -    const char *device;
> -
> -    device = error_get_field(encryption_err, "device");
>  
>      qmp_block_passwd(device, password, &err);
>      hmp_handle_error(mon, &err);
> -    error_free(encryption_err);
>  
>      monitor_read_command(mon, 1);
> +    g_free(device);
> +}
> +
> +static char *get_device_file(const char *device)
> +{
> +    BlockInfoList *bdev_list, *bdev;
> +    char *ret;
> +
> +    bdev_list = qmp_query_block(NULL);
> +    for (bdev = bdev_list; bdev; bdev = bdev->next) {
> +        if (!strcmp(bdev->value->device, device)) {
> +            break;
> +        }
> +    }
> +
> +    assert(bdev);
> +    assert(bdev->value->has_inserted);
> +
> +    ret = g_strdup(bdev->value->inserted->file);
> +    qapi_free_BlockInfoList(bdev_list);
> +
> +    return ret;
>  }
>  
>  void hmp_change(Monitor *mon, const QDict *qdict)
> @@ -814,9 +832,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>  
>      qmp_change(device, target, !!arg, arg, &err);
>      if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
> -        monitor_printf(mon, "%s (%s) is encrypted.\n",
> -                       error_get_field(err, "device"),
> -                       error_get_field(err, "filename"));
> +        char *filename = get_device_file(device);

Elsewhere, we use bdrv_get_encrypted_filename(), which does the right
thing for encrypted backing files.  Why is that not necessary here?

Why not simply bdrv_get_encrypted_filename(bdrv_find(device))?

> +        monitor_printf(mon, "%s (%s) is encrypted.\n", device, filename);
> +        g_free(filename);
>          if (!monitor_get_rs(mon)) {
>              monitor_printf(mon,
>                      "terminal does not support password prompting\n");
> @@ -824,9 +842,10 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>              return;
>          }
>          readline_start(monitor_get_rs(mon), "Password: ", 1,
> -                       cb_hmp_change_bdrv_pwd, err);
> +                       cb_hmp_change_bdrv_pwd, (void *) g_strdup(device));

Casting a pointer to void * is pointless noise :)

>          return;
>      }
> +
>      hmp_handle_error(mon, &err);
>  }

Re: [Qemu-devel] [PATCH 15/27] block: block_int: include qerror.h

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Several block/ files are relying on qerror.h being provided by
> qapi-types.h. However, qapi-types.h won't provide it anymore.

Squash it into commit that makes qapi-types.h no longer provide it?

Re: [Qemu-devel] [PATCH 16/27] hmp: hmp.h: include qdict.h

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> hmp.h is relying on qdict.h being provided by qapi-types.h. However,
> qapi-types.h won't provide it anymore.

Squash it into commit that makes qapi-types.h no longer provide it?

Re: [Qemu-devel] [PATCH 12/27] hmp: hmp_change(): don't use error_get_field()

[Anthony Liguori]

Markus Armbruster <armbru@redhat.com> writes:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> Use the 'device' passed by the user and call qmp_query_block() to
>> get the 'filename' info.
>>
>> error_get_field() is going to be dropped by a future commit.
>>
>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> ---
>>  hmp.c | 37 ++++++++++++++++++++++++++++---------
>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index a906f8a..435c9cd 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -783,17 +783,35 @@ static void hmp_change_read_arg(Monitor *mon, const char *password,
>>  static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
>>                                     void *opaque)
>>  {
>> -    Error *encryption_err = opaque;
>> +    char *device = opaque;
>>      Error *err = NULL;
>> -    const char *device;
>> -
>> -    device = error_get_field(encryption_err, "device");
>>  
>>      qmp_block_passwd(device, password, &err);
>>      hmp_handle_error(mon, &err);
>> -    error_free(encryption_err);
>>  
>>      monitor_read_command(mon, 1);
>> +    g_free(device);
>> +}
>> +
>> +static char *get_device_file(const char *device)
>> +{
>> +    BlockInfoList *bdev_list, *bdev;
>> +    char *ret;
>> +
>> +    bdev_list = qmp_query_block(NULL);
>> +    for (bdev = bdev_list; bdev; bdev = bdev->next) {
>> +        if (!strcmp(bdev->value->device, device)) {
>> +            break;
>> +        }
>> +    }
>> +
>> +    assert(bdev);
>> +    assert(bdev->value->has_inserted);
>> +
>> +    ret = g_strdup(bdev->value->inserted->file);
>> +    qapi_free_BlockInfoList(bdev_list);
>> +
>> +    return ret;
>>  }
>>  
>>  void hmp_change(Monitor *mon, const QDict *qdict)
>> @@ -814,9 +832,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>>  
>>      qmp_change(device, target, !!arg, arg, &err);
>>      if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
>> -        monitor_printf(mon, "%s (%s) is encrypted.\n",
>> -                       error_get_field(err, "device"),
>> -                       error_get_field(err, "filename"));
>> +        char *filename = get_device_file(device);
>
> Elsewhere, we use bdrv_get_encrypted_filename(), which does the right
> thing for encrypted backing files.  Why is that not necessary here?
>
> Why not simply bdrv_get_encrypted_filename(bdrv_find(device))?

Those are not QMP functions.  hmp.c is only allowed to use QMP interfaces.

Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Luiz Capitulino]

On Wed, 01 Aug 2012 19:33:27 +0800
Amos Kong <akong@redhat.com> wrote:

> On 31/07/12 22:44, Luiz Capitulino wrote:
> > On Fri, 27 Jul 2012 18:31:41 -0300
> > Luiz Capitulino<lcapitulino@redhat.com>  wrote:
> >
> >> [Please, read below why this is an RFC]
> >>
> >> This series implements the 'Plan for error handling in QMP' as described
> >> by Anthony in this email:
> 
> 
> 
> Tested with 
> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1

Thanks for testing Amos, but that branch is where I'm working currently so
the code there is constantly changing. It's better to wait until I post it
to the list.

Could you share your test-case, btw?

Re: [Qemu-devel] [PATCH 26/27] error, qerror: pass desc string to error calls

[Luiz Capitulino]

On Wed, 01 Aug 2012 19:37:29 +0800
Amos Kong <akong@redhat.com> wrote:

> On 28/07/12 05:32, Luiz Capitulino wrote:
> > FIXME: broke error_is_type() and qemu-ga.
> >
> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > ---
> >   error.c  |   5 ++-
> >   qerror.c |  44 ++------------------
> >   qerror.h | 143 +++++++++++++++++++++++++++++++--------------------------------
> >   3 files changed, 77 insertions(+), 115 deletions(-)
> >
> > diff --git a/error.c b/error.c
> > index 1c37092..dd20ab8 100644
> > --- a/error.c
> > +++ b/error.c
> > @@ -29,6 +29,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
> >   {
> >       Error *err;
> >       va_list ap;
> > +    char msg[2048];
> >
> >       if (errp == NULL) {
> >           return;
> > @@ -37,9 +38,9 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
> >       err = g_malloc0(sizeof(*err));
> >
> >       va_start(ap, fmt);
> > -    err->obj = qobject_to_qdict(qobject_from_jsonv(fmt,&ap));
> 
> err->obj is not set in latest error_set(), it would be used in 
> error_is_type()

Yes, I'm aware about this and wrote to the changelog above so that I don't
forget about it.

> 
>     >> error_class = qdict_get_str(err->obj, "class");
> 
> 
> 
> > +    vsnprintf(msg, sizeof(msg), fmt, ap);
> >       va_end(ap);
> > -    err->msg = qerror_format(fmt, err->obj);
> > +    err->msg = g_strdup(msg);
> >       err->err_class = err_class;
> >
> >       *errp = err;
> > diff --git a/qerror.c b/qerror.c
> > index 85e65b8..f443261 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -346,45 +346,6 @@ static QError *qerror_new(void)
> >       return qerr;
> >   }
> >
> > -static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
> > -{
> > -    QObject *obj;
> > -    QDict *ret;
> > -
> > -    obj = qobject_from_jsonv(fmt, va);
> > -    if (!obj) {
> > -        fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
> > -        abort();
> > -    }
> > -    if (qobject_type(obj) != QTYPE_QDICT) {
> > -        fprintf(stderr, "error is not a dict '%s'\n", fmt);
> > -        abort();
> > -    }
> > -
> > -    ret = qobject_to_qdict(obj);
> > -    obj = qdict_get(ret, "class");
> > -    if (!obj) {
> > -        fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
> > -        abort();
> > -    }
> > -    if (qobject_type(obj) != QTYPE_QSTRING) {
> > -        fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
> > -        abort();
> > -    }
> > -
> > -    obj = qdict_get(ret, "data");
> > -    if (!obj) {
> > -        fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
> > -        abort();
> > -    }
> > -    if (qobject_type(obj) != QTYPE_QDICT) {
> > -        fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
> > -        abort();
> > -    }
> > -
> > -    return ret;
> > -}
> > -
> >   /**
> >    * qerror_from_info(): Create a new QError from error information
> >    *
> > @@ -394,13 +355,14 @@ static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
> >                                   va_list *va)
> >   {
> >       QError *qerr;
> > +    char msg[2048];
> >
> >       qerr = qerror_new();
> >       loc_save(&qerr->loc);
> >
> >       qerr->err_class = err_class;
> > -    qerr->error = error_obj_from_fmt_no_fail(fmt, va);
> > -    qerr->err_msg = qerror_format(fmt, qerr->error);
> > +    vsnprintf(msg, sizeof(msg), fmt, *va);
> > +    qerr->err_msg = g_strdup(msg);
> >
> >       return qerr;
> >   }
> > diff --git a/qerror.h b/qerror.h
> > index 14b9bfc..92a4b83 100644
> > --- a/qerror.h
> > +++ b/qerror.h
> > @@ -45,217 +45,216 @@ char *qerror_format(const char *fmt, QDict *error);
> >    * Use scripts/check-qerror.sh to check.
> >    */
> >   #define QERR_ADD_CLIENT_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Could not add client"
> >
> >   #define QERR_AMBIGUOUS_PATH \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"
> >
> >   #define QERR_BAD_BUS_FOR_DEVICE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus"
> >
> >   #define QERR_BASE_NOT_FOUND \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
> >
> >   #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
> >
> >   #define QERR_BUFFER_OVERRUN \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "An internal buffer overran"
> >
> >   #define QERR_BUS_NO_HOTPLUG \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Bus '%s' does not support hotplugging"
> >
> >   #define QERR_BUS_NOT_FOUND \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Bus '%s' not found"
> >
> >   #define QERR_COMMAND_DISABLED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "The command %s has been disabled for this instance"
> >
> >   #define QERR_COMMAND_NOT_FOUND \
> > -    ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found"
> >
> >   #define QERR_DEVICE_ENCRYPTED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is encrypted (filename=%s)"
> >
> >   #define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when using feature '%s' in device '%s'"
> >
> >   #define QERR_DEVICE_HAS_NO_MEDIUM \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no medium"
> >
> >   #define QERR_DEVICE_INIT_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' could not be initialized"
> >
> >   #define QERR_DEVICE_IN_USE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
> >
> >   #define QERR_DEVICE_IS_READ_ONLY \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
> >
> >   #define QERR_DEVICE_LOCKED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is locked"
> >
> >   #define QERR_DEVICE_MULTIPLE_BUSSES \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has multiple child busses"
> >
> >   #define QERR_DEVICE_NO_BUS \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no child bus"
> >
> >   #define QERR_DEVICE_NO_HOTPLUG \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
> >
> >   #define QERR_DEVICE_NOT_ACTIVE \
> > -    ERROR_CLASS_DEVICE_NOT_ACTIVE, "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_DEVICE_NOT_ACTIVE, "Device '%s' has not been activated"
> >
> >   #define QERR_DEVICE_NOT_ENCRYPTED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not encrypted"
> >
> >   #define QERR_DEVICE_NOT_FOUND \
> > -    ERROR_CLASS_DEVICE_NOT_FOUND, "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found"
> >
> >   #define QERR_DEVICE_NOT_REMOVABLE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not removable"
> >
> >   #define QERR_DUPLICATE_ID \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Duplicate ID '%s' for %s"
> >
> >   #define QERR_FD_NOT_FOUND \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "File descriptor named '%s' not found"
> >
> >   #define QERR_FD_NOT_SUPPLIED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FdNotSupplied', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "No file descriptor supplied via SCM_RIGHTS"
> >
> >   #define QERR_FEATURE_DISABLED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
> >
> >   #define QERR_INVALID_BLOCK_FORMAT \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Invalid block format '%s'"
> >
> >   #define QERR_INVALID_OPTION_GROUP \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "There is no option group '%s'"
> >
> >   #define QERR_INVALID_PARAMETER \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter '%s'"
> >
> >   #define QERR_INVALID_PARAMETER_COMBINATION \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterCombination', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter combination"
> >
> >   #define QERR_INVALID_PARAMETER_TYPE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Invalid parameter type for '%s', expected: %s"
> >
> >   #define QERR_INVALID_PARAMETER_VALUE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' expects %s"
> >
> >   #define QERR_INVALID_PASSWORD \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'InvalidPassword', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Password incorrect"
> >
> >   #define QERR_IO_ERROR \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'IOError', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "An IO error has occurred"
> >
> >   #define QERR_JSON_PARSE_ERROR \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "JSON parse error, %s"
> >
> >   #define QERR_JSON_PARSING \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'JSONParsing', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
> >
> >   #define QERR_KVM_MISSING_CAP \
> > -    ERROR_CLASS_K_V_M_MISSING_CAP, "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
> > +    ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
> >
> >   #define QERR_MIGRATION_ACTIVE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationActive', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
> >
> >   #define QERR_MIGRATION_NOT_SUPPORTED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device '%s'"
> >
> >   #define QERR_MIGRATION_EXPECTED \
> > -    ERROR_CLASS_MIGRATION_EXPECTED, "{ 'class': 'MigrationExpected', 'data': {} }"
> > +    ERROR_CLASS_MIGRATION_EXPECTED, "An incoming migration is expected before this command can be executed"
> >
> >   #define QERR_MISSING_PARAMETER \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
> >
> >   #define QERR_NO_BUS_FOR_DEVICE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "No '%s' bus found for device '%s'"
> >
> >   #define QERR_NOT_SUPPORTED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'NotSupported', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Not supported"
> >
> >   #define QERR_OPEN_FILE_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'"
> >
> >   #define QERR_PERMISSION_DENIED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PermissionDenied', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"
> >
> >   #define QERR_PROPERTY_NOT_FOUND \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' not found"
> >
> >   #define QERR_PROPERTY_VALUE_BAD \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' doesn't take value '%s'"
> >
> >   #define QERR_PROPERTY_VALUE_IN_USE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' can't take value '%s', it's in use"
> >
> >   #define QERR_PROPERTY_VALUE_NOT_FOUND \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' can't find value '%s'"
> >
> >   #define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
> > -    "'device': %s, 'property': %s, 'value': %"PRId64" } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value '%" PRId64 "', it's not a power of 2"
> >
> >   #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")"
> >
> >   #define QERR_QGA_COMMAND_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Guest agent command failed, error was '%s'"
> >
> >   #define QERR_QGA_LOGGING_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QgaLoggingFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Guest agent failed to log non-optional log statement"
> >
> >   #define QERR_QMP_BAD_INPUT_OBJECT \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Expected '%s' in QMP input"
> >
> >   #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' expects '%s'"
> >
> >   #define QERR_QMP_EXTRA_MEMBER \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' is unexpected"
> >
> >   #define QERR_RESET_REQUIRED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'ResetRequired', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Resetting the Virtual Machine is required"
> >
> >   #define QERR_SET_PASSWD_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SetPasswdFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Could not set password"
> >
> >   #define QERR_TOO_MANY_FILES \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'TooManyFiles', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Too many open files"
> >
> >   #define QERR_UNDEFINED_ERROR \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UndefinedError', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "An undefined error has occurred"
> >
> >   #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "'%s' uses a %s feature which is not supported by this qemu version: %s"
> >
> >   #define QERR_UNSUPPORTED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'Unsupported', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "this feature or command is not currently supported"
> >
> >   #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'"
> >
> >   #define QERR_VNC_SERVER_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Could not start VNC server on %s"
> >
> >   #define QERR_SOCKET_CONNECT_IN_PROGRESS \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectInprogress', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Connection can not be completed immediately"
> >
> >   #define QERR_SOCKET_CONNECT_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockConnectFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
> >
> >   #define QERR_SOCKET_LISTEN_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockListenFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Failed to set socket to listening mode"
> >
> >   #define QERR_SOCKET_BIND_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockBindFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Failed to bind socket"
> >
> >   #define QERR_SOCKET_CREATE_FAILED \
> > -    ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'SockCreateFailed', 'data': {} }"
> > +    ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
> >
> >   #endif /* QERROR_H */
> 

Re: [Qemu-devel] [PATCH 11/27] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED

[Luiz Capitulino]

On Wed, 01 Aug 2012 13:37:44 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Today, hmp_cont() checks for QERR_DEVICE_ENCRYPTED in order to know
> > if qmp_cont() failed due to an encrypted device. If it did,
> > hmp_cont() accesses QERR_DEVICE_ENCRYPTED's data member 'device' to
> > precisely know for which device an encryption key must be set.
> >
> > However, all errors data members are going to be dropped by a later
> > commit, so hmp_cont() can't do this anymore.
> >
> > This commit changes hmp_cont() to loop through all block devices
> > and proactively set an encryption key for any encrypted device
> > without a valid one.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hmp.c | 43 +++++++++++++++++++++++++------------------
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 6b72a64..a906f8a 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -610,34 +610,41 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
> >  
> >  static void hmp_cont_cb(void *opaque, int err)
> >  {
> > -    Monitor *mon = opaque;
> > -
> >      if (!err) {
> > -        hmp_cont(mon, NULL);
> > +        qmp_cont(NULL);
> >      }
> >  }
> >  
> > -void hmp_cont(Monitor *mon, const QDict *qdict)
> > +static bool block_dev_is_encrypted(const BlockInfo *bdev)
> >  {
> > -    Error *errp = NULL;
> > -
> > -    qmp_cont(&errp);
> > -    if (error_is_set(&errp)) {
> > -        if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
> > -            const char *device;
> > +    return (bdev->inserted && bdev->inserted->encrypted);
> > +}
> >  
> > -            /* The device is encrypted. Ask the user for the password
> > -               and retry */
> > +static bool block_dev_key_is_set(const BlockInfo *bdev)
> > +{
> > +    return (bdev->inserted && bdev->inserted->valid_encryption_key);
> > +}
> 
> New static helpers block_dev_is_encrypted(), block_dev_key_is_set().
> They work on BlockInfo.  Call them blockinfo_{is_encrypted,key_is_set}()?

Done for v1.

> 
> >  
> > -            device = error_get_field(errp, "device");
> > -            assert(device != NULL);
> > +void hmp_cont(Monitor *mon, const QDict *qdict)
> > +{
> > +    BlockInfoList *bdev_list, *bdev;
> > +    Error *errp = NULL;
> >  
> > -            monitor_read_block_device_key(mon, device, hmp_cont_cb, mon);
> > -            error_free(errp);
> > -            return;
> > +    bdev_list = qmp_query_block(NULL);
> > +    for (bdev = bdev_list; bdev; bdev = bdev->next) {
> > +        if (block_dev_is_encrypted(bdev->value) &&
> > +            !block_dev_key_is_set(bdev->value)) {
> > +                monitor_read_block_device_key(mon, bdev->value->device,
> > +                                              hmp_cont_cb, NULL);
> > +                goto out;
> 
> Any particular reason for creating BlockInfos just to find out whether
> we lack the key?  Why not bdrv_key_required()?

As Anthony answered in the other email, hmp.c is a real QMP client so it's
only allowed to use QMP and monitor functions.

> 
> >          }
> > -        hmp_handle_error(mon, &errp);
> >      }
> > +
> > +    qmp_cont(&errp);
> > +    hmp_handle_error(mon, &errp);
> > +
> > +out:
> > +    qapi_free_BlockInfoList(bdev_list);
> >  }
> >  
> >  void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
> 
> Diff makes this change look worse than it is.  Odd: M-x ediff gets it
> right.  Anyway, here's how I think it works:
> 
> Unchanged qmp_cont(): search the bdrv_states for the first encrypted one
> without a key.  If found, set err argument to QERR_DEVICE_ENCRYPTED.
> Other errors unrelated to encrypted devices are also possible.

Right, and this patch doesn't touch qmp_cont().

> hmp_cont() before: try qmp_cont().  If we get QERR_DEVICE_ENCRYPTED,
> extract the device from the error object, and prompt for its key, with a
> callback that retries hmp_cont() if the key was provided.

Correct.

> After: search the bdrv_states for an encrypted one without a key.  If
> there is none, qmp_cont(), no special error handling.  If there is one,
> prompt for its key, with a callback that runs qmp_cont() if the key was
> provided.

Exactly.

> Have you tested this with multiple devices lacking their key?

I've tested with two devices. Will test with four or five for v1.

Re: [Qemu-devel] [PATCH 12/27] hmp: hmp_change(): don't use error_get_field()

[Luiz Capitulino]

On Wed, 01 Aug 2012 14:39:04 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Use the 'device' passed by the user and call qmp_query_block() to
> > get the 'filename' info.
> >
> > error_get_field() is going to be dropped by a future commit.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hmp.c | 37 ++++++++++++++++++++++++++++---------
> >  1 file changed, 28 insertions(+), 9 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index a906f8a..435c9cd 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -783,17 +783,35 @@ static void hmp_change_read_arg(Monitor *mon, const char *password,
> >  static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
> >                                     void *opaque)
> >  {
> > -    Error *encryption_err = opaque;
> > +    char *device = opaque;
> >      Error *err = NULL;
> > -    const char *device;
> > -
> > -    device = error_get_field(encryption_err, "device");
> >  
> >      qmp_block_passwd(device, password, &err);
> >      hmp_handle_error(mon, &err);
> > -    error_free(encryption_err);
> >  
> >      monitor_read_command(mon, 1);
> > +    g_free(device);
> > +}
> > +
> > +static char *get_device_file(const char *device)
> > +{
> > +    BlockInfoList *bdev_list, *bdev;
> > +    char *ret;
> > +
> > +    bdev_list = qmp_query_block(NULL);
> > +    for (bdev = bdev_list; bdev; bdev = bdev->next) {
> > +        if (!strcmp(bdev->value->device, device)) {
> > +            break;
> > +        }
> > +    }
> > +
> > +    assert(bdev);
> > +    assert(bdev->value->has_inserted);
> > +
> > +    ret = g_strdup(bdev->value->inserted->file);
> > +    qapi_free_BlockInfoList(bdev_list);
> > +
> > +    return ret;
> >  }
> >  
> >  void hmp_change(Monitor *mon, const QDict *qdict)
> > @@ -814,9 +832,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> >  
> >      qmp_change(device, target, !!arg, arg, &err);
> >      if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
> > -        monitor_printf(mon, "%s (%s) is encrypted.\n",
> > -                       error_get_field(err, "device"),
> > -                       error_get_field(err, "filename"));
> > +        char *filename = get_device_file(device);
> 
> Elsewhere, we use bdrv_get_encrypted_filename(), which does the right
> thing for encrypted backing files.  Why is that not necessary here?
> 
> Why not simply bdrv_get_encrypted_filename(bdrv_find(device))?
> 
> > +        monitor_printf(mon, "%s (%s) is encrypted.\n", device, filename);
> > +        g_free(filename);
> >          if (!monitor_get_rs(mon)) {
> >              monitor_printf(mon,
> >                      "terminal does not support password prompting\n");
> > @@ -824,9 +842,10 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> >              return;
> >          }
> >          readline_start(monitor_get_rs(mon), "Password: ", 1,
> > -                       cb_hmp_change_bdrv_pwd, err);
> > +                       cb_hmp_change_bdrv_pwd, (void *) g_strdup(device));
> 
> Casting a pointer to void * is pointless noise :)

I don't remember why I did it, but I've fixed for v1.

Btw, this patch is different v1 because I'm dropping error_is_type()
usage (like I did for the previous patch).

Btw 2, I'm assuming that the other questions you asked have already been
answered in other emails.

> 
> >          return;
> >      }
> > +
> >      hmp_handle_error(mon, &err);
> >  }
> 

Re: [Qemu-devel] [PATCH 15/27] block: block_int: include qerror.h

[Luiz Capitulino]

On Wed, 01 Aug 2012 14:42:02 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Several block/ files are relying on qerror.h being provided by
> > qapi-types.h. However, qapi-types.h won't provide it anymore.
> 
> Squash it into commit that makes qapi-types.h no longer provide it?

As block_int.h and hmp.h (next commit) depend on different header files,
I think it's better to keep them on different commits.

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Amos Kong]

On 01/08/12 21:29, Luiz Capitulino wrote:
> On Wed, 01 Aug 2012 19:33:27 +0800
> Amos Kong<akong@redhat.com>  wrote:
>
>> On 31/07/12 22:44, Luiz Capitulino wrote:
>>> On Fri, 27 Jul 2012 18:31:41 -0300
>>> Luiz Capitulino<lcapitulino@redhat.com>   wrote:
>>>
>>>> [Please, read below why this is an RFC]
>>>>
>>>> This series implements the 'Plan for error handling in QMP' as described
>>>> by Anthony in this email:
>>
>>
>>
>> Tested with
>> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1
>
> Thanks for testing Amos, but that branch is where I'm working currently so
> the code there is constantly changing. It's better to wait until I post it
> to the list.

Got it.

> Could you share your test-case, btw?

1. start a migration listen vm
x86_64-softmmu/qemu-system-x86_64 -monitor stdio -boot n -vnc :2 
-incoming tcp:0:1234

2. start a migration client vm
x86_64-softmmu/qemu-system-x86_64 --enable-kvm -monitor stdio -boot n 
-vnc :1

3. execute migration
vm2 (qemu) migrate -d tcp:0:1234
vm2 (qemu) info migration

expected result: migration should complete successfully



-- 
			Amos.

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Luiz Capitulino]

On Thu, 02 Aug 2012 10:31:28 +0800
Amos Kong <akong@redhat.com> wrote:

> On 01/08/12 21:29, Luiz Capitulino wrote:
> > On Wed, 01 Aug 2012 19:33:27 +0800
> > Amos Kong<akong@redhat.com>  wrote:
> >
> >> On 31/07/12 22:44, Luiz Capitulino wrote:
> >>> On Fri, 27 Jul 2012 18:31:41 -0300
> >>> Luiz Capitulino<lcapitulino@redhat.com>   wrote:
> >>>
> >>>> [Please, read below why this is an RFC]
> >>>>
> >>>> This series implements the 'Plan for error handling in QMP' as described
> >>>> by Anthony in this email:
> >>
> >>
> >>
> >> Tested with
> >> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1
> >
> > Thanks for testing Amos, but that branch is where I'm working currently so
> > the code there is constantly changing. It's better to wait until I post it
> > to the list.
> 
> Got it.
> 
> > Could you share your test-case, btw?
> 
> 1. start a migration listen vm
> x86_64-softmmu/qemu-system-x86_64 -monitor stdio -boot n -vnc :2 
> -incoming tcp:0:1234
> 
> 2. start a migration client vm
> x86_64-softmmu/qemu-system-x86_64 --enable-kvm -monitor stdio -boot n 
> -vnc :1
> 
> 3. execute migration
> vm2 (qemu) migrate -d tcp:0:1234
> vm2 (qemu) info migration
> 
> expected result: migration should complete successfully

This should work fine with v1 I posted yesterday.

Actually, master is buggy:

 (qemu) migrate -d tcp:0:4444
 migrate: Connection can not be completed immediately
 (qemu) 

I wonder how you did not get this on your test-case? Anyway, this is
also fixed in v1.

Re: [Qemu-devel] [PATCH 15/27] block: block_int: include qerror.h

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Wed, 01 Aug 2012 14:42:02 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> > Several block/ files are relying on qerror.h being provided by
>> > qapi-types.h. However, qapi-types.h won't provide it anymore.
>> 
>> Squash it into commit that makes qapi-types.h no longer provide it?
>
> As block_int.h and hmp.h (next commit) depend on different header files,
> I think it's better to keep them on different commits.

I disagree, but I won't insist.

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Amos Kong]

----- Original Message -----
> On Thu, 02 Aug 2012 10:31:28 +0800
> Amos Kong <akong@redhat.com> wrote:
> 
> > On 01/08/12 21:29, Luiz Capitulino wrote:
> > > On Wed, 01 Aug 2012 19:33:27 +0800
> > > Amos Kong<akong@redhat.com>  wrote:
> > >
> > >> On 31/07/12 22:44, Luiz Capitulino wrote:
> > >>> On Fri, 27 Jul 2012 18:31:41 -0300
> > >>> Luiz Capitulino<lcapitulino@redhat.com>   wrote:
> > >>>
> > >>>> [Please, read below why this is an RFC]
> > >>>>
> > >>>> This series implements the 'Plan for error handling in QMP' as
> > >>>> described
> > >>>> by Anthony in this email:
> > >>
> > >>
> > >>
> > >> Tested with
> > >> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1
> > >
> > > Thanks for testing Amos, but that branch is where I'm working
> > > currently so
> > > the code there is constantly changing. It's better to wait until
> > > I post it
> > > to the list.
> > 
> > Got it.
> > 
> > > Could you share your test-case, btw?
> > 
> > 1. start a migration listen vm
> > x86_64-softmmu/qemu-system-x86_64 -monitor stdio -boot n -vnc :2
> > -incoming tcp:0:1234
> > 
> > 2. start a migration client vm
> > x86_64-softmmu/qemu-system-x86_64 --enable-kvm -monitor stdio -boot
> > n
> > -vnc :1
> > 
> > 3. execute migration
> > vm2 (qemu) migrate -d tcp:0:1234
> > vm2 (qemu) info migration
> > 
> > expected result: migration should complete successfully
> 
> This should work fine with v1 I posted yesterday.
> 
> Actually, master is buggy:
> 
>  (qemu) migrate -d tcp:0:4444
>  migrate: Connection can not be completed immediately


I thought it's a note of real socket status, not an error message.


>  (qemu)
> 
> I wonder how you did not get this on your test-case?

> Anyway, this is also fixed in v1.

Thanks.

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Luiz Capitulino]

On Mon, 6 Aug 2012 02:35:03 -0400 (EDT)
Amos Kong <akong@redhat.com> wrote:

> ----- Original Message -----
> > On Thu, 02 Aug 2012 10:31:28 +0800
> > Amos Kong <akong@redhat.com> wrote:
> > 
> > > On 01/08/12 21:29, Luiz Capitulino wrote:
> > > > On Wed, 01 Aug 2012 19:33:27 +0800
> > > > Amos Kong<akong@redhat.com>  wrote:
> > > >
> > > >> On 31/07/12 22:44, Luiz Capitulino wrote:
> > > >>> On Fri, 27 Jul 2012 18:31:41 -0300
> > > >>> Luiz Capitulino<lcapitulino@redhat.com>   wrote:
> > > >>>
> > > >>>> [Please, read below why this is an RFC]
> > > >>>>
> > > >>>> This series implements the 'Plan for error handling in QMP' as
> > > >>>> described
> > > >>>> by Anthony in this email:
> > > >>
> > > >>
> > > >>
> > > >> Tested with
> > > >> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1
> > > >
> > > > Thanks for testing Amos, but that branch is where I'm working
> > > > currently so
> > > > the code there is constantly changing. It's better to wait until
> > > > I post it
> > > > to the list.
> > > 
> > > Got it.
> > > 
> > > > Could you share your test-case, btw?
> > > 
> > > 1. start a migration listen vm
> > > x86_64-softmmu/qemu-system-x86_64 -monitor stdio -boot n -vnc :2
> > > -incoming tcp:0:1234
> > > 
> > > 2. start a migration client vm
> > > x86_64-softmmu/qemu-system-x86_64 --enable-kvm -monitor stdio -boot
> > > n
> > > -vnc :1
> > > 
> > > 3. execute migration
> > > vm2 (qemu) migrate -d tcp:0:1234
> > > vm2 (qemu) info migration
> > > 
> > > expected result: migration should complete successfully
> > 
> > This should work fine with v1 I posted yesterday.
> > 
> > Actually, master is buggy:
> > 
> >  (qemu) migrate -d tcp:0:4444
> >  migrate: Connection can not be completed immediately
> 
> 
> I thought it's a note of real socket status, not an error message.

That's not something relevant for the user to know because it's temporary,
and the user can't do anything about it anyway.

> 
> 
> >  (qemu)
> > 
> > I wonder how you did not get this on your test-case?
> 
> > Anyway, this is also fixed in v1.
> 
> Thanks.
> 
>