Why this is such a big job? There are two issues with a naive conversion: * Error message degradation
The error messages are worded for -device. They aren't so hot to begin with, because we typically have many -device, and to which one a message applies is often not obvious. Now, QMP wants relatively generic errors. For instance, "-device: no driver specified" becomes "Parameter 'driver' is missing". That's just too mean to users. However, if you know *where* the parameter is missing, the generic message is perfectly adequate: "-device a=b: Parameter 'driver' is missing". In fact, this is even superior to the old message. So the first part of the patch series is about error locations. I feel it's very useful all by itself. I can split it off into its own patch series. But then the rest of this series depends on it, so I'm not sure splitting is all that useful. However, if you think the first part is ready for commit while the other parts are not, please feel free to commit just the first part. We may still encounter cases where a generic message is not adequate even with precise location information. We'll solve that problem when we hit it. * String argument with option syntax, i.e. NAME=VALUE,... QMP uses JSON to encode collections of name/value pairs. Adding a second encoding for the same thing would be a mistake, in my opinion. Note that we already have two competing dictionary data types in our code, though: QDict and QemuOpts. We should not permit that to leak into an external protocol like QMP. QemuOpts originated in the command line and spread from there into a few monitor commands, including device_add, and a few internal interfaces. QDict originated in the monitor. It sits right at the interface between monitor core and command handlers. In the long run, we might be better off standardizing on a single dictionary data type in our code, but that's clearly out of scope for this series. My proposed solution is modest and pragmatic: * Lift parsing of option strings into QemuOpts from monitor handlers up into the human monitor core. This removes QemuOpts from the handler interface, and thus avoids leaking it into QMP. It's exactly what we've done for other argument types with syntax inappropriate for QMP, such as arguments of migrate_set_speed and migrate_set_downtime. * Monitor handlers that need to pass their arguments in QemuOpts-form to internal interfaces use a converter function to translate from QDict to QemuOpts. This is what makes up the bulk of the patch series' last part. If you'd prefer a different solution, let's talk. I can split it off into its own patch series if that helps. However, the patches before it aren't all that useful without it, so I'm not sure splitting buys us much. As this series is struggling with obesity, I refrained from covering the general case in a few places where we don't need it yet. They are all clearly marked in commit messages and code. A possible alternative is to add the concept of optional named arguments to the monitor. Instead of encoding multiple optional named arguments in a single positional argument, we encode them as multiple named arguments. For instance, "device_add ide-drive,drive=hda,bus=ide.0,unit=0" becomes "device_add ide-drive drive=hda bus=ide.0 unit=0". Of course, if you think that adding a second encoding for collections of name/value pairs to QMP is fine, then this last part becomes much simpler. So, the series starts with error locations (part I), and ends with the conversion of device_add to QObject, taking care to keep QemuOpts out of QMP (part III). Wedged in between is the conversion of device_add to QError (part II). In more detail: Part I: Error Locations [01-07] Preliminary cleanup & fixes [08] Separate "default monitor" and "current monitor" cleanly [09-17] More cleanup [18-22] Error Locations Part II: Convert device_add to QError [23-26] Preliminary qdev cleanup & fixes [27-44] Convert device_add to QError Part III Convert device_add to QObject [45] Conversions between QDict and QemuOpts [46-48] New monitor argument type O [49-50] Convert device_add to QObject Not much changed since the RFC: * Restore accidentally lost newline in scsi_hot_add() [PATCH 15]. * Rename qemu_error() and qemu_error_new() to error_report() and qerror_report() [PATCH 16-17]. Requested by Luiz & Paolo. * Prettier struct Location [PATCH 18,20,22]. Requested by Luiz. * Name new function monitor_cur_is_qmp() instead of in_qmp_mon() [PATCH 27,38,50]. Requested by Luiz. * Let converted handlers print in human monitor [PATCH 28]. * Make QMP accept non-string arguments for monitor argument type 'O' [PATCH 45]. * Restrict new monitor argument type 'O' to lists with empty desc for now, because that's all that is exercised in this series [PATCH 48]. * Fix typo in device_add help text [PATCH 49]. * More comments. * Rebased. Also reordered for better legibility (affects PATCH 13-16). Markus Armbruster (50): usb: Remove disabled monitor_printf() in usb_read_file() savevm: Fix -loadvm to report errors to stderr, not the monitor pc: Fix error reporting for -boot once pc: Factor common code out of pc_boot_set() and cmos_init() tools: Remove unused cur_mon from qemu-tool.c monitor: Separate "default monitor" and "current monitor" cleanly block: Simplify usb_msd_initfn() test for "can read bdrv key" monitor: Factor monitor_set_error() out of qemu_error_internal() error: Move qemu_error() & friends from monitor.c to own file error: Simplify error sink setup error: Move qemu_error & friends into their own header error: New error_printf() and error_vprintf() error: Don't abuse qemu_error() for non-error in qdev_device_help() error: Don't abuse qemu_error() for non-error in qbus_find() error: Don't abuse qemu_error() for non-error in scsi_hot_add() error: Replace qemu_error() by error_report() error: Rename qemu_error_new() to qerror_report() error: Infrastructure to track locations for error reporting error: Include the program name in error messages to stderr error: Track locations in configuration files QemuOpts: Fix qemu_config_parse() to catch file read errors error: Track locations on command line qdev: Fix -device and device_add to handle unsuitable bus gracefully qdev: Factor qdev_create_from_info() out of qdev_create() qdev: Hide "no_user" devices from users qdev: Hide "ptr" properties from users monitor: New monitor_cur_is_qmp() error: Let converted handlers print in human monitor error: Polish human-readable error descriptions error: New QERR_PROPERTY_NOT_FOUND error: New QERR_PROPERTY_VALUE_BAD qdev: convert setting device properties to QError qdev: Relax parsing of bus option error: New QERR_BUS_NOT_FOUND error: New QERR_DEVICE_MULTIPLE_BUSSES error: New QERR_DEVICE_NO_BUS qdev: Convert qbus_find() to QError error: New error_printf_unless_qmp() error: New QERR_BAD_BUS_FOR_DEVICE error: New QERR_BUS_NO_HOTPLUG error: New QERR_DEVICE_INIT_FAILED error: New QERR_NO_BUS_FOR_DEVICE Revert "qdev: Use QError for 'device not found' error" error: Convert do_device_add() to QError qemu-option: Functions to convert to/from QDict qemu-option: Move the implied first name into QemuOptsList qemu-option: Rename find_list() to qemu_find_opts() & external linkage monitor: New argument type 'O' monitor: Use argument type 'O' for device_add monitor: convert do_device_add() to QObject Makefile.target | 1 + audio/audio.c | 4 +- hw/pc.c | 35 ++---- hw/pci-hotplug.c | 14 +- hw/pci.c | 14 +- hw/qdev-properties.c | 27 ++--- hw/qdev.c | 236 ++++++++++++++++++++-------------- hw/qdev.h | 2 +- hw/scsi-bus.c | 4 +- hw/scsi-disk.c | 5 +- hw/scsi-generic.c | 9 +- hw/usb-bus.c | 4 +- hw/usb-msd.c | 4 +- hw/usb-net.c | 2 +- hw/usb-serial.c | 9 +- hw/virtio-net.c | 5 +- hw/virtio-pci.c | 4 +- hw/virtio-serial-bus.c | 2 +- monitor.c | 337 +++++++++++++++++++++--------------------------- monitor.h | 7 + net.c | 32 +++--- net/dump.c | 5 +- net/slirp.c | 28 ++-- net/socket.c | 12 +- net/tap-bsd.c | 7 +- net/tap-linux.c | 9 +- net/tap-solaris.c | 4 +- net/tap-win32.c | 2 +- net/tap.c | 3 +- qemu-config.c | 56 +++++--- qemu-config.h | 3 +- qemu-error.c | 227 ++++++++++++++++++++++++++++++++ qemu-error.h | 47 +++++++ qemu-monitor.hx | 7 +- qemu-option.c | 93 +++++++++++++- qemu-option.h | 6 +- qemu-tool.c | 30 ++++- qerror.c | 75 ++++++++--- qerror.h | 45 ++++++- savevm.c | 27 ++-- slirp/misc.c | 2 +- sysemu.h | 13 +-- usb-linux.c | 8 - vl.c | 44 ++++--- vnc.c | 5 +- 45 files changed, 987 insertions(+), 528 deletions(-) create mode 100644 qemu-error.c create mode 100644 qemu-error.h