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: we typically have many -device options, 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".
Emitting such an error with our lengthy command lines is 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, it's even superior to our current 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.
We may still encounter cases where a generic message is not adequate
even with precise location information. Let's solve that problem
when we actually encounter 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 encodings in our code: QDict
and QemuOpts. But we should not permit that to leak into an
external interface 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.
My proposed solution is modest and pragmatic:
* Lift the parsing of arguments 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 did for other argument types with syntax
inappropriate for QMP, such as arguments of migrate_set_speed and
migrate_set_downtime (commits 9da92c49..b0fbf7d3).
* 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 the last part of the patch series is about. If you'd
prefer a different solution, let's talk.
I can split this part 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.
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
can be dropped.
So, the series starts with error locations (part I), and ends with
keeping 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-16] More cleanup
[17-21] Error Locations
Part II: Convert device_add to QError
[22-25] Preliminary qdev cleanup & fixes
[26-42] Convert device_add to QError
Part III
[43] Conversions between QDict and QemuOpts
[44-46] New monitor argument type O
[47-48] Convert device_add to QObject
I cut a few corners clearly marked in commit messages and code. I'll
fix them up for the non-RFC submit.
Markus Armbruster (48):
monitor: Factor monitor_set_error() out of qemu_error_internal()
error: Move qemu_error() & friends from monitor.c to own file
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"
error: Simplify error sink setup
error: Move qemu_error & friends into their own header
error: New error_printf() and error_vprintf()
error: Make qemu_error() add a newline, strip it from arguments
error: Don't abuse qemu_error() for non-error in scsi_hot_add()
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: 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
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
monitor: New in_qmp_mon()
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 | 13 ++--
hw/pci.c | 8 +-
hw/qdev-properties.c | 27 ++----
hw/qdev.c | 221 +++++++++++++++++++++++++++--------------------
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 | 191 ++++++++++++++++++------------------------
monitor.h | 7 ++
net.c | 28 +++---
net/dump.c | 5 +-
net/slirp.c | 20 ++--
net/socket.c | 12 ++--
net/tap-bsd.c | 3 +-
net/tap-linux.c | 5 +-
net/tap-win32.c | 2 +-
net/tap.c | 3 +-
qemu-config.c | 54 ++++++++----
qemu-config.h | 3 +-
qemu-error.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++
qemu-error.h | 47 ++++++++++
qemu-monitor.hx | 7 +-
qemu-option.c | 68 +++++++++++++++-
qemu-option.h | 6 +-
qemu-tool.c | 28 ++++++-
qerror.c | 73 ++++++++++++----
qerror.h | 45 ++++++++--
savevm.c | 27 +++---
slirp/misc.c | 2 +-
sysemu.h | 13 +---
usb-linux.c | 8 --
vl.c | 44 ++++++----
vnc.c | 5 +-
44 files changed, 858 insertions(+), 429 deletions(-)
create mode 100644 qemu-error.c
create mode 100644 qemu-error.h
