[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError |
Date: |
Thu, 4 Mar 2010 16:56:21 +0100 |
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
- [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError,
Markus Armbruster <=
- [Qemu-devel] [PATCH 01/50] usb: Remove disabled monitor_printf() in usb_read_file(), Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 05/50] tools: Remove unused cur_mon from qemu-tool.c, Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 02/50] savevm: Fix -loadvm to report errors to stderr, not the monitor, Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 03/50] pc: Fix error reporting for -boot once, Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 10/50] error: Simplify error sink setup, Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 04/50] pc: Factor common code out of pc_boot_set() and cmos_init(), Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 12/50] error: New error_printf() and error_vprintf(), Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 09/50] error: Move qemu_error() & friends from monitor.c to own file, Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 11/50] error: Move qemu_error & friends into their own header, Markus Armbruster, 2010/03/04
- [Qemu-devel] [PATCH 35/50] error: New QERR_DEVICE_MULTIPLE_BUSSES, Markus Armbruster, 2010/03/04