error: Misc cleanups and improvements

PATCH Applied Cole Robinson 3 participants 12 messages Mar 21, 2014 – Mar 27, 2014 Block layer

lore

Share & tools

lore

Cole Robinson

author

2014-03-21 23:42 UTC #1 lore raw

PATCH v2 1/9 slirp: Remove default_mon usage

In reply to Cole Robinson

These errors don't seem user initiated, so forcibly printing to the monitor doesn't seem right. Just use error_report.

Drop lprint since it's now unused.

Cc Jan Kiszka <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---

v2:
Use error_report instead of fprintf

 slirp/misc.c  | 13 ++-----------
 slirp/slirp.c |  8 ++++----
 slirp/slirp.h |  2 --
 3 files changed, 6 insertions(+), 17 deletions(-)

slirp/misc.c +2 ~~-11~~

index 6c1636f..b8eb74c 100644
@@ -136,7 +136,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 		if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
 		    bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
 		    listen(s, 1) < 0) {
-			lprint("Error: inet socket: %s\n", strerror(errno));
+			error_report("Error: inet socket: %s", strerror(errno));
 			closesocket(s);
 
 			return 0;

@@ -146,7 +146,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 	pid = fork();
 	switch(pid) {
 	 case -1:
-		lprint("Error: fork failed: %s\n", strerror(errno));
+		error_report("Error: fork failed: %s", strerror(errno));
 		close(s);
 		return 0;

@@ -242,15 +242,6 @@ strdup(str)
 }
 #endif
 
-void lprint(const char *format, ...)
-{
-    va_list args;
-
-    va_start(args, format);
-    monitor_vprintf(default_mon, format, args);
-    va_end(args);
-}
-
 void slirp_connection_info(Slirp *slirp, Monitor *mon)
 {
     const char * const tcpstates[] = {

slirp/slirp.c +4 -4

index bad8dad..3fb48a4 100644
@@ -139,7 +139,7 @@ int get_dns_addr(struct in_addr *pdns_addr)
         return -1;
 
 #ifdef DEBUG
-    lprint("IP address of your DNS(s): ");
+    fprintf(stderr, "IP address of your DNS(s): ");
 #endif
     while (fgets(buff, 512, f) != NULL) {
         if (sscanf(buff, "nameserver%*[ \t]%256s", buff2) == 1) {

@@ -153,17 +153,17 @@ int get_dns_addr(struct in_addr *pdns_addr)
             }
 #ifdef DEBUG
             else
-                lprint(", ");
+                fprintf(stderr, ", ");
 #endif
             if (++found > 3) {
 #ifdef DEBUG
-                lprint("(more)");
+                fprintf(stderr, "(more)");
 #endif
                 break;
             }
 #ifdef DEBUG
             else
-                lprint("%s", inet_ntoa(tmp_addr));
+                fprintf(stderr, "%s", inet_ntoa(tmp_addr));
 #endif
         }
     }

slirp/slirp.h +0 -2

index e4a1bd4..6589d7e 100644
@@ -287,8 +287,6 @@ void if_start(struct ttys *);
  long gethostid(void);
 #endif
 
-void lprint(const char *, ...) GCC_FMT_ATTR(1, 2);
-
 #ifndef _WIN32
 #include <netdb.h>
 #endif

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #0 lore raw

v2 cover error: Misc cleanups and improvements

Minor error API cleanups, and one error reporting improvement when connected to qmp (patch #5)

v2:
Dropped the error_printf_unless_qmp patch Added the last 4 patches, new cleanups
Use error_report instead of fprintf in patch 1 and 2

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Cole Robinson (9):
slirp: Remove default_mon usage
vnc: Remove default_mon usage
error: Privatize error_print_loc
monitor: Remove unused monitor_print_filename
error: Print error_report() to stderr if using qmp qerror.h: Remove unused error classes
qerror.h: Remove QERR defines that are only used once qerror.h: Replace QERR_NOT_SUPPORTED with QERR_UNSUPPORTED
error: Remove some unused headers

 block/commit.c              |  2 +-
 blockdev.c                  |  9 ++++--
 blockjob.c                  |  2 +-
 hw/9pfs/virtio-9p.c         |  5 +--
 hw/core/qdev-properties.c   |  9 +++---
 hw/misc/ivshmem.c           |  4 +--
 include/monitor/monitor.h   |  1 -
 include/qapi/qmp/qerror.h   | 79 ---------------------------------------------
 include/qemu/error-report.h |  1 -
 monitor.c                   | 27 ----------------
 qapi/qmp-dispatch.c         |  3 +-
 qapi/qmp-input-visitor.c    |  2 +-
 qdev-monitor.c              | 12 ++++---
 qmp.c                       |  2 +-
 qobject/json-parser.c       |  2 +-
 qom/object.c                |  5 +--
 savevm.c                    |  3 +-
 slirp/misc.c                | 13 ++------
 slirp/slirp.c               |  8 ++---
 slirp/slirp.h               |  2 --
 stubs/Makefile.objs         |  1 -
 stubs/arch-query-cpu-def.c  |  2 +-
 stubs/mon-print-filename.c  |  6 ----
 ui/vnc.c                    |  4 +--
 util/error.c                |  5 +--
 util/qemu-config.c          |  2 +-
 util/qemu-error.c           |  4 +--
 util/qemu-option.c          |  2 +-
 28 files changed, 50 insertions(+), 167 deletions(-)
 delete mode 100644 stubs/mon-print-filename.c

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #3 lore raw

PATCH v2 3/9 error: Privatize error_print_loc

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 include/qemu/error-report.h | 1 -
 util/qemu-error.c           | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

include/qemu/error-report.h +0 -1

index 3b098a9..000eae3 100644
@@ -37,7 +37,6 @@ void loc_set_file(const char *fname, int lno);
 void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
 void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
-void error_print_loc(void);
 void error_set_progname(const char *argv0);
 void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 const char *error_get_progname(void);

util/qemu-error.c +1 -1

index fec02c6..80df49a 100644
@@ -165,7 +165,7 @@ const char *error_get_progname(void)
 /*
  * Print current location to current monitor if we have one, else to stderr.
  */
-void error_print_loc(void)
+static void error_print_loc(void)
 {
     const char *sep = "";
     int i;

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #4 lore raw

PATCH v2 4/9 monitor: Remove unused monitor_print_filename

In reply to Cole Robinson

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 include/monitor/monitor.h  |  1 -
 monitor.c                  | 27 ---------------------------
 stubs/Makefile.objs        |  1 -
 stubs/mon-print-filename.c |  6 ------
 4 files changed, 35 deletions(-)
 delete mode 100644 stubs/mon-print-filename.c

include/monitor/monitor.h +0 -1

index a49ea11..42d8671 100644
@@ -79,7 +79,6 @@ int monitor_handle_fd_param(Monitor *mon, const char *fdname);
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
     GCC_FMT_ATTR(2, 0);
 void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
-void monitor_print_filename(Monitor *mon, const char *filename);
 void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);

monitor.c +0 ~~-27~~

index 342e83b..c8428a1 100644
@@ -352,33 +352,6 @@ void monitor_printf(Monitor *mon, const char *fmt, ...)
     va_end(ap);
 }
 
-void monitor_print_filename(Monitor *mon, const char *filename)
-{
-    int i;
-
-    for (i = 0; filename[i]; i++) {
-        switch (filename[i]) {
-        case ' ':
-        case '"':
-        case '\\':
-            monitor_printf(mon, "\\%c", filename[i]);
-            break;
-        case '\t':
-            monitor_printf(mon, "\\t");
-            break;
-        case '\r':
-            monitor_printf(mon, "\\r");
-            break;
-        case '\n':
-            monitor_printf(mon, "\\n");
-            break;
-        default:
-            monitor_printf(mon, "%c", filename[i]);
-            break;
-        }
-    }
-}
-
 static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,
                                               const char *fmt, ...)
 {

stubs/Makefile.objs +0 -1

index 5ed1d38..d99e2b9 100644
@@ -14,7 +14,6 @@ stub-obj-y += iothread-lock.o
 stub-obj-y += migr-blocker.o
 stub-obj-y += mon-is-qmp.o
 stub-obj-y += mon-printf.o
-stub-obj-y += mon-print-filename.o
 stub-obj-y += mon-protocol-event.o
 stub-obj-y += mon-set-error.o
 stub-obj-y += pci-drive-hot-add.o

stubs/mon-print-filename.c deleted +0 -6

deleted file mode 100644
index 9c93964..0000000
@@ -1,6 +0,0 @@
-#include "qemu-common.h"
-#include "monitor/monitor.h"
-
-void monitor_print_filename(Monitor *mon, const char *filename)
-{
-}

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #5 lore raw

PATCH v2 5/9 error: Print error_report() to stderr if using qmp

In reply to Cole Robinson

monitor_printf will drop the requested output if cur_mon is qmp (for good reason). However these messages are often helpful for debugging issues with via libvirt.

If we know the message won't hit the monitor, send it to stderr.

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 util/qemu-error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

util/qemu-error.c +1 -1

index 80df49a..7b167fd 100644
@@ -20,7 +20,7 @@
  */
 void error_vprintf(const char *fmt, va_list ap)
 {
-    if (cur_mon) {
+    if (cur_mon && !monitor_cur_is_qmp()) {
         monitor_vprintf(cur_mon, fmt, ap);
     } else {
         vfprintf(stderr, fmt, ap);

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #6 lore raw

PATCH v2 6/9 qerror.h: Remove unused error classes

In reply to Cole Robinson

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 include/qapi/qmp/qerror.h | 9 ---------
 1 file changed, 9 deletions(-)

include/qapi/qmp/qerror.h +0 -9

index da75abf..a72bbc9 100644
@@ -35,9 +35,6 @@ void qerror_report_err(Error *err);
  * Please keep the definitions in alphabetical order.
  * Use scripts/check-qerror.sh to check.
  */
-#define QERR_ADD_CLIENT_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "Could not add client"
-
 #define QERR_AMBIGUOUS_PATH \
     ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"

@@ -200,9 +197,6 @@ void qerror_report_err(Error *err);
 #define QERR_QGA_COMMAND_FAILED \
     ERROR_CLASS_GENERIC_ERROR, "Guest agent command failed, error was '%s'"
 
-#define QERR_QGA_LOGGING_FAILED \
-    ERROR_CLASS_GENERIC_ERROR, "Guest agent failed to log non-optional log statement"
-
 #define QERR_QMP_BAD_INPUT_OBJECT \
     ERROR_CLASS_GENERIC_ERROR, "Expected '%s' in QMP input"

@@ -218,9 +212,6 @@ void qerror_report_err(Error *err);
 #define QERR_SET_PASSWD_FAILED \
     ERROR_CLASS_GENERIC_ERROR, "Could not set password"
 
-#define QERR_TOO_MANY_FILES \
-    ERROR_CLASS_GENERIC_ERROR, "Too many open files"
-
 #define QERR_UNDEFINED_ERROR \
     ERROR_CLASS_GENERIC_ERROR, "An undefined error has occurred"

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #7 lore raw

PATCH v2 7/9 qerror.h: Remove QERR defines that are only used once

In reply to Cole Robinson

Just hardcode them in the callers

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 block/commit.c            |  2 +-
 blockdev.c                |  9 ++++---
 hw/9pfs/virtio-9p.c       |  5 ++--
 hw/core/qdev-properties.c |  9 ++++---
 hw/misc/ivshmem.c         |  4 +--
 include/qapi/qmp/qerror.h | 66 -----------------------------------------------
 qapi/qmp-dispatch.c       |  3 ++-
 qapi/qmp-input-visitor.c  |  2 +-
 qdev-monitor.c            | 12 ++++++---
 qmp.c                     |  2 +-
 qobject/json-parser.c     |  2 +-
 qom/object.c              |  5 ++--
 savevm.c                  |  3 ++-
 util/qemu-config.c        |  2 +-
 util/qemu-option.c        |  2 +-
 15 files changed, 37 insertions(+), 91 deletions(-)

block/commit.c +1 -1

index acec4ac..5c09f44 100644
@@ -194,7 +194,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
     if ((on_error == BLOCKDEV_ON_ERROR_STOP ||
          on_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
         !bdrv_iostatus_is_enabled(bs)) {
-        error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+        error_setg(errp, "Invalid parameter combination");
         return;
     }

blockdev.c +6 -3

index c3422a1..2ccb257 100644
@@ -1521,14 +1521,16 @@ static void eject_device(BlockDriverState *bs, int force, Error **errp)
         return;
     }
     if (!bdrv_dev_has_removable_media(bs)) {
-        error_set(errp, QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
+        error_setg(errp, "Device '%s' is not removable",
+                   bdrv_get_device_name(bs));
         return;
     }
 
     if (bdrv_dev_is_medium_locked(bs) && !bdrv_dev_is_tray_open(bs)) {
         bdrv_dev_eject_request(bs, force);
         if (!force) {
-            error_set(errp, QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+            error_setg(errp, "Device '%s' is locked",
+                       bdrv_get_device_name(bs));
             return;
         }
     }

@@ -2216,7 +2218,8 @@ void qmp_block_job_cancel(const char *device,
         return;
     }
     if (job->paused && !force) {
-        error_set(errp, QERR_BLOCK_JOB_PAUSED, device);
+        error_setg(errp, "The block job for device '%s' is currently paused",
+                   device);
         return;
     }

hw/9pfs/virtio-9p.c +3 -2

index 83e4e93..9aa6725 100644
@@ -987,8 +987,9 @@ static void v9fs_attach(void *opaque)
      */
     if (!s->migration_blocker) {
         s->root_fid = fid;
-        error_set(&s->migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
-                  s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
+        error_setg(&s->migration_blocker,
+                   "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
+                   s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
         migrate_add_blocker(s->migration_blocker);
     }
 out:

hw/core/qdev-properties.c +5 -4

index c67acf5..585a8e9 100644
@@ -587,8 +587,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
 
     /* We rely on power-of-2 blocksizes for bitmasks */
     if ((value & (value - 1)) != 0) {
-        error_set(errp, QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
-                  dev->id?:"", name, (int64_t)value);
+        error_setg(errp,
+                  "Property %s.%s doesn't take value '%" PRId64 "', it's not a power of 2",
+                  dev->id ?: "", name, (int64_t)value);
         return;
     }

@@ -853,7 +854,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 {
     switch (ret) {
     case -EEXIST:
-        error_set(errp, QERR_PROPERTY_VALUE_IN_USE,
+        error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
                   object_get_typename(OBJECT(dev)), prop->name, value);
         break;
     default:

@@ -862,7 +863,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
                   object_get_typename(OBJECT(dev)), prop->name, value);
         break;
     case -ENOENT:
-        error_set(errp, QERR_PROPERTY_VALUE_NOT_FOUND,
+        error_setg(errp, "Property '%s.%s' can't find value '%s'",
                   object_get_typename(OBJECT(dev)), prop->name, value);
         break;
     case 0:

hw/misc/ivshmem.c +2 -2

index 8d144ba..768e528 100644
@@ -684,8 +684,8 @@ static int pci_ivshmem_init(PCIDevice *dev)
     }
 
     if (s->role_val == IVSHMEM_PEER) {
-        error_set(&s->migration_blocker, QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-                  "peer mode", "ivshmem");
+        error_setg(&s->migration_blocker,
+                   "Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
         migrate_add_blocker(s->migration_blocker);
     }

include/qapi/qmp/qerror.h +0 ~~-66~~

153 lines not on this page · raw message

qapi/qmp-dispatch.c +2 -1

index 921de33..9c61449 100644
@@ -80,7 +80,8 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
         return NULL;
     }
     if (!cmd->enabled) {
-        error_set(errp, QERR_COMMAND_DISABLED, command);
+        error_setg(errp, "The command %s has been disabled for this instance",
+                   command);
         return NULL;
     }

qapi/qmp-input-visitor.c +1 -1

index bf42c04..a2bed1e 100644
@@ -71,7 +71,7 @@ static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
     GHashTable *h;
 
     if (qiv->nb_stack >= QIV_STACK_SIZE) {
-        error_set(errp, QERR_BUFFER_OVERRUN);
+        error_setg(errp, "An internal buffer overran");
         return;
     }

qdev-monitor.c +8 -4

index 9268c87..6189780 100644
@@ -422,12 +422,14 @@ static BusState *qbus_find(const char *path)
              * one child bus accept it nevertheless */
             switch (dev->num_child_bus) {
             case 0:
-                qerror_report(QERR_DEVICE_NO_BUS, elem);
+                qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                              "Device '%s' has no child bus", elem);
                 return NULL;
             case 1:
                 return QLIST_FIRST(&dev->child_bus);
             default:
-                qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
+                qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                              "Device '%s' has multiple child busses", elem);
                 if (!monitor_cur_is_qmp()) {
                     qbus_list_bus(dev);
                 }

@@ -505,14 +507,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
             return NULL;
         }
         if (!object_dynamic_cast(OBJECT(bus), dc->bus_type)) {
-            qerror_report(QERR_BAD_BUS_FOR_DEVICE,
+            qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                          "Device '%s' can't go on a %s bus",
                           driver, object_get_typename(OBJECT(bus)));
             return NULL;
         }
     } else if (dc->bus_type != NULL) {
         bus = qbus_find_recursive(sysbus_get_default(), NULL, dc->bus_type);
         if (!bus) {
-            qerror_report(QERR_NO_BUS_FOR_DEVICE,
+            qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                          "No '%s' bus found for device '%s'",
                           dc->bus_type, driver);
             return NULL;
         }

qmp.c +1 -1

index 87a28f7..5e2a66c 100644
@@ -166,7 +166,7 @@ void qmp_cont(Error **errp)
     Error *local_err = NULL;
 
     if (runstate_needs_reset()) {
-        error_set(errp, QERR_RESET_REQUIRED);
+        error_setg(errp, "Resetting the Virtual Machine is required");
         return;
     } else if (runstate_check(RUN_STATE_SUSPENDED)) {
         return;

qobject/json-parser.c +1 -1

index e7947b3..e46c264 100644
@@ -110,7 +110,7 @@ static void GCC_FMT_ATTR(3, 4) parse_error(JSONParserContext *ctxt,
         error_free(ctxt->err);
         ctxt->err = NULL;
     }
-    error_set(&ctxt->err, QERR_JSON_PARSE_ERROR, message);
+    error_setg(&ctxt->err, "JSON parse error, %s", message);
 }
 
 /**

qom/object.c +3 -2

index f4de619..dd2f91c 100644
@@ -768,7 +768,7 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
         }
     }
 
-    error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name);
+    error_setg(errp, "Property '.%s' not found", name);
     return NULL;
 }

@@ -1075,7 +1075,8 @@ static Object *object_resolve_link(Object *obj, const char *name,
     target = object_resolve_path_type(path, target_type, &ambiguous);
 
     if (ambiguous) {
-        error_set(errp, QERR_AMBIGUOUS_PATH, path);
+        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+                  "Path '%s' does not uniquely identify an object", path);
     } else if (!target) {
         target = object_resolve_path(path, &ambiguous);
         if (target || ambiguous) {

savevm.c +2 -1

index d094fbb..274fa26 100644
@@ -450,7 +450,8 @@ bool qemu_savevm_state_blocked(Error **errp)
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (se->no_migrate) {
-            error_set(errp, QERR_MIGRATION_NOT_SUPPORTED, se->idstr);
+            error_setg(errp, "State blocked by non-migratable device '%s'",
+                       se->idstr);
             return true;
         }
     }

util/qemu-config.c +1 -1

index f610101..bcf0b86 100644
@@ -20,7 +20,7 @@ static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
             break;
     }
     if (lists[i] == NULL) {
-        error_set(errp, QERR_INVALID_OPTION_GROUP, group);
+        error_setg(errp, "There is no option group '%s'", group);
     }
     return lists[i];
 }

util/qemu-option.c +1 -1

index 9d898af..8bbc3ad 100644
@@ -819,7 +819,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
         opts = qemu_opts_find(list, id);
         if (opts != NULL) {
             if (fail_if_exists && !list->merge_lists) {
-                error_set(errp, QERR_DUPLICATE_ID, id, list->name);
+                error_setg(errp, "Duplicate ID '%s' for %s", id, list->name);
                 return NULL;
             } else {
                 return opts;

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #8 lore raw

PATCH v2 8/9 qerror.h: Replace QERR_NOT_SUPPORTED with QERR_UNSUPPORTED

In reply to Cole Robinson

The former is only used twice, the latter is used over 30 times, and has a nicer error message.

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 blockjob.c                 | 2 +-
 include/qapi/qmp/qerror.h  | 3 ---
 stubs/arch-query-cpu-def.c | 2 +-
 3 files changed, 2 insertions(+), 5 deletions(-)

blockjob.c +1 -1

index b3ce14c..cd4784f 100644
@@ -88,7 +88,7 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
     Error *local_err = NULL;
 
     if (!job->driver->set_speed) {
-        error_set(errp, QERR_NOT_SUPPORTED);
+        error_set(errp, QERR_UNSUPPORTED);
         return;
     }
     job->driver->set_speed(job, speed, &local_err);

include/qapi/qmp/qerror.h +0 -3

index 01d1d06..f5335e6 100644
@@ -122,9 +122,6 @@ void qerror_report_err(Error *err);
 #define QERR_MISSING_PARAMETER \
     ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
 
-#define QERR_NOT_SUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "Not supported"
-
 #define QERR_PERMISSION_DENIED \
     ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"

stubs/arch-query-cpu-def.c +1 -1

index fa67895..22e0b43 100644
@@ -4,6 +4,6 @@
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
-    error_set(errp, QERR_NOT_SUPPORTED);
+    error_set(errp, QERR_UNSUPPORTED);
     return NULL;
 }

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:42 UTC #9 lore raw

PATCH v2 9/9 error: Remove some unused headers

In reply to Cole Robinson

Makes it a bit clear how the interdependencies work.

Cc Luiz Capitulino <[email protected]>

Cc Markus Armbruster <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
 include/qapi/qmp/qerror.h | 1 -
 util/error.c              | 5 +----
 2 files changed, 1 insertion(+), 5 deletions(-)

include/qapi/qmp/qerror.h +0 -1

index f5335e6..902d1a7 100644
@@ -12,7 +12,6 @@
 #ifndef QERROR_H
 #define QERROR_H
 
-#include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"

util/error.c +1 -4

index f11f1d5..2bb42e1 100644
@@ -12,10 +12,7 @@
 
 #include "qemu-common.h"
 #include "qapi/error.h"
-#include "qapi/qmp/qjson.h"
-#include "qapi/qmp/qdict.h"
-#include "qapi-types.h"
-#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
 
 struct Error
 {

Signature · 1 line

1.8.5.3

Cole Robinson

author

2014-03-21 23:43 UTC #2 lore raw

PATCH v2 2/9 vnc: Remove default_mon usage

In reply to Cole Robinson

These errors don't seem user initiated, so forcibly printing to the monitor doesn't seem right. Just use error_report.

Cc Anthony Liguori <[email protected]>

Cc Gerd Hoffmann <[email protected]>

Signed-off-by Cole Robinson <[email protected]>

---
v2:
Use error_report instead of fprintf

 ui/vnc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

ui/vnc.c +2 -2

index 5925774..2d7def9 100644
@@ -996,7 +996,7 @@ static void audio_add(VncState *vs)
     struct audio_capture_ops ops;
 
     if (vs->audio_cap) {
-        monitor_printf(default_mon, "audio already running\n");
+        error_report("audio already running");
         return;
     }

@@ -1006,7 +1006,7 @@ static void audio_add(VncState *vs)
 
     vs->audio_cap = AUD_add_capture(&vs->as, &ops, vs);
     if (!vs->audio_cap) {
-        monitor_printf(default_mon, "Failed to add audio capture\n");
+        error_report("Failed to add audio capture");
     }
 }

Signature · 1 line

1.8.5.3

Paolo Bonzini

Reviewed-by on the v2 series

2014-03-22 09:07 UTC #10 lore raw

In reply to Cole Robinson

Il 22/03/2014 00:42, Cole Robinson ha scritto:

Quoted 44 lines

Minor error API cleanups, and one error reporting improvement when connected to qmp (patch #5)
v2:
Dropped the error_printf_unless_qmp patch Added the last 4 patches, new cleanups
Use error_report instead of fprintf in patch 1 and 2
Cc Luiz Capitulino <[email protected]>
Cc Markus Armbruster <[email protected]>
Cole Robinson (9):
slirp: Remove default_mon usage
vnc: Remove default_mon usage
error: Privatize error_print_loc
monitor: Remove unused monitor_print_filename
error: Print error_report() to stderr if using qmp qerror.h: Remove unused error classes
qerror.h: Remove QERR defines that are only used once qerror.h: Replace QERR_NOT_SUPPORTED with QERR_UNSUPPORTED
error: Remove some unused headers
 block/commit.c              |  2 +-
 blockdev.c                  |  9 ++++--
 blockjob.c                  |  2 +-
 hw/9pfs/virtio-9p.c         |  5 +--
 hw/core/qdev-properties.c   |  9 +++---
 hw/misc/ivshmem.c           |  4 +--
 include/monitor/monitor.h   |  1 -
 include/qapi/qmp/qerror.h   | 79 ---------------------------------------------
 include/qemu/error-report.h |  1 -
 monitor.c                   | 27 ----------------
 qapi/qmp-dispatch.c         |  3 +-
 qapi/qmp-input-visitor.c    |  2 +-
 qdev-monitor.c              | 12 ++++---
 qmp.c                       |  2 +-
 qobject/json-parser.c       |  2 +-
 qom/object.c                |  5 +--
 savevm.c                    |  3 +-
 slirp/misc.c                | 13 ++------
 slirp/slirp.c               |  8 ++---
 slirp/slirp.h               |  2 --
 stubs/Makefile.objs         |  1 -
 stubs/arch-query-cpu-def.c  |  2 +-

Quoted 9 lines

 stubs/mon-print-filename.c  |  6 ----
 ui/vnc.c                    |  4 +--
 util/error.c                |  5 +--
 util/qemu-config.c          |  2 +-
 util/qemu-error.c           |  4 +--
 util/qemu-option.c          |  2 +-
 28 files changed, 50 insertions(+), 167 deletions(-)
 delete mode 100644 stubs/mon-print-filename.c

Nice diffstat. :)

Reviewed-by Paolo Bonzini <[email protected]>

Luiz Capitulino

Applied

2014-03-27 21:00 UTC #11 lore raw

In reply to Cole Robinson

Quoted 2 lines · On Fri, 21 Mar 2014 19:42:19 -0400 Cole Robinson <[email protected]> wrote:

Minor error API cleanups, and one error reporting improvement when connected to qmp (patch #5)

Applied to the qmp-next branch, very nice cleanup!