From 02d16089802234f4883a4792300889d2823cbb77 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:20 -0400 Subject: [PATCH 01/16] slirp: Remove default_mon usage 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 Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- slirp/misc.c | 13 ++----------- slirp/slirp.c | 8 ++++---- slirp/slirp.h | 2 -- 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/slirp/misc.c b/slirp/misc.c index 6c1636f7b6..b8eb74cab0 100644 --- a/slirp/misc.c +++ b/slirp/misc.c @@ -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[] = { diff --git a/slirp/slirp.c b/slirp/slirp.c index bad8dad02e..3fb48a4921 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -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 } } diff --git a/slirp/slirp.h b/slirp/slirp.h index e4a1bd4abb..6589d7eef0 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -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 #endif From 027a79c373954920d5e9a1d0bafe1f315a2cd624 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:21 -0400 Subject: [PATCH 02/16] vnc: Remove default_mon usage These errors don't seem user initiated, so forcibly printing to the monitor doesn't seem right. Just use error_report. Cc: Anthony Liguori Cc: Gerd Hoffmann Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- ui/vnc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 5925774509..2d7def9aa2 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -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"); } } From d876f60d14d491c27719bafeaed21669e15348a3 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:22 -0400 Subject: [PATCH 03/16] error: Privatize error_print_loc Cc: Luiz Capitulino Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- include/qemu/error-report.h | 1 - util/qemu-error.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 3b098a9173..000eae3957 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -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); diff --git a/util/qemu-error.c b/util/qemu-error.c index fec02c6075..80df49a874 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -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; From 4a66d3bf9ad3d121c32375081dc1379c3478fdb8 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:23 -0400 Subject: [PATCH 04/16] monitor: Remove unused monitor_print_filename Cc: Luiz Capitulino Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- 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 diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index a49ea11eb4..42d867155b 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -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); diff --git a/monitor.c b/monitor.c index 342e83baea..c8428a12db 100644 --- a/monitor.c +++ b/monitor.c @@ -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, ...) { diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 5ed1d38d70..d99e2b9259 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -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 diff --git a/stubs/mon-print-filename.c b/stubs/mon-print-filename.c deleted file mode 100644 index 9c939641ff..0000000000 --- a/stubs/mon-print-filename.c +++ /dev/null @@ -1,6 +0,0 @@ -#include "qemu-common.h" -#include "monitor/monitor.h" - -void monitor_print_filename(Monitor *mon, const char *filename) -{ -} From 4ad417baa43424b6b988c52b83989fd95670c113 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:24 -0400 Subject: [PATCH 05/16] error: Print error_report() to stderr if using qmp 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 Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- util/qemu-error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/qemu-error.c b/util/qemu-error.c index 80df49a874..7b167fd06b 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -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); From d73f0beadb57f885e678bffc362864f4401262e0 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:25 -0400 Subject: [PATCH 06/16] qerror.h: Remove unused error classes Cc: Luiz Capitulino Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- include/qapi/qmp/qerror.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index da75abf6d6..a72bbc9850 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -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" From f231b88db14f13ee9a41599896f57f3594c1ca8b Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:26 -0400 Subject: [PATCH 07/16] qerror.h: Remove QERR defines that are only used once Just hardcode them in the callers Cc: Luiz Capitulino Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- 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(-) diff --git a/block/commit.c b/block/commit.c index acec4ac5a8..5c09f4444e 100644 --- a/block/commit.c +++ b/block/commit.c @@ -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; } diff --git a/blockdev.c b/blockdev.c index 09826f10cf..9486358e48 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1520,14 +1520,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; } } @@ -2219,7 +2221,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; } diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index 83e4e93983..9aa6725f09 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -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: diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index c67acf58b5..585a8e902e 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -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: diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index 8d144baa1e..768e5288bc 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -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); } diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index a72bbc9850..01d1d0661c 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -35,48 +35,30 @@ void qerror_report_err(Error *err); * Please keep the definitions in alphabetical order. * Use scripts/check-qerror.sh to check. */ -#define QERR_AMBIGUOUS_PATH \ - ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object" - -#define QERR_BAD_BUS_FOR_DEVICE \ - ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus" - #define QERR_BASE_NOT_FOUND \ ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found" #define QERR_BLOCK_JOB_NOT_ACTIVE \ ERROR_CLASS_DEVICE_NOT_ACTIVE, "No active block job on device '%s'" -#define QERR_BLOCK_JOB_PAUSED \ - ERROR_CLASS_GENERIC_ERROR, "The block job for device '%s' is currently paused" - #define QERR_BLOCK_JOB_NOT_READY \ ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be completed" #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \ ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'" -#define QERR_BUFFER_OVERRUN \ - ERROR_CLASS_GENERIC_ERROR, "An internal buffer overran" - #define QERR_BUS_NO_HOTPLUG \ ERROR_CLASS_GENERIC_ERROR, "Bus '%s' does not support hotplugging" #define QERR_BUS_NOT_FOUND \ ERROR_CLASS_GENERIC_ERROR, "Bus '%s' not found" -#define QERR_COMMAND_DISABLED \ - ERROR_CLASS_GENERIC_ERROR, "The command %s has been disabled for this instance" - #define QERR_COMMAND_NOT_FOUND \ ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found" #define QERR_DEVICE_ENCRYPTED \ ERROR_CLASS_DEVICE_ENCRYPTED, "'%s' (%s) is encrypted" -#define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \ - ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when using feature '%s' in device '%s'" - #define QERR_DEVICE_HAS_NO_MEDIUM \ ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no medium" @@ -89,15 +71,6 @@ void qerror_report_err(Error *err); #define QERR_DEVICE_IS_READ_ONLY \ ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only" -#define QERR_DEVICE_LOCKED \ - ERROR_CLASS_GENERIC_ERROR, "Device '%s' is locked" - -#define QERR_DEVICE_MULTIPLE_BUSSES \ - ERROR_CLASS_GENERIC_ERROR, "Device '%s' has multiple child busses" - -#define QERR_DEVICE_NO_BUS \ - ERROR_CLASS_GENERIC_ERROR, "Device '%s' has no child bus" - #define QERR_DEVICE_NO_HOTPLUG \ ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging" @@ -110,12 +83,6 @@ void qerror_report_err(Error *err); #define QERR_DEVICE_NOT_FOUND \ ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found" -#define QERR_DEVICE_NOT_REMOVABLE \ - ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not removable" - -#define QERR_DUPLICATE_ID \ - ERROR_CLASS_GENERIC_ERROR, "Duplicate ID '%s' for %s" - #define QERR_FD_NOT_FOUND \ ERROR_CLASS_GENERIC_ERROR, "File descriptor named '%s' not found" @@ -128,15 +95,9 @@ void qerror_report_err(Error *err); #define QERR_INVALID_BLOCK_FORMAT \ ERROR_CLASS_GENERIC_ERROR, "Invalid block format '%s'" -#define QERR_INVALID_OPTION_GROUP \ - ERROR_CLASS_GENERIC_ERROR, "There is no option group '%s'" - #define QERR_INVALID_PARAMETER \ ERROR_CLASS_GENERIC_ERROR, "Invalid parameter '%s'" -#define QERR_INVALID_PARAMETER_COMBINATION \ - ERROR_CLASS_GENERIC_ERROR, "Invalid parameter combination" - #define QERR_INVALID_PARAMETER_TYPE \ ERROR_CLASS_GENERIC_ERROR, "Invalid parameter type for '%s', expected: %s" @@ -149,9 +110,6 @@ void qerror_report_err(Error *err); #define QERR_IO_ERROR \ ERROR_CLASS_GENERIC_ERROR, "An IO error has occurred" -#define QERR_JSON_PARSE_ERROR \ - ERROR_CLASS_GENERIC_ERROR, "JSON parse error, %s" - #define QERR_JSON_PARSING \ ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax" @@ -161,36 +119,18 @@ void qerror_report_err(Error *err); #define QERR_MIGRATION_ACTIVE \ ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress" -#define QERR_MIGRATION_NOT_SUPPORTED \ - ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device '%s'" - #define QERR_MISSING_PARAMETER \ ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing" -#define QERR_NO_BUS_FOR_DEVICE \ - ERROR_CLASS_GENERIC_ERROR, "No '%s' bus found for device '%s'" - #define QERR_NOT_SUPPORTED \ ERROR_CLASS_GENERIC_ERROR, "Not supported" #define QERR_PERMISSION_DENIED \ ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation" -#define QERR_PROPERTY_NOT_FOUND \ - ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' not found" - #define QERR_PROPERTY_VALUE_BAD \ ERROR_CLASS_GENERIC_ERROR, "Property '%s.%s' doesn't take value '%s'" -#define QERR_PROPERTY_VALUE_IN_USE \ - 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, "Property '%s.%s' can't find value '%s'" - -#define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \ - 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, "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" @@ -206,9 +146,6 @@ void qerror_report_err(Error *err); #define QERR_QMP_EXTRA_MEMBER \ ERROR_CLASS_GENERIC_ERROR, "QMP input object member '%s' is unexpected" -#define QERR_RESET_REQUIRED \ - ERROR_CLASS_GENERIC_ERROR, "Resetting the Virtual Machine is required" - #define QERR_SET_PASSWD_FAILED \ ERROR_CLASS_GENERIC_ERROR, "Could not set password" @@ -221,9 +158,6 @@ void qerror_report_err(Error *err); #define QERR_UNSUPPORTED \ ERROR_CLASS_GENERIC_ERROR, "this feature or command is not currently supported" -#define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \ - ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'" - #define QERR_SOCKET_CONNECT_FAILED \ ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket" diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 921de33bce..9c614494f1 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -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; } diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index bf42c04ea6..a2bed1ef10 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -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; } diff --git a/qdev-monitor.c b/qdev-monitor.c index 9268c8759f..6189780fd7 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -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; } diff --git a/qmp.c b/qmp.c index 87a28f797d..5e2a66caab 100644 --- a/qmp.c +++ b/qmp.c @@ -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; diff --git a/qobject/json-parser.c b/qobject/json-parser.c index e7947b340c..e46c26448e 100644 --- a/qobject/json-parser.c +++ b/qobject/json-parser.c @@ -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); } /** diff --git a/qom/object.c b/qom/object.c index 9a730e74c1..e42b254303 100644 --- a/qom/object.c +++ b/qom/object.c @@ -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) { diff --git a/savevm.c b/savevm.c index 22123be4f9..da8aa2428d 100644 --- a/savevm.c +++ b/savevm.c @@ -453,7 +453,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; } } diff --git a/util/qemu-config.c b/util/qemu-config.c index f6101012c0..bcf0b86f91 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -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]; } diff --git a/util/qemu-option.c b/util/qemu-option.c index 9d898af443..8bbc3ad4a3 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -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; From 0b15abfcbcb6d4f658bf92d618386f79a7f7e9e8 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:27 -0400 Subject: [PATCH 08/16] qerror.h: Replace QERR_NOT_SUPPORTED with QERR_UNSUPPORTED The former is only used twice, the latter is used over 30 times, and has a nicer error message. Cc: Luiz Capitulino Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- blockjob.c | 2 +- include/qapi/qmp/qerror.h | 3 --- stubs/arch-query-cpu-def.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/blockjob.c b/blockjob.c index b3ce14cebd..cd4784f053 100644 --- a/blockjob.c +++ b/blockjob.c @@ -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); diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 01d1d0661c..f5335e67d6 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -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" diff --git a/stubs/arch-query-cpu-def.c b/stubs/arch-query-cpu-def.c index fa6789598a..22e0b43de9 100644 --- a/stubs/arch-query-cpu-def.c +++ b/stubs/arch-query-cpu-def.c @@ -4,6 +4,6 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) { - error_set(errp, QERR_NOT_SUPPORTED); + error_set(errp, QERR_UNSUPPORTED); return NULL; } From 073a341151ecd02b4d4b7ae01ce7c3ff4e9b71f8 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Fri, 21 Mar 2014 19:42:28 -0400 Subject: [PATCH 09/16] error: Remove some unused headers Makes it a bit clear how the interdependencies work. Cc: Luiz Capitulino Cc: Markus Armbruster Signed-off-by: Cole Robinson Reviewed-by: Paolo Bonzini Signed-off-by: Luiz Capitulino --- include/qapi/qmp/qerror.h | 1 - util/error.c | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index f5335e67d6..902d1a7a18 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -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" diff --git a/util/error.c b/util/error.c index f11f1d57a0..2bb42e1c4b 100644 --- a/util/error.c +++ b/util/error.c @@ -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 { From f7bdc41acc0f9e3f35562ff6a2c37d1a09b79b7b Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Sun, 13 Apr 2014 16:25:05 +0100 Subject: [PATCH 10/16] monitor: Fix drive_del id argument type completion. Signed-off-by: Hani Benhabiles Signed-off-by: Luiz Capitulino --- hmp-commands.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index f3fc514427..6bf47972e5 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -176,7 +176,7 @@ ETEXI { .name = "drive_del", - .args_type = "id:s", + .args_type = "id:B", .params = "device", .help = "remove host block device", .user_print = monitor_user_noop, From bfa40f77af05731e2e976c5aa8c4edc86a87acd3 Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Sun, 13 Apr 2014 16:25:06 +0100 Subject: [PATCH 11/16] monitor: Add command_completion callback to mon_cmd_t. Convert object_add and object_del commands to use the new callback. Signed-off-by: Hani Benhabiles Signed-off-by: Luiz Capitulino --- hmp-commands.hx | 2 ++ hmp.h | 3 +++ monitor.c | 19 +++++++++++++------ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 6bf47972e5..1b382b6bf2 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1254,6 +1254,7 @@ ETEXI .params = "[qom-type=]type,id=str[,prop=value][,...]", .help = "create QOM object", .mhandler.cmd = hmp_object_add, + .command_completion = object_add_completion, }, STEXI @@ -1268,6 +1269,7 @@ ETEXI .params = "id", .help = "destroy QOM object", .mhandler.cmd = hmp_object_del, + .command_completion = object_del_completion, }, STEXI diff --git a/hmp.h b/hmp.h index ed58f0ea41..2f2c059e00 100644 --- a/hmp.h +++ b/hmp.h @@ -15,6 +15,7 @@ #define HMP_H #include "qemu-common.h" +#include "qemu/readline.h" #include "qapi-types.h" #include "qapi/qmp/qdict.h" @@ -92,5 +93,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict); void hmp_cpu_add(Monitor *mon, const QDict *qdict); void hmp_object_add(Monitor *mon, const QDict *qdict); void hmp_object_del(Monitor *mon, const QDict *qdict); +void object_add_completion(ReadLineState *rs, int nb_args, const char *str); +void object_del_completion(ReadLineState *rs, int nb_args, const char *str); #endif diff --git a/monitor.c b/monitor.c index c8428a12db..49578f8a01 100644 --- a/monitor.c +++ b/monitor.c @@ -137,6 +137,7 @@ typedef struct mon_cmd_t { * used, and mhandler of 1st level plays the role of help function. */ struct mon_cmd_t *sub_table; + void (*command_completion)(ReadLineState *rs, int nb_args, const char *str); } mon_cmd_t; /* file descriptors passed via SCM_RIGHTS */ @@ -4271,11 +4272,15 @@ static void device_add_completion(ReadLineState *rs, const char *str) g_slist_free(list); } -static void object_add_completion(ReadLineState *rs, const char *str) +void object_add_completion(ReadLineState *rs, int nb_args, const char *str) { GSList *list, *elt; size_t len; + if (nb_args != 2) { + return; + } + len = strlen(str); readline_set_completion_index(rs, len); list = elt = object_class_get_list(TYPE_USER_CREATABLE, false); @@ -4310,11 +4315,14 @@ static void device_del_completion(ReadLineState *rs, BusState *bus, } } -static void object_del_completion(ReadLineState *rs, const char *str) +void object_del_completion(ReadLineState *rs, int nb_args, const char *str) { ObjectPropertyInfoList *list, *start; size_t len; + if (nb_args != 2) { + return; + } len = strlen(str); readline_set_completion_index(rs, len); @@ -4368,6 +4376,9 @@ static void monitor_find_completion_by_table(Monitor *mon, return monitor_find_completion_by_table(mon, cmd->sub_table, &args[1], nb_args - 1); } + if (cmd->command_completion) { + return cmd->command_completion(mon->rs, nb_args, args[nb_args - 1]); + } ptype = next_arg_type(cmd->args_type); for(i = 0; i < nb_args - 2; i++) { @@ -4397,8 +4408,6 @@ static void monitor_find_completion_by_table(Monitor *mon, case 'O': if (!strcmp(cmd->name, "device_add") && nb_args == 2) { device_add_completion(mon->rs, str); - } else if (!strcmp(cmd->name, "object_add") && nb_args == 2) { - object_add_completion(mon->rs, str); } break; case 's': @@ -4418,8 +4427,6 @@ static void monitor_find_completion_by_table(Monitor *mon, size_t len = strlen(str); readline_set_completion_index(mon->rs, len); device_del_completion(mon->rs, sysbus_get_default(), str, len); - } else if (!strcmp(cmd->name, "object_del") && nb_args == 2) { - object_del_completion(mon->rs, str); } break; default: From 2da1b3abbccd267f09ebda7ba604fbbb0220f406 Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Sun, 13 Apr 2014 16:25:07 +0100 Subject: [PATCH 12/16] monitor: Add device_add and device_del completion. Also fix device_add completion including non-hotpluggable devices. Signed-off-by: Hani Benhabiles Signed-off-by: Luiz Capitulino --- hmp-commands.hx | 2 ++ hmp.h | 2 ++ monitor.c | 38 ++++++++++++++++++++++++-------------- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 1b382b6bf2..4c4d2619c6 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -658,6 +658,7 @@ ETEXI .help = "add device, like -device on the command line", .user_print = monitor_user_noop, .mhandler.cmd_new = do_device_add, + .command_completion = device_add_completion, }, STEXI @@ -673,6 +674,7 @@ ETEXI .params = "device", .help = "remove device", .mhandler.cmd = hmp_device_del, + .command_completion = device_del_completion, }, STEXI diff --git a/hmp.h b/hmp.h index 2f2c059e00..20ef454b4a 100644 --- a/hmp.h +++ b/hmp.h @@ -95,5 +95,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict); void hmp_object_del(Monitor *mon, const QDict *qdict); void object_add_completion(ReadLineState *rs, int nb_args, const char *str); void object_del_completion(ReadLineState *rs, int nb_args, const char *str); +void device_add_completion(ReadLineState *rs, int nb_args, const char *str); +void device_del_completion(ReadLineState *rs, int nb_args, const char *str); #endif diff --git a/monitor.c b/monitor.c index 49578f8a01..9ad857820b 100644 --- a/monitor.c +++ b/monitor.c @@ -4251,11 +4251,15 @@ static const char *next_arg_type(const char *typestr) return (p != NULL ? ++p : typestr); } -static void device_add_completion(ReadLineState *rs, const char *str) +void device_add_completion(ReadLineState *rs, int nb_args, const char *str) { GSList *list, *elt; size_t len; + if (nb_args != 2) { + return; + } + len = strlen(str); readline_set_completion_index(rs, len); list = elt = object_class_get_list(TYPE_DEVICE, false); @@ -4264,7 +4268,9 @@ static void device_add_completion(ReadLineState *rs, const char *str) DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, elt->data, TYPE_DEVICE); name = object_class_get_name(OBJECT_CLASS(dc)); - if (!strncmp(name, str, len)) { + + if (!dc->cannot_instantiate_with_device_add_yet + && !strncmp(name, str, len)) { readline_add_completion(rs, name); } elt = elt->next; @@ -4296,8 +4302,8 @@ void object_add_completion(ReadLineState *rs, int nb_args, const char *str) g_slist_free(list); } -static void device_del_completion(ReadLineState *rs, BusState *bus, - const char *str, size_t len) +static void device_del_bus_completion(ReadLineState *rs, BusState *bus, + const char *str, size_t len) { BusChild *kid; @@ -4310,11 +4316,24 @@ static void device_del_completion(ReadLineState *rs, BusState *bus, } QLIST_FOREACH(dev_child, &dev->child_bus, sibling) { - device_del_completion(rs, dev_child, str, len); + device_del_bus_completion(rs, dev_child, str, len); } } } +void device_del_completion(ReadLineState *rs, int nb_args, const char *str) +{ + size_t len; + + if (nb_args != 2) { + return; + } + + len = strlen(str); + readline_set_completion_index(rs, len); + device_del_bus_completion(rs, sysbus_get_default(), str, len); +} + void object_del_completion(ReadLineState *rs, int nb_args, const char *str) { ObjectPropertyInfoList *list, *start; @@ -4405,11 +4424,6 @@ static void monitor_find_completion_by_table(Monitor *mon, readline_set_completion_index(mon->rs, strlen(str)); bdrv_iterate(block_completion_it, &mbs); break; - case 'O': - if (!strcmp(cmd->name, "device_add") && nb_args == 2) { - device_add_completion(mon->rs, str); - } - break; case 's': case 'S': if (!strcmp(cmd->name, "sendkey")) { @@ -4423,10 +4437,6 @@ static void monitor_find_completion_by_table(Monitor *mon, } else if (!strcmp(cmd->name, "help|?")) { monitor_find_completion_by_table(mon, cmd_table, &args[1], nb_args - 1); - } else if (!strcmp(cmd->name, "device_del") && nb_args == 2) { - size_t len = strlen(str); - readline_set_completion_index(mon->rs, len); - device_del_completion(mon->rs, sysbus_get_default(), str, len); } break; default: From c3481247e58ff3f13337ce0a262b058799bd156c Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Wed, 16 Apr 2014 14:39:38 -0300 Subject: [PATCH 13/16] qmp: object-add: Validate class before creating object Currently it is very easy to crash QEMU by issuing an object-add command using an abstract class or a class that doesn't support TYPE_USER_CREATABLE as parameter. Example: with the following QMP command: (QEMU) object-add qom-type=cpu id=foo QEMU aborts at: ERROR:qom/object.c:335:object_initialize_with_type: assertion failed: (type->abstract == false) This patch moves the check for TYPE_USER_CREATABLE before object_new(), and adds a check to prevent the code from trying to instantiate abstract classes. Signed-off-by: Eduardo Habkost Reviewed-by: Matthew Rosato Tested-by: Matthew Rosato Signed-off-by: Luiz Capitulino --- qmp.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/qmp.c b/qmp.c index 5e2a66caab..74107be41b 100644 --- a/qmp.c +++ b/qmp.c @@ -540,14 +540,27 @@ void object_add(const char *type, const char *id, const QDict *qdict, Visitor *v, Error **errp) { Object *obj; + ObjectClass *klass; const QDictEntry *e; Error *local_err = NULL; - if (!object_class_by_name(type)) { + klass = object_class_by_name(type); + if (!klass) { error_setg(errp, "invalid class name"); return; } + if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) { + error_setg(errp, "object type '%s' isn't supported by object-add", + type); + return; + } + + if (object_class_is_abstract(klass)) { + error_setg(errp, "object type '%s' is abstract", type); + return; + } + obj = object_new(type); if (qdict) { for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) { @@ -558,12 +571,6 @@ void object_add(const char *type, const char *id, const QDict *qdict, } } - if (!object_dynamic_cast(obj, TYPE_USER_CREATABLE)) { - error_setg(&local_err, "object type '%s' isn't supported by object-add", - type); - goto out; - } - user_creatable_complete(obj, &local_err); if (local_err) { goto out; From c20499d9854e5d04710f17d577df65ed7c7aacf1 Mon Sep 17 00:00:00 2001 From: Qiao Nuohan Date: Thu, 17 Apr 2014 16:15:06 +0800 Subject: [PATCH 14/16] HMP: fix doc of dump-guest-memory Signed-off-by: Qiao Nuohan Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- hmp-commands.hx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 4c4d2619c6..d79cb977a9 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1001,25 +1001,26 @@ ETEXI { .name = "dump-guest-memory", .args_type = "paging:-p,filename:F,begin:i?,length:i?", - .params = "[-p] filename [begin] [length]", - .help = "dump guest memory to file" - "\n\t\t\t begin(optional): the starting physical address" - "\n\t\t\t length(optional): the memory size, in bytes", + .params = "[-p] filename [begin length]", + .help = "dump guest memory into file 'filename'.\n\t\t\t" + "-p: do paging to get guest's memory mapping.\n\t\t\t" + "begin: the starting physical address.\n\t\t\t" + "length: the memory size, in bytes.", .mhandler.cmd = hmp_dump_guest_memory, }, STEXI -@item dump-guest-memory [-p] @var{protocol} @var{begin} @var{length} +@item dump-guest-memory [-p] @var{filename} @var{begin} @var{length} @findex dump-guest-memory Dump guest memory to @var{protocol}. The file can be processed with crash or gdb. - filename: dump file name - paging: do paging to get guest's memory mapping + -p: do paging to get guest's memory mapping. + filename: dump file name. begin: the starting physical address. It's optional, and should be - specified with length together. + specified together with length. length: the memory size, in bytes. It's optional, and should be specified - with begin together. + together with begin. ETEXI { From 1b7a0f758bb3e49c8468fd2af75ebb215b26e6f4 Mon Sep 17 00:00:00 2001 From: Qiao Nuohan Date: Thu, 17 Apr 2014 16:15:07 +0800 Subject: [PATCH 15/16] HMP: support specifying dump format for dump-guest-memory Dumping guest memory is available to specify the dump format now. This patch adds options '-z|-l|-s' to HMP command dump-guest-memory to specify dumping in kdump-compression format, with zlib/lzo/snappy compression. And without these options ELF format will be used. The discussion about this feature is here: http://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg04235.html Signed-off-by: Qiao Nuohan Suggested-by: Christian Borntraeger Tested-by: Christian Borntraeger (on s390x/kvm) Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- hmp-commands.hx | 13 ++++++++++--- hmp.c | 25 ++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index d79cb977a9..8971f1b153 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1000,10 +1000,13 @@ ETEXI { .name = "dump-guest-memory", - .args_type = "paging:-p,filename:F,begin:i?,length:i?", - .params = "[-p] filename [begin length]", + .args_type = "paging:-p,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:i?,length:i?", + .params = "[-p] [-z|-l|-s] filename [begin length]", .help = "dump guest memory into file 'filename'.\n\t\t\t" "-p: do paging to get guest's memory mapping.\n\t\t\t" + "-z: dump in kdump-compressed format, with zlib compression.\n\t\t\t" + "-l: dump in kdump-compressed format, with lzo compression.\n\t\t\t" + "-s: dump in kdump-compressed format, with snappy compression.\n\t\t\t" "begin: the starting physical address.\n\t\t\t" "length: the memory size, in bytes.", .mhandler.cmd = hmp_dump_guest_memory, @@ -1012,10 +1015,14 @@ ETEXI STEXI @item dump-guest-memory [-p] @var{filename} @var{begin} @var{length} +@item dump-guest-memory [-z|-l|-s] @var{filename} @findex dump-guest-memory Dump guest memory to @var{protocol}. The file can be processed with crash or -gdb. +gdb. Without -z|-l|-s, the dump format is ELF. -p: do paging to get guest's memory mapping. + -z: dump in kdump-compressed format, with zlib compression. + -l: dump in kdump-compressed format, with lzo compression. + -s: dump in kdump-compressed format, with snappy compression. filename: dump file name. begin: the starting physical address. It's optional, and should be specified together with length. diff --git a/hmp.c b/hmp.c index 2f279c4ff2..ca869bafa8 100644 --- a/hmp.c +++ b/hmp.c @@ -1308,16 +1308,35 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) { Error *errp = NULL; int paging = qdict_get_try_bool(qdict, "paging", 0); + int zlib = qdict_get_try_bool(qdict, "zlib", 0); + int lzo = qdict_get_try_bool(qdict, "lzo", 0); + int snappy = qdict_get_try_bool(qdict, "snappy", 0); const char *file = qdict_get_str(qdict, "filename"); bool has_begin = qdict_haskey(qdict, "begin"); bool has_length = qdict_haskey(qdict, "length"); - /* kdump-compressed format is not supported for HMP */ - bool has_format = false; int64_t begin = 0; int64_t length = 0; enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF; char *prot; + if (zlib + lzo + snappy > 1) { + error_setg(&errp, "only one of '-z|-l|-s' can be set"); + hmp_handle_error(mon, &errp); + return; + } + + if (zlib) { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB; + } + + if (lzo) { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO; + } + + if (snappy) { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY; + } + if (has_begin) { begin = qdict_get_int(qdict, "begin"); } @@ -1328,7 +1347,7 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) prot = g_strconcat("file:", file, NULL); qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length, - has_format, dump_format, &errp); + true, dump_format, &errp); hmp_handle_error(mon, &errp); g_free(prot); } From 0b9f0e2fd7c5070fa06cd6bd5ec69055e3a7d2b1 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 24 Apr 2014 13:58:18 +0200 Subject: [PATCH 16/16] monitor: fix qmp_getfd() fd leak in error case qemu_chr_fe_get_msgfd() transfers ownership of the file descriptor to the caller. Therefore all code paths in qmp_getfd() should either register the file descriptor somewhere or close it. Signed-off-by: Stefan Hajnoczi Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- monitor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/monitor.c b/monitor.c index 9ad857820b..1266ba06fb 100644 --- a/monitor.c +++ b/monitor.c @@ -2228,6 +2228,7 @@ void qmp_getfd(const char *fdname, Error **errp) } if (qemu_isdigit(fdname[0])) { + close(fd); error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname", "a name not starting with a digit"); return;