* [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError
@ 2010-03-04 15:56 Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 01/50] usb: Remove disabled monitor_printf() in usb_read_file() Markus Armbruster
` (50 more replies)
0 siblings, 51 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
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
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 01/50] usb: Remove disabled monitor_printf() in usb_read_file()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 02/50] savevm: Fix -loadvm to report errors to stderr, not the monitor Markus Armbruster
` (49 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
The monitor_printf() reports failure. Printing is wrong, because the
caller tries various arguments, and expects the function to fail for
some or all.
Disabled since commit 26a9e82a. Remove it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
usb-linux.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index a9c15c6..d0d7cff 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1188,9 +1188,6 @@ static int usb_host_scan_dev(void *opaque, USBScanFunc *func)
*/
static int usb_host_read_file(char *line, size_t line_size, const char *device_file, const char *device_name)
{
-#if 0
- Monitor *mon = cur_mon;
-#endif
FILE *f;
int ret = 0;
char filename[PATH_MAX];
@@ -1201,11 +1198,6 @@ static int usb_host_read_file(char *line, size_t line_size, const char *device_f
if (f) {
ret = fgets(line, line_size, f) != NULL;
fclose(f);
-#if 0
- } else {
- if (mon)
- monitor_printf(mon, "husb: could not open %s\n", filename);
-#endif
}
return ret;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 02/50] savevm: Fix -loadvm to report errors to stderr, not the monitor
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 01/50] usb: Remove disabled monitor_printf() in usb_read_file() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 03/50] pc: Fix error reporting for -boot once Markus Armbruster
` (48 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
A monitor may not even exist.
Change load_vmstate() to use qemu_error() instead of monitor_printf().
Parameter mon is now unused, remove it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor.c | 2 +-
savevm.c | 24 +++++++++++-------------
sysemu.h | 2 +-
vl.c | 2 +-
4 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/monitor.c b/monitor.c
index 19470d1..b16c8c3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2530,7 +2530,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
vm_stop(0);
- if (load_vmstate(mon, name) >= 0 && saved_vm_running)
+ if (load_vmstate(name) >= 0 && saved_vm_running)
vm_start();
}
diff --git a/savevm.c b/savevm.c
index 4b58663..1f6c25e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1733,7 +1733,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
vm_start();
}
-int load_vmstate(Monitor *mon, const char *name)
+int load_vmstate(const char *name)
{
DriveInfo *dinfo;
BlockDriverState *bs, *bs1;
@@ -1743,7 +1743,7 @@ int load_vmstate(Monitor *mon, const char *name)
bs = get_bs_snapshots();
if (!bs) {
- monitor_printf(mon, "No block device supports snapshots\n");
+ qemu_error("No block device supports snapshots\n");
return -EINVAL;
}
@@ -1756,21 +1756,19 @@ int load_vmstate(Monitor *mon, const char *name)
ret = bdrv_snapshot_goto(bs1, name);
if (ret < 0) {
if (bs != bs1)
- monitor_printf(mon, "Warning: ");
+ qemu_error("Warning: ");
switch(ret) {
case -ENOTSUP:
- monitor_printf(mon,
- "Snapshots not supported on device '%s'\n",
- bdrv_get_device_name(bs1));
+ qemu_error("Snapshots not supported on device '%s'\n",
+ bdrv_get_device_name(bs1));
break;
case -ENOENT:
- monitor_printf(mon, "Could not find snapshot '%s' on "
- "device '%s'\n",
- name, bdrv_get_device_name(bs1));
+ qemu_error("Could not find snapshot '%s' on device '%s'\n",
+ name, bdrv_get_device_name(bs1));
break;
default:
- monitor_printf(mon, "Error %d while activating snapshot on"
- " '%s'\n", ret, bdrv_get_device_name(bs1));
+ qemu_error("Error %d while activating snapshot on '%s'\n",
+ ret, bdrv_get_device_name(bs1));
break;
}
/* fatal on snapshot block device */
@@ -1788,13 +1786,13 @@ int load_vmstate(Monitor *mon, const char *name)
/* restore the VM state */
f = qemu_fopen_bdrv(bs, 0);
if (!f) {
- monitor_printf(mon, "Could not open VM state file\n");
+ qemu_error("Could not open VM state file\n");
return -EINVAL;
}
ret = qemu_loadvm_state(f);
qemu_fclose(f);
if (ret < 0) {
- monitor_printf(mon, "Error %d while loading VM state\n", ret);
+ qemu_error("Error %d while loading VM state\n", ret);
return ret;
}
return 0;
diff --git a/sysemu.h b/sysemu.h
index 8ba618e..4d8b7a2 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -54,7 +54,7 @@ extern qemu_irq qemu_system_powerdown;
void qemu_system_reset(void);
void do_savevm(Monitor *mon, const QDict *qdict);
-int load_vmstate(Monitor *mon, const char *name);
+int load_vmstate(const char *name);
void do_delvm(Monitor *mon, const QDict *qdict);
void do_info_snapshots(Monitor *mon);
diff --git a/vl.c b/vl.c
index db7a178..8a588c7 100644
--- a/vl.c
+++ b/vl.c
@@ -6023,7 +6023,7 @@ int main(int argc, char **argv, char **envp)
qemu_system_reset();
if (loadvm) {
- if (load_vmstate(cur_mon, loadvm) < 0) {
+ if (load_vmstate(loadvm) < 0) {
autostart = 0;
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 03/50] pc: Fix error reporting for -boot once
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 01/50] usb: Remove disabled monitor_printf() in usb_read_file() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 02/50] savevm: Fix -loadvm to report errors to stderr, not the monitor Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 04/50] pc: Factor common code out of pc_boot_set() and cmos_init() Markus Armbruster
` (47 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Commit 0ecdffbb created pc_boot_set() for use from monitor command
"boot_set", via qemu_boot_set(). pc_boot_set() reports errors to
cur_mon, which works fine for monitor code.
Commit e0f084bf reused the function int reset handler
restore_boot_devices(). Use of cur_mon is problematic in that
context. For instance, the "Too many boot devices for PC" error for
"-boot order=abcdefgh,once=c" goes to the monitor instead of stderr.
The monitor may not even exist.
Fix by switching to qemu_error().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pc.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 4f6a522..d94980b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -234,7 +234,6 @@ static int boot_device2nibble(char boot_device)
and used there as well */
static int pc_boot_set(void *opaque, const char *boot_device)
{
- Monitor *mon = cur_mon;
#define PC_MAX_BOOT_DEVICES 3
RTCState *s = (RTCState *)opaque;
int nbds, bds[3] = { 0, };
@@ -242,14 +241,14 @@ static int pc_boot_set(void *opaque, const char *boot_device)
nbds = strlen(boot_device);
if (nbds > PC_MAX_BOOT_DEVICES) {
- monitor_printf(mon, "Too many boot devices for PC\n");
+ qemu_error("Too many boot devices for PC\n");
return(1);
}
for (i = 0; i < nbds; i++) {
bds[i] = boot_device2nibble(boot_device[i]);
if (bds[i] == 0) {
- monitor_printf(mon, "Invalid boot device for PC: '%c'\n",
- boot_device[i]);
+ qemu_error("Invalid boot device for PC: '%c'\n",
+ boot_device[i]);
return(1);
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 04/50] pc: Factor common code out of pc_boot_set() and cmos_init()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (2 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 03/50] pc: Fix error reporting for -boot once Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 05/50] tools: Remove unused cur_mon from qemu-tool.c Markus Armbruster
` (46 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Code duplicated in commit 0ecdffbb. The two versions are similar, but
not identical:
* cmos_init() reports errors to stderr, pc_boot_set() via
qemu_error(). The latter is fine for both, so pick that for the
common code.
* cmos_init() obeys fd_bootchk, pc_boot_set() ignores it. Make it a
parameter of the common code.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pc.c | 28 ++++++++--------------------
1 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index d94980b..1595283 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -230,12 +230,9 @@ static int boot_device2nibble(char boot_device)
return 0;
}
-/* copy/pasted from cmos_init, should be made a general function
- and used there as well */
-static int pc_boot_set(void *opaque, const char *boot_device)
+static int set_boot_dev(RTCState *s, const char *boot_device, int fd_bootchk)
{
#define PC_MAX_BOOT_DEVICES 3
- RTCState *s = (RTCState *)opaque;
int nbds, bds[3] = { 0, };
int i;
@@ -253,16 +250,20 @@ static int pc_boot_set(void *opaque, const char *boot_device)
}
}
rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
- rtc_set_memory(s, 0x38, (bds[2] << 4));
+ rtc_set_memory(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 0x1));
return(0);
}
+static int pc_boot_set(void *opaque, const char *boot_device)
+{
+ return set_boot_dev(opaque, boot_device, 0);
+}
+
/* hd_table must contain 4 block drivers */
static void cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
const char *boot_device, DriveInfo **hd_table)
{
RTCState *s = rtc_state;
- int nbds, bds[3] = { 0, };
int val;
int fd0, fd1, nb;
int i;
@@ -301,22 +302,9 @@ static void cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
rtc_set_memory(s, 0x5f, smp_cpus - 1);
/* set boot devices, and disable floppy signature check if requested */
-#define PC_MAX_BOOT_DEVICES 3
- nbds = strlen(boot_device);
- if (nbds > PC_MAX_BOOT_DEVICES) {
- fprintf(stderr, "Too many boot devices for PC\n");
+ if (set_boot_dev(s, boot_device, fd_bootchk)) {
exit(1);
}
- for (i = 0; i < nbds; i++) {
- bds[i] = boot_device2nibble(boot_device[i]);
- if (bds[i] == 0) {
- fprintf(stderr, "Invalid boot device for PC: '%c'\n",
- boot_device[i]);
- exit(1);
- }
- }
- rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
- rtc_set_memory(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 0x1));
/* floppy type */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 05/50] tools: Remove unused cur_mon from qemu-tool.c
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (3 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 04/50] pc: Factor common code out of pc_boot_set() and cmos_init() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 06/50] monitor: Separate "default monitor" and "current monitor" cleanly Markus Armbruster
` (45 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-tool.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/qemu-tool.c b/qemu-tool.c
index 18b48af..bbce99f 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -33,8 +33,6 @@ void qemu_service_io(void)
{
}
-Monitor *cur_mon;
-
void monitor_printf(Monitor *mon, const char *fmt, ...)
{
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 06/50] monitor: Separate "default monitor" and "current monitor" cleanly
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (4 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 05/50] tools: Remove unused cur_mon from qemu-tool.c Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 07/50] block: Simplify usb_msd_initfn() test for "can read bdrv key" Markus Armbruster
` (44 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Commits 376253ec..731b0364 introduced global variable cur_mon, which
points to the "default monitor" (if any), except during execution of
monitor_read() or monitor_control_read() it points to the monitor from
which we're reading instead (the "current monitor"). Monitor command
handlers run within monitor_read() or monitor_control_read().
Default monitor and current monitor are really separate things, and
squashing them together is confusing and error-prone.
For instance, usb_host_scan() can run both in "info usbhost" and
periodically via usb_host_auto_check(). It prints to cur_mon, which
is what we want in the former case: the monitor executing "info
usbhost". But since that's the default monitor in the latter case, it
periodically spams the default monitor there.
A few places use cur_mon to log stuff to the default monitor. If we
ever log something while cur_mon points to current monitor instead of
default monitor, the log temporarily "jumps" to another monitor.
Whether that can or cannot happen isn't always obvious.
Maybe logging to the default monitor (which may not even exist) is a
bad idea, and we should log to stderr or a logfile instead. But
that's outside the scope of this commit.
Change cur_mon to point to the current monitor. Create new
default_mon to point to the default monitor. Update users of cur_mon
accordingly.
This fixes the periodical spamming of the default monitor by
usb_host_scan(). It also stops "log jumping", should that problem
exist.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
audio/audio.c | 4 ++--
monitor.c | 7 ++++---
monitor.h | 1 +
slirp/misc.c | 2 +-
vnc.c | 5 ++---
5 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 2a20e5b..dbf0b96 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -330,10 +330,10 @@ void AUD_vlog (const char *cap, const char *fmt, va_list ap)
{
if (conf.log_to_monitor) {
if (cap) {
- monitor_printf(cur_mon, "%s: ", cap);
+ monitor_printf(default_mon, "%s: ", cap);
}
- monitor_vprintf(cur_mon, fmt, ap);
+ monitor_vprintf(default_mon, fmt, ap);
}
else {
if (cap) {
diff --git a/monitor.c b/monitor.c
index b16c8c3..8e2e5c3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -177,7 +177,8 @@ static QLIST_HEAD(mon_list, Monitor) mon_list;
static const mon_cmd_t mon_cmds[];
static const mon_cmd_t info_cmds[];
-Monitor *cur_mon = NULL;
+Monitor *cur_mon;
+Monitor *default_mon;
static void monitor_command_cb(Monitor *mon, const char *cmdline,
void *opaque);
@@ -4621,8 +4622,8 @@ void monitor_init(CharDriverState *chr, int flags)
}
QLIST_INSERT_HEAD(&mon_list, mon, entry);
- if (!cur_mon || (flags & MONITOR_IS_DEFAULT))
- cur_mon = mon;
+ if (!default_mon || (flags & MONITOR_IS_DEFAULT))
+ default_mon = mon;
}
static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)
diff --git a/monitor.h b/monitor.h
index fc09505..c481c9d 100644
--- a/monitor.h
+++ b/monitor.h
@@ -7,6 +7,7 @@
#include "block.h"
extern Monitor *cur_mon;
+extern Monitor *default_mon;
/* flags for monitor_init */
#define MONITOR_IS_DEFAULT 0x01
diff --git a/slirp/misc.c b/slirp/misc.c
index dcb1dc1..1aeb401 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -260,7 +260,7 @@ void lprint(const char *format, ...)
va_list args;
va_start(args, format);
- monitor_vprintf(cur_mon, format, args);
+ monitor_vprintf(default_mon, format, args);
va_end(args);
}
diff --git a/vnc.c b/vnc.c
index db34b0e..712043f 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1046,11 +1046,10 @@ static void audio_capture(void *opaque, void *buf, int size)
static void audio_add(VncState *vs)
{
- Monitor *mon = cur_mon;
struct audio_capture_ops ops;
if (vs->audio_cap) {
- monitor_printf(mon, "audio already running\n");
+ monitor_printf(default_mon, "audio already running\n");
return;
}
@@ -1060,7 +1059,7 @@ static void audio_add(VncState *vs)
vs->audio_cap = AUD_add_capture(&vs->as, &ops, vs);
if (!vs->audio_cap) {
- monitor_printf(mon, "Failed to add audio capture\n");
+ monitor_printf(default_mon, "Failed to add audio capture\n");
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 07/50] block: Simplify usb_msd_initfn() test for "can read bdrv key"
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (5 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 06/50] monitor: Separate "default monitor" and "current monitor" cleanly Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 08/50] monitor: Factor monitor_set_error() out of qemu_error_internal() Markus Armbruster
` (43 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
The old test assumes that "hotplugged" implies "we have a current
monitor for reading the key". This is in fact true, but it's not
obviously true.
Aside: if it were false, we could pass a null pointer to
monitor_read_bdrv_key_start(), which would then crash.
The previous commit permits us to check for "we have a current
monitor" directly, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/usb-msd.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 1a11bc5..0afb031 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -535,7 +535,7 @@ static int usb_msd_initfn(USBDevice *dev)
usb_msd_handle_reset(dev);
if (bdrv_key_required(s->conf.dinfo->bdrv)) {
- if (s->dev.qdev.hotplugged) {
+ if (cur_mon) {
monitor_read_bdrv_key_start(cur_mon, s->conf.dinfo->bdrv,
usb_msd_password_cb, s);
s->dev.auto_attach = 0;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 08/50] monitor: Factor monitor_set_error() out of qemu_error_internal()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (6 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 07/50] block: Simplify usb_msd_initfn() test for "can read bdrv key" Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 09/50] error: Move qemu_error() & friends from monitor.c to own file Markus Armbruster
` (42 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
This separates the monitor part from the QError part.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor.c | 21 +++++++++++++--------
monitor.h | 3 +++
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/monitor.c b/monitor.c
index 8e2e5c3..da52cb4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3855,6 +3855,18 @@ fail:
return NULL;
}
+void monitor_set_error(Monitor *mon, QError *qerror)
+{
+ /* report only the first error */
+ if (!mon->error) {
+ mon->error = qerror;
+ } else {
+ MON_DEBUG("Additional error report at %s:%d\n",
+ qerror->file, qerror->linenr);
+ QDECREF(qerror);
+ }
+}
+
static void monitor_print_error(Monitor *mon)
{
qerror_print(mon->error);
@@ -4756,14 +4768,7 @@ void qemu_error_internal(const char *file, int linenr, const char *func,
QDECREF(qerror);
break;
case ERR_SINK_MONITOR:
- /* report only the first error */
- if (!qemu_error_sink->mon->error) {
- qemu_error_sink->mon->error = qerror;
- } else {
- MON_DEBUG("Additional error report at %s:%d\n", qerror->file,
- qerror->linenr);
- QDECREF(qerror);
- }
+ monitor_set_error(qemu_error_sink->mon, qerror);
break;
}
}
diff --git a/monitor.h b/monitor.h
index c481c9d..6d7969c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -3,6 +3,7 @@
#include "qemu-common.h"
#include "qemu-char.h"
+#include "qerror.h"
#include "qdict.h"
#include "block.h"
@@ -48,4 +49,6 @@ void monitor_flush(Monitor *mon);
typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
+void monitor_set_error(Monitor *mon, QError *qerror);
+
#endif /* !MONITOR_H */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 09/50] error: Move qemu_error() & friends from monitor.c to own file
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (7 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 08/50] monitor: Factor monitor_set_error() out of qemu_error_internal() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 10/50] error: Simplify error sink setup Markus Armbruster
` (41 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
They're about reporting errors, not about the monitor.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
Makefile.target | 1 +
monitor.c | 89 -----------------------------------------------------
qemu-error.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 93 insertions(+), 89 deletions(-)
create mode 100644 qemu-error.c
diff --git a/Makefile.target b/Makefile.target
index 320f807..2d85a8e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -169,6 +169,7 @@ endif #CONFIG_BSD_USER
ifdef CONFIG_SOFTMMU
obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
+obj-y += qemu-error.o
# virtio has to be here due to weird dependency between PCI and virtio-net.
# need to fix this properly
obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o
diff --git a/monitor.c b/monitor.c
index da52cb4..35b4677 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4683,92 +4683,3 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
return err;
}
-
-typedef struct QemuErrorSink QemuErrorSink;
-struct QemuErrorSink {
- enum {
- ERR_SINK_FILE,
- ERR_SINK_MONITOR,
- } dest;
- union {
- FILE *fp;
- Monitor *mon;
- };
- QemuErrorSink *previous;
-};
-
-static QemuErrorSink *qemu_error_sink;
-
-void qemu_errors_to_file(FILE *fp)
-{
- QemuErrorSink *sink;
-
- sink = qemu_mallocz(sizeof(*sink));
- sink->dest = ERR_SINK_FILE;
- sink->fp = fp;
- sink->previous = qemu_error_sink;
- qemu_error_sink = sink;
-}
-
-void qemu_errors_to_mon(Monitor *mon)
-{
- QemuErrorSink *sink;
-
- sink = qemu_mallocz(sizeof(*sink));
- sink->dest = ERR_SINK_MONITOR;
- sink->mon = mon;
- sink->previous = qemu_error_sink;
- qemu_error_sink = sink;
-}
-
-void qemu_errors_to_previous(void)
-{
- QemuErrorSink *sink;
-
- assert(qemu_error_sink != NULL);
- sink = qemu_error_sink;
- qemu_error_sink = sink->previous;
- qemu_free(sink);
-}
-
-void qemu_error(const char *fmt, ...)
-{
- va_list args;
-
- assert(qemu_error_sink != NULL);
- switch (qemu_error_sink->dest) {
- case ERR_SINK_FILE:
- va_start(args, fmt);
- vfprintf(qemu_error_sink->fp, fmt, args);
- va_end(args);
- break;
- case ERR_SINK_MONITOR:
- va_start(args, fmt);
- monitor_vprintf(qemu_error_sink->mon, fmt, args);
- va_end(args);
- break;
- }
-}
-
-void qemu_error_internal(const char *file, int linenr, const char *func,
- const char *fmt, ...)
-{
- va_list va;
- QError *qerror;
-
- assert(qemu_error_sink != NULL);
-
- va_start(va, fmt);
- qerror = qerror_from_info(file, linenr, func, fmt, &va);
- va_end(va);
-
- switch (qemu_error_sink->dest) {
- case ERR_SINK_FILE:
- qerror_print(qerror);
- QDECREF(qerror);
- break;
- case ERR_SINK_MONITOR:
- monitor_set_error(qemu_error_sink->mon, qerror);
- break;
- }
-}
diff --git a/qemu-error.c b/qemu-error.c
new file mode 100644
index 0000000..df381f6
--- /dev/null
+++ b/qemu-error.c
@@ -0,0 +1,92 @@
+#include <stdio.h>
+#include "monitor.h"
+#include "sysemu.h"
+
+typedef struct QemuErrorSink QemuErrorSink;
+struct QemuErrorSink {
+ enum {
+ ERR_SINK_FILE,
+ ERR_SINK_MONITOR,
+ } dest;
+ union {
+ FILE *fp;
+ Monitor *mon;
+ };
+ QemuErrorSink *previous;
+};
+
+static QemuErrorSink *qemu_error_sink;
+
+void qemu_errors_to_file(FILE *fp)
+{
+ QemuErrorSink *sink;
+
+ sink = qemu_mallocz(sizeof(*sink));
+ sink->dest = ERR_SINK_FILE;
+ sink->fp = fp;
+ sink->previous = qemu_error_sink;
+ qemu_error_sink = sink;
+}
+
+void qemu_errors_to_mon(Monitor *mon)
+{
+ QemuErrorSink *sink;
+
+ sink = qemu_mallocz(sizeof(*sink));
+ sink->dest = ERR_SINK_MONITOR;
+ sink->mon = mon;
+ sink->previous = qemu_error_sink;
+ qemu_error_sink = sink;
+}
+
+void qemu_errors_to_previous(void)
+{
+ QemuErrorSink *sink;
+
+ assert(qemu_error_sink != NULL);
+ sink = qemu_error_sink;
+ qemu_error_sink = sink->previous;
+ qemu_free(sink);
+}
+
+void qemu_error(const char *fmt, ...)
+{
+ va_list args;
+
+ assert(qemu_error_sink != NULL);
+ switch (qemu_error_sink->dest) {
+ case ERR_SINK_FILE:
+ va_start(args, fmt);
+ vfprintf(qemu_error_sink->fp, fmt, args);
+ va_end(args);
+ break;
+ case ERR_SINK_MONITOR:
+ va_start(args, fmt);
+ monitor_vprintf(qemu_error_sink->mon, fmt, args);
+ va_end(args);
+ break;
+ }
+}
+
+void qemu_error_internal(const char *file, int linenr, const char *func,
+ const char *fmt, ...)
+{
+ va_list va;
+ QError *qerror;
+
+ assert(qemu_error_sink != NULL);
+
+ va_start(va, fmt);
+ qerror = qerror_from_info(file, linenr, func, fmt, &va);
+ va_end(va);
+
+ switch (qemu_error_sink->dest) {
+ case ERR_SINK_FILE:
+ qerror_print(qerror);
+ QDECREF(qerror);
+ break;
+ case ERR_SINK_MONITOR:
+ monitor_set_error(qemu_error_sink->mon, qerror);
+ break;
+ }
+}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 10/50] error: Simplify error sink setup
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (8 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 09/50] error: Move qemu_error() & friends from monitor.c to own file Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 11/50] error: Move qemu_error & friends into their own header Markus Armbruster
` (40 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
qemu_error_sink can either point to a monitor or a file. In practice,
it always points to the current monitor if we have one, else to
stderr. Simply route errors to the current monitor or else to stderr,
and remove qemu_error_sink along with the functions to control it.
Actually, the old code switches the sink slightly later, in
handle_user_command() and handle_qmp_command(), than it gets switched
now, implicitly, by setting the current monitor in monitor_read() and
monitor_control_read(). Likewise, it switches back slightly earlier
(same places). Doesn't make a difference, because there are no calls
of qemu_error() in between.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor.c | 6 ----
qemu-error.c | 76 +++++++---------------------------------------------------
sysemu.h | 3 --
vl.c | 1 -
4 files changed, 9 insertions(+), 77 deletions(-)
diff --git a/monitor.c b/monitor.c
index 35b4677..8dbcb62 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3968,8 +3968,6 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
if (!cmd)
goto out;
- qemu_errors_to_mon(mon);
-
if (monitor_handler_is_async(cmd)) {
user_async_cmd_handler(mon, cmd, qdict);
} else if (monitor_handler_ported(cmd)) {
@@ -3981,8 +3979,6 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
if (monitor_has_error(mon))
monitor_print_error(mon);
- qemu_errors_to_previous();
-
out:
QDECREF(qdict);
}
@@ -4384,7 +4380,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
const char *cmd_name, *info_item;
args = NULL;
- qemu_errors_to_mon(mon);
obj = json_parser_parse(tokens, NULL);
if (!obj) {
@@ -4465,7 +4460,6 @@ err_out:
monitor_protocol_emitter(mon, NULL);
out:
QDECREF(args);
- qemu_errors_to_previous();
}
/**
diff --git a/qemu-error.c b/qemu-error.c
index df381f6..63bcdcf 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -2,70 +2,17 @@
#include "monitor.h"
#include "sysemu.h"
-typedef struct QemuErrorSink QemuErrorSink;
-struct QemuErrorSink {
- enum {
- ERR_SINK_FILE,
- ERR_SINK_MONITOR,
- } dest;
- union {
- FILE *fp;
- Monitor *mon;
- };
- QemuErrorSink *previous;
-};
-
-static QemuErrorSink *qemu_error_sink;
-
-void qemu_errors_to_file(FILE *fp)
-{
- QemuErrorSink *sink;
-
- sink = qemu_mallocz(sizeof(*sink));
- sink->dest = ERR_SINK_FILE;
- sink->fp = fp;
- sink->previous = qemu_error_sink;
- qemu_error_sink = sink;
-}
-
-void qemu_errors_to_mon(Monitor *mon)
-{
- QemuErrorSink *sink;
-
- sink = qemu_mallocz(sizeof(*sink));
- sink->dest = ERR_SINK_MONITOR;
- sink->mon = mon;
- sink->previous = qemu_error_sink;
- qemu_error_sink = sink;
-}
-
-void qemu_errors_to_previous(void)
-{
- QemuErrorSink *sink;
-
- assert(qemu_error_sink != NULL);
- sink = qemu_error_sink;
- qemu_error_sink = sink->previous;
- qemu_free(sink);
-}
-
void qemu_error(const char *fmt, ...)
{
va_list args;
- assert(qemu_error_sink != NULL);
- switch (qemu_error_sink->dest) {
- case ERR_SINK_FILE:
- va_start(args, fmt);
- vfprintf(qemu_error_sink->fp, fmt, args);
- va_end(args);
- break;
- case ERR_SINK_MONITOR:
- va_start(args, fmt);
- monitor_vprintf(qemu_error_sink->mon, fmt, args);
- va_end(args);
- break;
+ va_start(args, fmt);
+ if (cur_mon) {
+ monitor_vprintf(cur_mon, fmt, args);
+ } else {
+ vfprintf(stderr, fmt, args);
}
+ va_end(args);
}
void qemu_error_internal(const char *file, int linenr, const char *func,
@@ -74,19 +21,14 @@ void qemu_error_internal(const char *file, int linenr, const char *func,
va_list va;
QError *qerror;
- assert(qemu_error_sink != NULL);
-
va_start(va, fmt);
qerror = qerror_from_info(file, linenr, func, fmt, &va);
va_end(va);
- switch (qemu_error_sink->dest) {
- case ERR_SINK_FILE:
+ if (cur_mon) {
+ monitor_set_error(cur_mon, qerror);
+ } else {
qerror_print(qerror);
QDECREF(qerror);
- break;
- case ERR_SINK_MONITOR:
- monitor_set_error(qemu_error_sink->mon, qerror);
- break;
}
}
diff --git a/sysemu.h b/sysemu.h
index 4d8b7a2..cd84ee8 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,9 +69,6 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f);
void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f);
int qemu_loadvm_state(QEMUFile *f);
-void qemu_errors_to_file(FILE *fp);
-void qemu_errors_to_mon(Monitor *mon);
-void qemu_errors_to_previous(void);
void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void qemu_error_internal(const char *file, int linenr, const char *func,
const char *fmt, ...)
diff --git a/vl.c b/vl.c
index 8a588c7..c4c8c1f 100644
--- a/vl.c
+++ b/vl.c
@@ -4824,7 +4824,6 @@ int main(int argc, char **argv, char **envp)
init_clocks();
- qemu_errors_to_file(stderr);
qemu_cache_utils_init(envp);
QLIST_INIT (&vm_change_state_head);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 11/50] error: Move qemu_error & friends into their own header
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (9 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 10/50] error: Simplify error sink setup Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 12/50] error: New error_printf() and error_vprintf() Markus Armbruster
` (39 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/scsi-bus.c | 2 +-
hw/scsi-disk.c | 3 +--
hw/scsi-generic.c | 1 +
hw/usb-serial.c | 1 +
hw/virtio-net.c | 1 +
hw/virtio-pci.c | 2 +-
monitor.h | 1 +
net/dump.c | 1 +
net/socket.c | 2 +-
net/tap-linux.c | 1 +
net/tap.c | 1 +
qemu-config.c | 1 +
qemu-error.h | 12 ++++++++++++
qemu-tool.c | 2 +-
qerror.c | 2 +-
sysemu.h | 8 --------
16 files changed, 26 insertions(+), 15 deletions(-)
create mode 100644 qemu-error.h
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index a2f9cc1..c41ce9b 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1,5 +1,5 @@
#include "hw.h"
-#include "sysemu.h"
+#include "qemu-error.h"
#include "scsi.h"
#include "scsi-defs.h"
#include "block.h"
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b2f61fe..b1424de 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -19,8 +19,6 @@
* the host adapter emulator.
*/
-#include <qemu-common.h>
-#include <sysemu.h>
//#define DEBUG_SCSI
#ifdef DEBUG_SCSI
@@ -34,6 +32,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
#include "qemu-common.h"
+#include "qemu-error.h"
#include "block.h"
#include "scsi.h"
#include "scsi-defs.h"
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index de778ef..cfd9903 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -12,6 +12,7 @@
*/
#include "qemu-common.h"
+#include "qemu-error.h"
#include "block.h"
#include "scsi.h"
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index 1410b11..6db9446 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -9,6 +9,7 @@
*/
#include "qemu-common.h"
+#include "qemu-error.h"
#include "usb.h"
#include "qemu-char.h"
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5c0093e..8359be6 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -15,6 +15,7 @@
#include "net.h"
#include "net/checksum.h"
#include "net/tap.h"
+#include "qemu-error.h"
#include "qemu-timer.h"
#include "virtio-net.h"
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index bcd40f7..7781328 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -19,7 +19,7 @@
#include "virtio-blk.h"
#include "virtio-net.h"
#include "pci.h"
-#include "sysemu.h"
+#include "qemu-error.h"
#include "msix.h"
#include "net.h"
#include "block_int.h"
diff --git a/monitor.h b/monitor.h
index 6d7969c..1222ff0 100644
--- a/monitor.h
+++ b/monitor.h
@@ -3,6 +3,7 @@
#include "qemu-common.h"
#include "qemu-char.h"
+#include "qemu-error.h"
#include "qerror.h"
#include "qdict.h"
#include "block.h"
diff --git a/net/dump.c b/net/dump.c
index d50b4ee..e702830 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -25,6 +25,7 @@
#include "dump.h"
#include "qemu-common.h"
#include "sysemu.h"
+#include "qemu-error.h"
#include "qemu-log.h"
typedef struct DumpState {
diff --git a/net/socket.c b/net/socket.c
index 442a9c7..474d573 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -28,9 +28,9 @@
#include "net.h"
#include "qemu-char.h"
#include "qemu-common.h"
+#include "qemu-error.h"
#include "qemu-option.h"
#include "qemu_socket.h"
-#include "sysemu.h"
typedef struct NetSocketState {
VLANClientState nc;
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 6af9e82..c5748e6 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -31,6 +31,7 @@
#include "sysemu.h"
#include "qemu-common.h"
+#include "qemu-error.h"
int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required)
{
diff --git a/net/tap.c b/net/tap.c
index 7a7320c..9ba9b4a 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -38,6 +38,7 @@
#include "sysemu.h"
#include "qemu-char.h"
#include "qemu-common.h"
+#include "qemu-error.h"
#include "net/tap-linux.h"
diff --git a/qemu-config.c b/qemu-config.c
index 246fae6..2c9a7a5 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -1,4 +1,5 @@
#include "qemu-common.h"
+#include "qemu-error.h"
#include "qemu-option.h"
#include "qemu-config.h"
#include "sysemu.h"
diff --git a/qemu-error.h b/qemu-error.h
new file mode 100644
index 0000000..fa16113
--- /dev/null
+++ b/qemu-error.h
@@ -0,0 +1,12 @@
+#ifndef QEMU_ERROR_H
+#define QEMU_ERROR_H
+
+void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
+void qemu_error_internal(const char *file, int linenr, const char *func,
+ const char *fmt, ...)
+ __attribute__ ((format(printf, 4, 5)));
+
+#define qemu_error_new(fmt, ...) \
+ qemu_error_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
+
+#endif
diff --git a/qemu-tool.c b/qemu-tool.c
index bbce99f..26f46eb 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -13,9 +13,9 @@
#include "qemu-common.h"
#include "monitor.h"
-#include "sysemu.h"
#include "qemu-timer.h"
#include "qemu-log.h"
+#include "qemu-error.h"
#include <sys/time.h>
diff --git a/qerror.c b/qerror.c
index 2f657f4..a418bde 100644
--- a/qerror.c
+++ b/qerror.c
@@ -12,8 +12,8 @@
#include "qjson.h"
#include "qerror.h"
#include "qstring.h"
-#include "sysemu.h"
#include "qemu-common.h"
+#include "qemu-error.h"
static void qerror_destroy_obj(QObject *obj);
diff --git a/sysemu.h b/sysemu.h
index cd84ee8..6a2dd93 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,14 +69,6 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f);
void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f);
int qemu_loadvm_state(QEMUFile *f);
-void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
-void qemu_error_internal(const char *file, int linenr, const char *func,
- const char *fmt, ...)
- __attribute__ ((format(printf, 4, 5)));
-
-#define qemu_error_new(fmt, ...) \
- qemu_error_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
-
#ifdef _WIN32
/* Polling handling */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 12/50] error: New error_printf() and error_vprintf()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (10 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 11/50] error: Move qemu_error & friends into their own header Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 13/50] error: Don't abuse qemu_error() for non-error in qdev_device_help() Markus Armbruster
` (38 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-error.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
qemu-error.h | 14 ++++++++++++++
2 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index 63bcdcf..d20fd0f 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -1,18 +1,53 @@
+/*
+ * Error reporting
+ *
+ * Copyright (C) 2010 Red Hat Inc.
+ *
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
#include <stdio.h>
#include "monitor.h"
#include "sysemu.h"
-void qemu_error(const char *fmt, ...)
+/*
+ * Print to current monitor if we have one, else to stderr.
+ * TODO should return int, so callers can calculate width, but that
+ * requires surgery to monitor_vprintf(). Left for another day.
+ */
+void error_vprintf(const char *fmt, va_list ap)
{
- va_list args;
-
- va_start(args, fmt);
if (cur_mon) {
- monitor_vprintf(cur_mon, fmt, args);
+ monitor_vprintf(cur_mon, fmt, ap);
} else {
- vfprintf(stderr, fmt, args);
+ vfprintf(stderr, fmt, ap);
}
- va_end(args);
+}
+
+/*
+ * Print to current monitor if we have one, else to stderr.
+ * TODO just like error_vprintf()
+ */
+void error_printf(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ error_vprintf(fmt, ap);
+ va_end(ap);
+}
+
+void qemu_error(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ error_vprintf(fmt, ap);
+ va_end(ap);
}
void qemu_error_internal(const char *file, int linenr, const char *func,
diff --git a/qemu-error.h b/qemu-error.h
index fa16113..d90f1da 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -1,6 +1,20 @@
+/*
+ * Error reporting
+ *
+ * Copyright (C) 2010 Red Hat Inc.
+ *
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
#ifndef QEMU_ERROR_H
#define QEMU_ERROR_H
+void error_vprintf(const char *fmt, va_list ap);
+void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void qemu_error_internal(const char *file, int linenr, const char *func,
const char *fmt, ...)
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 13/50] error: Don't abuse qemu_error() for non-error in qdev_device_help()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (11 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 12/50] error: New error_printf() and error_vprintf() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 14/50] error: Don't abuse qemu_error() for non-error in qbus_find() Markus Armbruster
` (37 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
qdev_device_help() prints device information with qemu_error(). A
later commit will make qemu_error() print additional stuff that is
only appropriate for proper errors, and then this will break. Use
error_printf() instead.
While there, simplify: instead of printing a buffer filled by
qdev_print_devinfo() in one go, make qdev_print_devinfo() print it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 31 ++++++++++---------------------
1 files changed, 10 insertions(+), 21 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index d0052d4..6842bd6 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -113,27 +113,20 @@ DeviceState *qdev_create(BusState *bus, const char *name)
return dev;
}
-static int qdev_print_devinfo(DeviceInfo *info, char *dest, int len)
+static void qdev_print_devinfo(DeviceInfo *info)
{
- int pos = 0;
- int ret;
-
- ret = snprintf(dest+pos, len-pos, "name \"%s\", bus %s",
- info->name, info->bus_info->name);
- pos += MIN(len-pos,ret);
+ error_printf("name \"%s\", bus %s",
+ info->name, info->bus_info->name);
if (info->alias) {
- ret = snprintf(dest+pos, len-pos, ", alias \"%s\"", info->alias);
- pos += MIN(len-pos,ret);
+ error_printf(", alias \"%s\"", info->alias);
}
if (info->desc) {
- ret = snprintf(dest+pos, len-pos, ", desc \"%s\"", info->desc);
- pos += MIN(len-pos,ret);
+ error_printf(", desc \"%s\"", info->desc);
}
if (info->no_user) {
- ret = snprintf(dest+pos, len-pos, ", no-user");
- pos += MIN(len-pos,ret);
+ error_printf(", no-user");
}
- return pos;
+ error_printf("\n");
}
static int set_property(const char *name, const char *value, void *opaque)
@@ -157,14 +150,12 @@ int qdev_device_help(QemuOpts *opts)
{
const char *driver;
DeviceInfo *info;
- char msg[256];
Property *prop;
driver = qemu_opt_get(opts, "driver");
if (driver && !strcmp(driver, "?")) {
for (info = device_info_list; info != NULL; info = info->next) {
- qdev_print_devinfo(info, msg, sizeof(msg));
- qemu_error("%s\n", msg);
+ qdev_print_devinfo(info);
}
return 1;
}
@@ -179,7 +170,7 @@ int qdev_device_help(QemuOpts *opts)
}
for (prop = info->props; prop && prop->name; prop++) {
- qemu_error("%s.%s=%s\n", info->name, prop->name, prop->info->name);
+ error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
}
return 1;
}
@@ -735,11 +726,9 @@ void do_info_qtree(Monitor *mon)
void do_info_qdm(Monitor *mon)
{
DeviceInfo *info;
- char msg[256];
for (info = device_info_list; info != NULL; info = info->next) {
- qdev_print_devinfo(info, msg, sizeof(msg));
- monitor_printf(mon, "%s\n", msg);
+ qdev_print_devinfo(info);
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 14/50] error: Don't abuse qemu_error() for non-error in qbus_find()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (12 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 13/50] error: Don't abuse qemu_error() for non-error in qdev_device_help() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 15/50] error: Don't abuse qemu_error() for non-error in scsi_hot_add() Markus Armbruster
` (36 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
qbus_find() adds an informational line to error messages, and prints
both lines with one qemu_error(). Use error_printf() for the
informational line instead.
While there, simplify: instead of printing buffers filled by
qbus_list_bus() and qbus_list_dev() in one go, make them print it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 37 +++++++++++++++++--------------------
1 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 6842bd6..253b992 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -456,35 +456,33 @@ static DeviceState *qdev_find_recursive(BusState *bus, const char *id)
return NULL;
}
-static void qbus_list_bus(DeviceState *dev, char *dest, int len)
+static void qbus_list_bus(DeviceState *dev)
{
BusState *child;
const char *sep = " ";
- int pos = 0;
- pos += snprintf(dest+pos, len-pos,"child busses at \"%s\":",
- dev->id ? dev->id : dev->info->name);
+ error_printf("child busses at \"%s\":",
+ dev->id ? dev->id : dev->info->name);
QLIST_FOREACH(child, &dev->child_bus, sibling) {
- pos += snprintf(dest+pos, len-pos, "%s\"%s\"", sep, child->name);
+ error_printf("%s\"%s\"", sep, child->name);
sep = ", ";
}
+ error_printf("\n");
}
-static void qbus_list_dev(BusState *bus, char *dest, int len)
+static void qbus_list_dev(BusState *bus)
{
DeviceState *dev;
const char *sep = " ";
- int pos = 0;
- pos += snprintf(dest+pos, len-pos, "devices at \"%s\":",
- bus->name);
+ error_printf("devices at \"%s\":", bus->name);
QLIST_FOREACH(dev, &bus->children, sibling) {
- pos += snprintf(dest+pos, len-pos, "%s\"%s\"",
- sep, dev->info->name);
+ error_printf("%s\"%s\"", sep, dev->info->name);
if (dev->id)
- pos += snprintf(dest+pos, len-pos, "/\"%s\"", dev->id);
+ error_printf("/\"%s\"", dev->id);
sep = ", ";
}
+ error_printf("\n");
}
static BusState *qbus_find_bus(DeviceState *dev, char *elem)
@@ -531,7 +529,7 @@ static BusState *qbus_find(const char *path)
{
DeviceState *dev;
BusState *bus;
- char elem[128], msg[256];
+ char elem[128];
int pos, len;
/* find start element */
@@ -565,8 +563,8 @@ static BusState *qbus_find(const char *path)
pos += len;
dev = qbus_find_dev(bus, elem);
if (!dev) {
- qbus_list_dev(bus, msg, sizeof(msg));
- qemu_error("device \"%s\" not found\n%s\n", elem, msg);
+ qemu_error("device \"%s\" not found\n", elem);
+ qbus_list_dev(bus);
return NULL;
}
if (path[pos] == '\0') {
@@ -579,9 +577,8 @@ static BusState *qbus_find(const char *path)
case 1:
return QLIST_FIRST(&dev->child_bus);
default:
- qbus_list_bus(dev, msg, sizeof(msg));
- qemu_error("device has multiple child busses (%s)\n%s\n",
- path, msg);
+ qemu_error("device has multiple child busses (%s)\n", path);
+ qbus_list_bus(dev);
return NULL;
}
}
@@ -594,8 +591,8 @@ static BusState *qbus_find(const char *path)
pos += len;
bus = qbus_find_bus(dev, elem);
if (!bus) {
- qbus_list_bus(dev, msg, sizeof(msg));
- qemu_error("child bus \"%s\" not found\n%s\n", elem, msg);
+ qemu_error("child bus \"%s\" not found\n", elem);
+ qbus_list_bus(dev);
return NULL;
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 15/50] error: Don't abuse qemu_error() for non-error in scsi_hot_add()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (13 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 14/50] error: Don't abuse qemu_error() for non-error in qbus_find() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 16/50] error: Replace qemu_error() by error_report() Markus Armbruster
` (35 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Commit 30d335d6 converted an informational message from
monitor_printf() to qemu_error(), probably because the latter doesn't
need a mon argument. A later commit will make qemu_error() print
additional stuff that is only appropriate for proper errors, and then
this will break. Clean it up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci-hotplug.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index bd82c6a..41c243c 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -73,7 +73,8 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
}
-static int scsi_hot_add(DeviceState *adapter, DriveInfo *dinfo, int printinfo)
+static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
+ DriveInfo *dinfo, int printinfo)
{
SCSIBus *scsibus;
SCSIDevice *scsidev;
@@ -97,7 +98,8 @@ static int scsi_hot_add(DeviceState *adapter, DriveInfo *dinfo, int printinfo)
dinfo->unit = scsidev->id;
if (printinfo)
- qemu_error("OK bus %d, unit %d\n", scsibus->busnr, scsidev->id);
+ monitor_printf(mon, "OK bus %d, unit %d\n",
+ scsibus->busnr, scsidev->id);
return 0;
}
@@ -131,7 +133,7 @@ void drive_hot_add(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "no pci device with address %s\n", pci_addr);
goto err;
}
- if (scsi_hot_add(&dev->qdev, dinfo, 1) != 0) {
+ if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
goto err;
}
break;
@@ -203,7 +205,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
if (qdev_init(&dev->qdev) < 0)
dev = NULL;
if (dev && dinfo) {
- if (scsi_hot_add(&dev->qdev, dinfo, 0) != 0) {
+ if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
qdev_unplug(&dev->qdev);
dev = NULL;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 16/50] error: Replace qemu_error() by error_report()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (14 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 15/50] error: Don't abuse qemu_error() for non-error in scsi_hot_add() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-13 2:34 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 15:56 ` [Qemu-devel] [PATCH 17/50] error: Rename qemu_error_new() to qerror_report() Markus Armbruster
` (34 subsequent siblings)
50 siblings, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
error_report() terminates the message with a newline. Strip it it
from its arguments.
This fixes a few error messages lacking a newline:
net_handle_fd_param()'s "No file descriptor named %s found", and
tap_open()'s "vnet_hdr=1 requested, but no kernel support for
IFF_VNET_HDR available" (all three versions).
There's one place that passes arguments without newlines
intentionally: load_vmstate(). Fix it up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pc.c | 6 +++---
hw/pci-hotplug.c | 2 +-
hw/pci.c | 14 +++++++-------
hw/qdev.c | 42 +++++++++++++++++++++---------------------
hw/scsi-bus.c | 2 +-
hw/scsi-disk.c | 2 +-
hw/scsi-generic.c | 8 ++++----
hw/usb-bus.c | 4 ++--
hw/usb-msd.c | 2 +-
hw/usb-serial.c | 8 ++++----
hw/virtio-net.c | 4 ++--
hw/virtio-pci.c | 2 +-
hw/virtio-serial-bus.c | 2 +-
net.c | 28 ++++++++++++++--------------
net/dump.c | 4 ++--
net/slirp.c | 28 ++++++++++++++--------------
net/socket.c | 10 +++++-----
net/tap-bsd.c | 7 ++++---
net/tap-linux.c | 8 ++++----
net/tap-solaris.c | 4 ++--
net/tap-win32.c | 2 +-
net/tap.c | 2 +-
qemu-config.c | 10 +++++-----
qemu-error.c | 8 +++++++-
qemu-error.h | 2 +-
qemu-tool.c | 2 +-
qerror.c | 4 ++--
savevm.c | 23 ++++++++++++-----------
vl.c | 4 ++--
29 files changed, 126 insertions(+), 118 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 1595283..70acb74 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -238,14 +238,14 @@ static int set_boot_dev(RTCState *s, const char *boot_device, int fd_bootchk)
nbds = strlen(boot_device);
if (nbds > PC_MAX_BOOT_DEVICES) {
- qemu_error("Too many boot devices for PC\n");
+ error_report("Too many boot devices for PC");
return(1);
}
for (i = 0; i < nbds; i++) {
bds[i] = boot_device2nibble(boot_device[i]);
if (bds[i] == 0) {
- qemu_error("Invalid boot device for PC: '%c'\n",
- boot_device[i]);
+ error_report("Invalid boot device for PC: '%c'",
+ boot_device[i]);
return(1);
}
}
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 41c243c..d608a85 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -81,7 +81,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
scsibus = DO_UPCAST(SCSIBus, qbus, QLIST_FIRST(&adapter->child_bus));
if (!scsibus || strcmp(scsibus->qbus.info->name, "SCSI") != 0) {
- qemu_error("Device is not a SCSI adapter\n");
+ error_report("Device is not a SCSI adapter");
return -1;
}
diff --git a/hw/pci.c b/hw/pci.c
index eb2043e..0dbca17 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -589,12 +589,12 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
if (!bus->devices[devfn])
goto found;
}
- qemu_error("PCI: no devfn available for %s, all in use\n", name);
+ error_report("PCI: no devfn available for %s, all in use", name);
return NULL;
found: ;
} else if (bus->devices[devfn]) {
- qemu_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
- name, bus->devices[devfn]->name);
+ error_report("PCI: devfn %d not available for %s, in use by %s",
+ devfn, name, bus->devices[devfn]->name);
return NULL;
}
pci_dev->bus = bus;
@@ -1476,8 +1476,8 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
bus = pci_get_bus_devfn(&devfn, devaddr);
if (!bus) {
- qemu_error("Invalid PCI device address %s for device %s\n",
- devaddr, pci_nic_names[i]);
+ error_report("Invalid PCI device address %s for device %s",
+ devaddr, pci_nic_names[i]);
return NULL;
}
@@ -1768,8 +1768,8 @@ static int pci_add_option_rom(PCIDevice *pdev)
size = get_image_size(path);
if (size < 0) {
- qemu_error("%s: failed to find romfile \"%s\"\n", __FUNCTION__,
- pdev->romfile);
+ error_report("%s: failed to find romfile \"%s\"",
+ __FUNCTION__, pdev->romfile);
return -1;
}
if (size & (size - 1)) {
diff --git a/hw/qdev.c b/hw/qdev.c
index 253b992..f170ea0 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -139,8 +139,8 @@ static int set_property(const char *name, const char *value, void *opaque)
return 0;
if (qdev_prop_parse(dev, name, value) == -1) {
- qemu_error("can't set property \"%s\" to \"%s\" for \"%s\"\n",
- name, value, dev->info->name);
+ error_report("can't set property \"%s\" to \"%s\" for \"%s\"",
+ name, value, dev->info->name);
return -1;
}
return 0;
@@ -184,7 +184,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
driver = qemu_opt_get(opts, "driver");
if (!driver) {
- qemu_error("-device: no driver specified\n");
+ error_report("-device: no driver specified");
return NULL;
}
@@ -195,8 +195,8 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return NULL;
}
if (info->no_user) {
- qemu_error("device \"%s\" can't be added via command line\n",
- info->name);
+ error_report("device \"%s\" can't be added via command line",
+ info->name);
return NULL;
}
@@ -208,13 +208,13 @@ DeviceState *qdev_device_add(QemuOpts *opts)
bus = qbus_find_recursive(main_system_bus, NULL, info->bus_info);
}
if (!bus) {
- qemu_error("Did not find %s bus for %s\n",
- path ? path : info->bus_info->name, info->name);
+ error_report("Did not find %s bus for %s",
+ path ? path : info->bus_info->name, info->name);
return NULL;
}
if (qdev_hotplug && !bus->allow_hotplug) {
- qemu_error("Bus %s does not support hotplugging\n",
- bus->name);
+ error_report("Bus %s does not support hotplugging",
+ bus->name);
return NULL;
}
@@ -229,7 +229,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return NULL;
}
if (qdev_init(qdev) < 0) {
- qemu_error("Error initializing device %s\n", driver);
+ error_report("Error initializing device %s", driver);
return NULL;
}
qdev->opts = opts;
@@ -268,8 +268,8 @@ int qdev_init(DeviceState *dev)
int qdev_unplug(DeviceState *dev)
{
if (!dev->parent_bus->allow_hotplug) {
- qemu_error("Bus %s does not support hotplugging\n",
- dev->parent_bus->name);
+ error_report("Bus %s does not support hotplugging",
+ dev->parent_bus->name);
return -1;
}
assert(dev->info->unplug != NULL);
@@ -538,12 +538,12 @@ static BusState *qbus_find(const char *path)
pos = 0;
} else {
if (sscanf(path, "%127[^/]%n", elem, &len) != 1) {
- qemu_error("path parse error (\"%s\")\n", path);
+ error_report("path parse error (\"%s\")", path);
return NULL;
}
bus = qbus_find_recursive(main_system_bus, elem, NULL);
if (!bus) {
- qemu_error("bus \"%s\" not found\n", elem);
+ error_report("bus \"%s\" not found", elem);
return NULL;
}
pos = len;
@@ -557,13 +557,13 @@ static BusState *qbus_find(const char *path)
/* find device */
if (sscanf(path+pos, "/%127[^/]%n", elem, &len) != 1) {
- qemu_error("path parse error (\"%s\" pos %d)\n", path, pos);
+ error_report("path parse error (\"%s\" pos %d)", path, pos);
return NULL;
}
pos += len;
dev = qbus_find_dev(bus, elem);
if (!dev) {
- qemu_error("device \"%s\" not found\n", elem);
+ error_report("device \"%s\" not found", elem);
qbus_list_dev(bus);
return NULL;
}
@@ -572,12 +572,12 @@ static BusState *qbus_find(const char *path)
* one child bus accept it nevertheless */
switch (dev->num_child_bus) {
case 0:
- qemu_error("device has no child bus (%s)\n", path);
+ error_report("device has no child bus (%s)", path);
return NULL;
case 1:
return QLIST_FIRST(&dev->child_bus);
default:
- qemu_error("device has multiple child busses (%s)\n", path);
+ error_report("device has multiple child busses (%s)", path);
qbus_list_bus(dev);
return NULL;
}
@@ -585,13 +585,13 @@ static BusState *qbus_find(const char *path)
/* find bus */
if (sscanf(path+pos, "/%127[^/]%n", elem, &len) != 1) {
- qemu_error("path parse error (\"%s\" pos %d)\n", path, pos);
+ error_report("path parse error (\"%s\" pos %d)", path, pos);
return NULL;
}
pos += len;
bus = qbus_find_bus(dev, elem);
if (!bus) {
- qemu_error("child bus \"%s\" not found\n", elem);
+ error_report("child bus \"%s\" not found", elem);
qbus_list_bus(dev);
return NULL;
}
@@ -749,7 +749,7 @@ void do_device_del(Monitor *mon, const QDict *qdict)
dev = qdev_find_recursive(main_system_bus, id);
if (NULL == dev) {
- qemu_error("Device '%s' not found\n", id);
+ error_report("Device '%s' not found", id);
return;
}
qdev_unplug(dev);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index c41ce9b..383240b 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -41,7 +41,7 @@ static int scsi_qdev_init(DeviceState *qdev, DeviceInfo *base)
}
}
if (dev->id >= bus->ndev) {
- qemu_error("bad scsi device id: %d\n", dev->id);
+ error_report("bad scsi device id: %d", dev->id);
goto err;
}
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b1424de..3671589 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1021,7 +1021,7 @@ static int scsi_disk_initfn(SCSIDevice *dev)
uint64_t nb_sectors;
if (!s->qdev.conf.dinfo || !s->qdev.conf.dinfo->bdrv) {
- qemu_error("scsi-disk: drive property not set\n");
+ error_report("scsi-disk: drive property not set");
return -1;
}
s->bs = s->qdev.conf.dinfo->bdrv;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index cfd9903..c9aa853 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -464,27 +464,27 @@ static int scsi_generic_initfn(SCSIDevice *dev)
struct sg_scsi_id scsiid;
if (!s->qdev.conf.dinfo || !s->qdev.conf.dinfo->bdrv) {
- qemu_error("scsi-generic: drive property not set\n");
+ error_report("scsi-generic: drive property not set");
return -1;
}
s->bs = s->qdev.conf.dinfo->bdrv;
/* check we are really using a /dev/sg* file */
if (!bdrv_is_sg(s->bs)) {
- qemu_error("scsi-generic: not /dev/sg*\n");
+ error_report("scsi-generic: not /dev/sg*");
return -1;
}
/* check we are using a driver managing SG_IO (version 3 and after */
if (bdrv_ioctl(s->bs, SG_GET_VERSION_NUM, &sg_version) < 0 ||
sg_version < 30000) {
- qemu_error("scsi-generic: scsi generic interface too old\n");
+ error_report("scsi-generic: scsi generic interface too old");
return -1;
}
/* get LUN of the /dev/sg? */
if (bdrv_ioctl(s->bs, SG_GET_SCSI_ID, &scsiid)) {
- qemu_error("scsi-generic: SG_GET_SCSI_ID ioctl failed\n");
+ error_report("scsi-generic: SG_GET_SCSI_ID ioctl failed");
return -1;
}
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 7c82314..e2d87f2 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -291,14 +291,14 @@ USBDevice *usbdevice_create(const char *cmdline)
if (info == NULL) {
#if 0
/* no error because some drivers are not converted (yet) */
- qemu_error("usbdevice %s not found\n", driver);
+ error_report("usbdevice %s not found", driver);
#endif
return NULL;
}
if (!usb->usbdevice_init) {
if (params) {
- qemu_error("usbdevice %s accepts no params\n", driver);
+ error_report("usbdevice %s accepts no params", driver);
return NULL;
}
return usb_create_simple(bus, usb->qdev.name);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 0afb031..e90a47e 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -524,7 +524,7 @@ static int usb_msd_initfn(USBDevice *dev)
MSDState *s = DO_UPCAST(MSDState, dev, dev);
if (!s->conf.dinfo || !s->conf.dinfo->bdrv) {
- qemu_error("usb-msd: drive property not set\n");
+ error_report("usb-msd: drive property not set");
return -1;
}
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index 6db9446..69f0e44 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -565,26 +565,26 @@ static USBDevice *usb_serial_init(const char *filename)
if (strstart(filename, "vendorid=", &p)) {
vendorid = strtol(p, &e, 16);
if (e == p || (*e && *e != ',' && *e != ':')) {
- qemu_error("bogus vendor ID %s\n", p);
+ error_report("bogus vendor ID %s", p);
return NULL;
}
filename = e;
} else if (strstart(filename, "productid=", &p)) {
productid = strtol(p, &e, 16);
if (e == p || (*e && *e != ',' && *e != ':')) {
- qemu_error("bogus product ID %s\n", p);
+ error_report("bogus product ID %s", p);
return NULL;
}
filename = e;
} else {
- qemu_error("unrecognized serial USB option %s\n", filename);
+ error_report("unrecognized serial USB option %s", filename);
return NULL;
}
while(*filename == ',')
filename++;
}
if (!*filename) {
- qemu_error("character device specification needed\n");
+ error_report("character device specification needed");
return NULL;
}
filename++;
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 8359be6..be33c68 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -765,7 +765,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
if (version_id >= 7) {
if (qemu_get_be32(f) && !peer_has_vnet_hdr(n)) {
- qemu_error("virtio-net: saved image requires vnet_hdr=on\n");
+ error_report("virtio-net: saved image requires vnet_hdr=on");
return -1;
}
@@ -794,7 +794,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
if (version_id >= 11) {
if (qemu_get_byte(f) && !peer_has_ufo(n)) {
- qemu_error("virtio-net: saved image requires TUN_F_UFO support\n");
+ error_report("virtio-net: saved image requires TUN_F_UFO support");
return -1;
}
}
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 7781328..919ce2b 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -459,7 +459,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
proxy->class_code = PCI_CLASS_STORAGE_SCSI;
if (!proxy->block.dinfo) {
- qemu_error("virtio-blk-pci: drive property not set\n");
+ error_report("virtio-blk-pci: drive property not set");
return -1;
}
vdev = virtio_blk_init(&pci_dev->qdev, &proxy->block);
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index d0e0219..17c1ec1 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -485,7 +485,7 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
plugging_port0 = port->is_console && !find_port_by_id(port->vser, 0);
if (port->vser->config.nr_ports == bus->max_nr_ports && !plugging_port0) {
- qemu_error("virtio-serial-bus: Maximum device limit reached\n");
+ error_report("virtio-serial-bus: Maximum device limit reached");
return -1;
}
dev->info = info;
diff --git a/net.c b/net.c
index a1bf49f..503af26 100644
--- a/net.c
+++ b/net.c
@@ -731,7 +731,7 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
return i;
}
- qemu_error("qemu: Unsupported NIC model: %s\n", nd->model);
+ error_report("qemu: Unsupported NIC model: %s", nd->model);
return -1;
}
@@ -742,7 +742,7 @@ int net_handle_fd_param(Monitor *mon, const char *param)
fd = monitor_get_fd(mon, param);
if (fd == -1) {
- qemu_error("No file descriptor named %s found", param);
+ error_report("No file descriptor named %s found", param);
return -1;
}
@@ -763,7 +763,7 @@ static int net_init_nic(QemuOpts *opts,
idx = nic_get_free_idx();
if (idx == -1 || nb_nics >= MAX_NICS) {
- qemu_error("Too Many NICs\n");
+ error_report("Too Many NICs");
return -1;
}
@@ -774,7 +774,7 @@ static int net_init_nic(QemuOpts *opts,
if ((netdev = qemu_opt_get(opts, "netdev"))) {
nd->netdev = qemu_find_netdev(netdev);
if (!nd->netdev) {
- qemu_error("netdev '%s' not found\n", netdev);
+ error_report("netdev '%s' not found", netdev);
return -1;
}
} else {
@@ -800,14 +800,14 @@ static int net_init_nic(QemuOpts *opts,
if (qemu_opt_get(opts, "macaddr") &&
net_parse_macaddr(nd->macaddr, qemu_opt_get(opts, "macaddr")) < 0) {
- qemu_error("invalid syntax for ethernet address\n");
+ error_report("invalid syntax for ethernet address");
return -1;
}
nd->nvectors = qemu_opt_get_number(opts, "vectors", NIC_NVECTORS_UNSPECIFIED);
if (nd->nvectors != NIC_NVECTORS_UNSPECIFIED &&
(nd->nvectors < 0 || nd->nvectors > 0x7ffffff)) {
- qemu_error("invalid # of vectors: %d\n", nd->nvectors);
+ error_report("invalid # of vectors: %d", nd->nvectors);
return -1;
}
@@ -1057,12 +1057,12 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
if (!is_netdev) {
if (!type) {
- qemu_error("No type specified for -net\n");
+ error_report("No type specified for -net");
return -1;
}
} else {
if (!type) {
- qemu_error("No type specified for -netdev\n");
+ error_report("No type specified for -netdev");
return -1;
}
@@ -1074,21 +1074,21 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
strcmp(type, "vde") != 0 &&
#endif
strcmp(type, "socket") != 0) {
- qemu_error("The '%s' network backend type is not valid with -netdev\n",
- type);
+ error_report("The '%s' network backend type is not valid with -netdev",
+ type);
return -1;
}
if (qemu_opt_get(opts, "vlan")) {
- qemu_error("The 'vlan' parameter is not valid with -netdev\n");
+ error_report("The 'vlan' parameter is not valid with -netdev");
return -1;
}
if (qemu_opt_get(opts, "name")) {
- qemu_error("The 'name' parameter is not valid with -netdev\n");
+ error_report("The 'name' parameter is not valid with -netdev");
return -1;
}
if (!qemu_opts_id(opts)) {
- qemu_error("The id= parameter is required with -netdev\n");
+ error_report("The id= parameter is required with -netdev");
return -1;
}
}
@@ -1121,7 +1121,7 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
}
}
- qemu_error("Invalid -net type '%s'\n", type);
+ error_report("Invalid -net type '%s'", type);
return -1;
}
diff --git a/net/dump.c b/net/dump.c
index e702830..6db7ecf 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -108,7 +108,7 @@ static int net_dump_init(VLANState *vlan, const char *device,
fd = open(filename, O_CREAT | O_WRONLY | O_BINARY, 0644);
if (fd < 0) {
- qemu_error("-net dump: can't open %s\n", filename);
+ error_report("-net dump: can't open %s", filename);
return -1;
}
@@ -121,7 +121,7 @@ static int net_dump_init(VLANState *vlan, const char *device,
hdr.linktype = 1;
if (write(fd, &hdr, sizeof(hdr)) < sizeof(hdr)) {
- qemu_error("-net dump write error: %s\n", strerror(errno));
+ error_report("-net dump write error: %s", strerror(errno));
close(fd);
return -1;
}
diff --git a/net/slirp.c b/net/slirp.c
index 317cca7..20af33a 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -413,14 +413,14 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str,
if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port, guest_addr,
guest_port) < 0) {
- qemu_error("could not set up host forwarding rule '%s'\n",
- redir_str);
+ error_report("could not set up host forwarding rule '%s'",
+ redir_str);
return -1;
}
return 0;
fail_syntax:
- qemu_error("invalid host forwarding rule '%s'\n", redir_str);
+ error_report("invalid host forwarding rule '%s'", redir_str);
return -1;
}
@@ -473,10 +473,10 @@ static void slirp_smb_cleanup(SlirpState *s)
snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
ret = system(cmd);
if (!WIFEXITED(ret)) {
- qemu_error("'%s' failed.\n", cmd);
+ error_report("'%s' failed.", cmd);
} else if (WEXITSTATUS(ret)) {
- qemu_error("'%s' failed. Error code: %d\n",
- cmd, WEXITSTATUS(ret));
+ error_report("'%s' failed. Error code: %d",
+ cmd, WEXITSTATUS(ret));
}
s->smb_dir[0] = '\0';
}
@@ -493,7 +493,7 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
(long)getpid(), instance++);
if (mkdir(s->smb_dir, 0700) < 0) {
- qemu_error("could not create samba server dir '%s'\n", s->smb_dir);
+ error_report("could not create samba server dir '%s'", s->smb_dir);
return -1;
}
snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
@@ -501,8 +501,8 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
f = fopen(smb_conf, "w");
if (!f) {
slirp_smb_cleanup(s);
- qemu_error("could not create samba server configuration file '%s'\n",
- smb_conf);
+ error_report("could not create samba server configuration file '%s'",
+ smb_conf);
return -1;
}
fprintf(f,
@@ -533,7 +533,7 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
if (slirp_add_exec(s->slirp, 0, smb_cmdline, &vserver_addr, 139) < 0) {
slirp_smb_cleanup(s);
- qemu_error("conflicting/invalid smbserver address\n");
+ error_report("conflicting/invalid smbserver address");
return -1;
}
return 0;
@@ -618,14 +618,14 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str,
snprintf(buf, sizeof(buf), "guestfwd.tcp:%d", port);
fwd->hd = qemu_chr_open(buf, p, NULL);
if (!fwd->hd) {
- qemu_error("could not open guest forwarding device '%s'\n", buf);
+ error_report("could not open guest forwarding device '%s'", buf);
qemu_free(fwd);
return -1;
}
if (slirp_add_exec(s->slirp, 3, fwd->hd, &server, port) < 0) {
- qemu_error("conflicting/invalid host:port in guest forwarding "
- "rule '%s'\n", config_str);
+ error_report("conflicting/invalid host:port in guest forwarding "
+ "rule '%s'", config_str);
qemu_free(fwd);
return -1;
}
@@ -638,7 +638,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str,
return 0;
fail_syntax:
- qemu_error("invalid guest forwarding rule '%s'\n", config_str);
+ error_report("invalid guest forwarding rule '%s'", config_str);
return -1;
}
diff --git a/net/socket.c b/net/socket.c
index 474d573..1c4e153 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -506,7 +506,7 @@ int net_init_socket(QemuOpts *opts,
if (qemu_opt_get(opts, "listen") ||
qemu_opt_get(opts, "connect") ||
qemu_opt_get(opts, "mcast")) {
- qemu_error("listen=, connect= and mcast= is invalid with fd=\n");
+ error_report("listen=, connect= and mcast= is invalid with fd=");
return -1;
}
@@ -525,7 +525,7 @@ int net_init_socket(QemuOpts *opts,
if (qemu_opt_get(opts, "fd") ||
qemu_opt_get(opts, "connect") ||
qemu_opt_get(opts, "mcast")) {
- qemu_error("fd=, connect= and mcast= is invalid with listen=\n");
+ error_report("fd=, connect= and mcast= is invalid with listen=");
return -1;
}
@@ -540,7 +540,7 @@ int net_init_socket(QemuOpts *opts,
if (qemu_opt_get(opts, "fd") ||
qemu_opt_get(opts, "listen") ||
qemu_opt_get(opts, "mcast")) {
- qemu_error("fd=, listen= and mcast= is invalid with connect=\n");
+ error_report("fd=, listen= and mcast= is invalid with connect=");
return -1;
}
@@ -555,7 +555,7 @@ int net_init_socket(QemuOpts *opts,
if (qemu_opt_get(opts, "fd") ||
qemu_opt_get(opts, "connect") ||
qemu_opt_get(opts, "listen")) {
- qemu_error("fd=, connect= and listen= is invalid with mcast=\n");
+ error_report("fd=, connect= and listen= is invalid with mcast=");
return -1;
}
@@ -565,7 +565,7 @@ int net_init_socket(QemuOpts *opts,
return -1;
}
} else {
- qemu_error("-socket requires fd=, listen=, connect= or mcast=\n");
+ error_report("-socket requires fd=, listen=, connect= or mcast=");
return -1;
}
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 815997d..e51d068 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -69,7 +69,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
}
}
if (fd < 0) {
- qemu_error("warning: could not open %s (%s): no virtual network emulation\n", dname, strerror(errno));
+ error_report("warning: could not open %s (%s): no virtual network emulation",
+ dname, strerror(errno));
return -1;
}
#else
@@ -89,8 +90,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
- qemu_error("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_report("vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
diff --git a/net/tap-linux.c b/net/tap-linux.c
index c5748e6..03b8301 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -58,8 +58,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
}
if (vnet_hdr_required && !*vnet_hdr) {
- qemu_error("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_report("vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
@@ -97,7 +97,7 @@ int tap_set_sndbuf(int fd, QemuOpts *opts)
}
if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && qemu_opt_get(opts, "sndbuf")) {
- qemu_error("TUNSETSNDBUF ioctl failed: %s\n", strerror(errno));
+ error_report("TUNSETSNDBUF ioctl failed: %s", strerror(errno));
return -1;
}
return 0;
@@ -108,7 +108,7 @@ int tap_probe_vnet_hdr(int fd)
struct ifreq ifr;
if (ioctl(fd, TUNGETIFF, &ifr) != 0) {
- qemu_error("TUNGETIFF ioctl() failed: %s\n", strerror(errno));
+ error_report("TUNGETIFF ioctl() failed: %s", strerror(errno));
return 0;
}
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 0b42861..50d127a 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -186,8 +186,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
- qemu_error("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_report("vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 8370c80..a5c2ce9 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -706,7 +706,7 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
ifname = qemu_opt_get(opts, "ifname");
if (!ifname) {
- qemu_error("tap: no interface name\n");
+ error_report("tap: no interface name");
return -1;
}
diff --git a/net/tap.c b/net/tap.c
index 9ba9b4a..672b0ee 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -394,7 +394,7 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
qemu_opt_get(opts, "script") ||
qemu_opt_get(opts, "downscript") ||
qemu_opt_get(opts, "vnet_hdr")) {
- qemu_error("ifname=, script=, downscript= and vnet_hdr= is invalid with fd=\n");
+ error_report("ifname=, script=, downscript= and vnet_hdr= is invalid with fd=");
return -1;
}
diff --git a/qemu-config.c b/qemu-config.c
index 2c9a7a5..8e06770 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -313,7 +313,7 @@ static QemuOptsList *find_list(const char *group)
break;
}
if (lists[i] == NULL) {
- qemu_error("there is no option group \"%s\"\n", group);
+ error_report("there is no option group \"%s\"", group);
}
return lists[i];
}
@@ -327,7 +327,7 @@ int qemu_set_option(const char *str)
rc = sscanf(str, "%63[^.].%63[^.].%63[^=]%n", group, id, arg, &offset);
if (rc < 3 || str[offset] != '=') {
- qemu_error("can't parse: \"%s\"\n", str);
+ error_report("can't parse: \"%s\"", str);
return -1;
}
@@ -338,8 +338,8 @@ int qemu_set_option(const char *str)
opts = qemu_opts_find(list, id);
if (!opts) {
- qemu_error("there is no %s \"%s\" defined\n",
- list->name, id);
+ error_report("there is no %s \"%s\" defined",
+ list->name, id);
return -1;
}
@@ -357,7 +357,7 @@ int qemu_global_option(const char *str)
rc = sscanf(str, "%63[^.].%63[^=]%n", driver, property, &offset);
if (rc < 2 || str[offset] != '=') {
- qemu_error("can't parse: \"%s\"\n", str);
+ error_report("can't parse: \"%s\"", str);
return -1;
}
diff --git a/qemu-error.c b/qemu-error.c
index d20fd0f..51e2abf 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -41,13 +41,19 @@ void error_printf(const char *fmt, ...)
va_end(ap);
}
-void qemu_error(const char *fmt, ...)
+/*
+ * Print an error message to current monitor if we have one, else to stderr.
+ * Appends a newline to the message.
+ * It's wrong to call this in a QMP monitor. Use qemu_error_new() there.
+ */
+void error_report(const char *fmt, ...)
{
va_list ap;
va_start(ap, fmt);
error_vprintf(fmt, ap);
va_end(ap);
+ error_printf("\n");
}
void qemu_error_internal(const char *file, int linenr, const char *func,
diff --git a/qemu-error.h b/qemu-error.h
index d90f1da..99dfcce 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -15,7 +15,7 @@
void error_vprintf(const char *fmt, va_list ap);
void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
-void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
+void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void qemu_error_internal(const char *file, int linenr, const char *func,
const char *fmt, ...)
__attribute__ ((format(printf, 4, 5)));
diff --git a/qemu-tool.c b/qemu-tool.c
index 26f46eb..939049a 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -104,7 +104,7 @@ int64_t qemu_get_clock(QEMUClock *clock)
return (tv.tv_sec * 1000000000LL + (tv.tv_usec * 1000)) / 1000000;
}
-void qemu_error(const char *fmt, ...)
+void error_report(const char *fmt, ...)
{
va_list args;
diff --git a/qerror.c b/qerror.c
index a418bde..a6f0213 100644
--- a/qerror.c
+++ b/qerror.c
@@ -318,13 +318,13 @@ QString *qerror_human(const QError *qerror)
* qerror_print(): Print QError data
*
* This function will print the member 'desc' of the specified QError object,
- * it uses qemu_error() for this, so that the output is routed to the right
+ * it uses error_report() for this, so that the output is routed to the right
* place (ie. stderr or Monitor's device).
*/
void qerror_print(const QError *qerror)
{
QString *qstring = qerror_human(qerror);
- qemu_error("%s\n", qstring_get_str(qstring));
+ error_report("%s", qstring_get_str(qstring));
QDECREF(qstring);
}
diff --git a/savevm.c b/savevm.c
index 1f6c25e..0aef5cf 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1743,7 +1743,7 @@ int load_vmstate(const char *name)
bs = get_bs_snapshots();
if (!bs) {
- qemu_error("No block device supports snapshots\n");
+ error_report("No block device supports snapshots");
return -EINVAL;
}
@@ -1755,20 +1755,21 @@ int load_vmstate(const char *name)
if (bdrv_has_snapshot(bs1)) {
ret = bdrv_snapshot_goto(bs1, name);
if (ret < 0) {
- if (bs != bs1)
- qemu_error("Warning: ");
switch(ret) {
case -ENOTSUP:
- qemu_error("Snapshots not supported on device '%s'\n",
- bdrv_get_device_name(bs1));
+ error_report("%sSnapshots not supported on device '%s'",
+ bs != bs1 ? "Warning: " : "",
+ bdrv_get_device_name(bs1));
break;
case -ENOENT:
- qemu_error("Could not find snapshot '%s' on device '%s'\n",
- name, bdrv_get_device_name(bs1));
+ error_report("%sCould not find snapshot '%s' on device '%s'",
+ bs != bs1 ? "Warning: " : "",
+ name, bdrv_get_device_name(bs1));
break;
default:
- qemu_error("Error %d while activating snapshot on '%s'\n",
- ret, bdrv_get_device_name(bs1));
+ error_report("%sError %d while activating snapshot on '%s'",
+ bs != bs1 ? "Warning: " : "",
+ ret, bdrv_get_device_name(bs1));
break;
}
/* fatal on snapshot block device */
@@ -1786,13 +1787,13 @@ int load_vmstate(const char *name)
/* restore the VM state */
f = qemu_fopen_bdrv(bs, 0);
if (!f) {
- qemu_error("Could not open VM state file\n");
+ error_report("Could not open VM state file");
return -EINVAL;
}
ret = qemu_loadvm_state(f);
qemu_fclose(f);
if (ret < 0) {
- qemu_error("Error %d while loading VM state\n", ret);
+ error_report("Error %d while loading VM state", ret);
return ret;
}
return 0;
diff --git a/vl.c b/vl.c
index c4c8c1f..e8a368b 100644
--- a/vl.c
+++ b/vl.c
@@ -2526,7 +2526,7 @@ void do_usb_add(Monitor *mon, const QDict *qdict)
{
const char *devname = qdict_get_str(qdict, "devname");
if (usb_device_add(devname, 1) < 0) {
- qemu_error("could not add USB device '%s'\n", devname);
+ error_report("could not add USB device '%s'", devname);
}
}
@@ -2534,7 +2534,7 @@ void do_usb_del(Monitor *mon, const QDict *qdict)
{
const char *devname = qdict_get_str(qdict, "devname");
if (usb_device_del(devname) < 0) {
- qemu_error("could not delete USB device '%s'\n", devname);
+ error_report("could not delete USB device '%s'", devname);
}
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 17/50] error: Rename qemu_error_new() to qerror_report()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (15 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 16/50] error: Replace qemu_error() by error_report() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 18/50] error: Infrastructure to track locations for error reporting Markus Armbruster
` (33 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 2 +-
monitor.c | 80 +++++++++++++++++++++++++++++-----------------------------
qemu-error.c | 6 ++--
qemu-error.h | 10 +++---
4 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index f170ea0..25e1703 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -191,7 +191,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* find driver */
info = qdev_find_info(NULL, driver);
if (!info) {
- qemu_error_new(QERR_DEVICE_NOT_FOUND, driver);
+ qerror_report(QERR_DEVICE_NOT_FOUND, driver);
return NULL;
}
if (info->no_user) {
diff --git a/monitor.c b/monitor.c
index 8dbcb62..55d78d5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -208,7 +208,7 @@ static int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque)
{
if (monitor_ctrl_mode(mon)) {
- qemu_error_new(QERR_MISSING_PARAMETER, "password");
+ qerror_report(QERR_MISSING_PARAMETER, "password");
return -EINVAL;
} else if (mon->rs) {
readline_start(mon->rs, "Password: ", 1, readline_func, opaque);
@@ -604,7 +604,7 @@ static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
if (cmd->name == NULL) {
if (monitor_ctrl_mode(mon)) {
- qemu_error_new(QERR_COMMAND_NOT_FOUND, item);
+ qerror_report(QERR_COMMAND_NOT_FOUND, item);
return -1;
}
goto help;
@@ -636,7 +636,7 @@ static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
} else {
if (monitor_ctrl_mode(mon)) {
/* handler not converted yet */
- qemu_error_new(QERR_COMMAND_NOT_FOUND, item);
+ qerror_report(QERR_COMMAND_NOT_FOUND, item);
return -1;
} else {
cmd->mhandler.info(mon);
@@ -958,7 +958,7 @@ static int do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
int index = qdict_get_int(qdict, "index");
if (mon_set_cpu(index) < 0) {
- qemu_error_new(QERR_INVALID_PARAMETER, "index");
+ qerror_report(QERR_INVALID_PARAMETER, "index");
return -1;
}
return 0;
@@ -1011,12 +1011,12 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
if (bdrv_is_inserted(bs)) {
if (!force) {
if (!bdrv_is_removable(bs)) {
- qemu_error_new(QERR_DEVICE_NOT_REMOVABLE,
+ qerror_report(QERR_DEVICE_NOT_REMOVABLE,
bdrv_get_device_name(bs));
return -1;
}
if (bdrv_is_locked(bs)) {
- qemu_error_new(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+ qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
return -1;
}
}
@@ -1033,7 +1033,7 @@ static int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
bs = bdrv_find(filename);
if (!bs) {
- qemu_error_new(QERR_DEVICE_NOT_FOUND, filename);
+ qerror_report(QERR_DEVICE_NOT_FOUND, filename);
return -1;
}
return eject_device(mon, bs, force);
@@ -1046,12 +1046,12 @@ static int do_block_set_passwd(Monitor *mon, const QDict *qdict,
bs = bdrv_find(qdict_get_str(qdict, "device"));
if (!bs) {
- qemu_error_new(QERR_DEVICE_NOT_FOUND, qdict_get_str(qdict, "device"));
+ qerror_report(QERR_DEVICE_NOT_FOUND, qdict_get_str(qdict, "device"));
return -1;
}
if (bdrv_set_key(bs, qdict_get_str(qdict, "password")) < 0) {
- qemu_error_new(QERR_INVALID_PASSWORD);
+ qerror_report(QERR_INVALID_PASSWORD);
return -1;
}
@@ -1066,13 +1066,13 @@ static int do_change_block(Monitor *mon, const char *device,
bs = bdrv_find(device);
if (!bs) {
- qemu_error_new(QERR_DEVICE_NOT_FOUND, device);
+ qerror_report(QERR_DEVICE_NOT_FOUND, device);
return -1;
}
if (fmt) {
drv = bdrv_find_whitelisted_format(fmt);
if (!drv) {
- qemu_error_new(QERR_INVALID_BLOCK_FORMAT, fmt);
+ qerror_report(QERR_INVALID_BLOCK_FORMAT, fmt);
return -1;
}
}
@@ -1088,7 +1088,7 @@ static int do_change_block(Monitor *mon, const char *device,
static int change_vnc_password(const char *password)
{
if (vnc_display_password(NULL, password) < 0) {
- qemu_error_new(QERR_SET_PASSWD_FAILED);
+ qerror_report(QERR_SET_PASSWD_FAILED);
return -1;
}
@@ -1116,7 +1116,7 @@ static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
}
} else {
if (vnc_display_open(NULL, target) < 0) {
- qemu_error_new(QERR_VNC_SERVER_FAILED, target);
+ qerror_report(QERR_VNC_SERVER_FAILED, target);
return -1;
}
}
@@ -1488,7 +1488,7 @@ static int do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data)
f = fopen(filename, "wb");
if (!f) {
- qemu_error_new(QERR_OPEN_FILE_FAILED, filename);
+ qerror_report(QERR_OPEN_FILE_FAILED, filename);
return -1;
}
while (size != 0) {
@@ -1524,7 +1524,7 @@ static int do_physical_memory_save(Monitor *mon, const QDict *qdict,
f = fopen(filename, "wb");
if (!f) {
- qemu_error_new(QERR_OPEN_FILE_FAILED, filename);
+ qerror_report(QERR_OPEN_FILE_FAILED, filename);
return -1;
}
while (size != 0) {
@@ -2298,13 +2298,13 @@ static int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
int ret;
if (kvm_enabled() && !kvm_has_sync_mmu()) {
- qemu_error_new(QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
+ qerror_report(QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
return -1;
}
ret = qemu_balloon_status(cb, opaque);
if (!ret) {
- qemu_error_new(QERR_DEVICE_NOT_ACTIVE, "balloon");
+ qerror_report(QERR_DEVICE_NOT_ACTIVE, "balloon");
return -1;
}
@@ -2321,13 +2321,13 @@ static int do_balloon(Monitor *mon, const QDict *params,
int ret;
if (kvm_enabled() && !kvm_has_sync_mmu()) {
- qemu_error_new(QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
+ qerror_report(QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
return -1;
}
ret = qemu_balloon(qdict_get_int(params, "value"), cb, opaque);
if (ret == 0) {
- qemu_error_new(QERR_DEVICE_NOT_ACTIVE, "balloon");
+ qerror_report(QERR_DEVICE_NOT_ACTIVE, "balloon");
return -1;
}
@@ -2467,21 +2467,21 @@ static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
fd = qemu_chr_get_msgfd(mon->chr);
if (fd == -1) {
- qemu_error_new(QERR_FD_NOT_SUPPLIED);
+ qerror_report(QERR_FD_NOT_SUPPLIED);
return -1;
}
if (qemu_isdigit(fdname[0])) {
- qemu_error_new(QERR_INVALID_PARAMETER, "fdname");
+ qerror_report(QERR_INVALID_PARAMETER, "fdname");
return -1;
}
fd = dup(fd);
if (fd == -1) {
if (errno == EMFILE)
- qemu_error_new(QERR_TOO_MANY_FILES);
+ qerror_report(QERR_TOO_MANY_FILES);
else
- qemu_error_new(QERR_UNDEFINED_ERROR);
+ qerror_report(QERR_UNDEFINED_ERROR);
return -1;
}
@@ -2520,7 +2520,7 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
return 0;
}
- qemu_error_new(QERR_FD_NOT_FOUND, fdname);
+ qerror_report(QERR_FD_NOT_FOUND, fdname);
return -1;
}
@@ -3892,7 +3892,7 @@ static void handler_audit(Monitor *mon, const mon_cmd_t *cmd, int ret)
* Action: Report an internal error to the client if in QMP.
*/
if (monitor_ctrl_mode(mon)) {
- qemu_error_new(QERR_UNDEFINED_ERROR);
+ qerror_report(QERR_UNDEFINED_ERROR);
}
MON_DEBUG("command '%s' returned failure but did not pass an error\n",
cmd->name);
@@ -4215,7 +4215,7 @@ typedef struct CmdArgs {
static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
{
if (!cmd_args->optional) {
- qemu_error_new(QERR_MISSING_PARAMETER, name);
+ qerror_report(QERR_MISSING_PARAMETER, name);
return -1;
}
@@ -4248,7 +4248,7 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
case 'B':
case 's':
if (qobject_type(value) != QTYPE_QSTRING) {
- qemu_error_new(QERR_INVALID_PARAMETER_TYPE, name, "string");
+ qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "string");
return -1;
}
break;
@@ -4259,11 +4259,11 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
for (i = 0; keys[i]; i++) {
QObject *obj = qdict_get(args, keys[i]);
if (!obj) {
- qemu_error_new(QERR_MISSING_PARAMETER, name);
+ qerror_report(QERR_MISSING_PARAMETER, name);
return -1;
}
if (qobject_type(obj) != QTYPE_QINT) {
- qemu_error_new(QERR_INVALID_PARAMETER_TYPE, name, "int");
+ qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
return -1;
}
}
@@ -4273,21 +4273,21 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
case 'l':
case 'M':
if (qobject_type(value) != QTYPE_QINT) {
- qemu_error_new(QERR_INVALID_PARAMETER_TYPE, name, "int");
+ qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
return -1;
}
break;
case 'b':
case 'T':
if (qobject_type(value) != QTYPE_QINT && qobject_type(value) != QTYPE_QFLOAT) {
- qemu_error_new(QERR_INVALID_PARAMETER_TYPE, name, "number");
+ qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "number");
return -1;
}
break;
case '-':
if (qobject_type(value) != QTYPE_QINT &&
qobject_type(value) != QTYPE_QBOOL) {
- qemu_error_new(QERR_INVALID_PARAMETER_TYPE, name, "bool");
+ qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
return -1;
}
if (qobject_type(value) == QTYPE_QBOOL) {
@@ -4384,10 +4384,10 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
obj = json_parser_parse(tokens, NULL);
if (!obj) {
// FIXME: should be triggered in json_parser_parse()
- qemu_error_new(QERR_JSON_PARSING);
+ qerror_report(QERR_JSON_PARSING);
goto err_out;
} else if (qobject_type(obj) != QTYPE_QDICT) {
- qemu_error_new(QERR_QMP_BAD_INPUT_OBJECT, "object");
+ qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "object");
qobject_decref(obj);
goto err_out;
}
@@ -4399,17 +4399,17 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
obj = qdict_get(input, "execute");
if (!obj) {
- qemu_error_new(QERR_QMP_BAD_INPUT_OBJECT, "execute");
+ qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
goto err_input;
} else if (qobject_type(obj) != QTYPE_QSTRING) {
- qemu_error_new(QERR_QMP_BAD_INPUT_OBJECT, "string");
+ qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "string");
goto err_input;
}
cmd_name = qstring_get_str(qobject_to_qstring(obj));
if (invalid_qmp_mode(mon, cmd_name)) {
- qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
+ qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
goto err_input;
}
@@ -4418,7 +4418,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
* converted into 'query-' commands
*/
if (compare_cmd(cmd_name, "info")) {
- qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
+ qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
goto err_input;
} else if (strstart(cmd_name, "query-", &info_item)) {
cmd = monitor_find_command("info");
@@ -4427,7 +4427,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
} else {
cmd = monitor_find_command(cmd_name);
if (!cmd || !monitor_handler_ported(cmd)) {
- qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
+ qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
goto err_input;
}
}
@@ -4660,7 +4660,7 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
}
if (monitor_ctrl_mode(mon)) {
- qemu_error_new(QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs));
+ qerror_report(QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs));
return -1;
}
diff --git a/qemu-error.c b/qemu-error.c
index 51e2abf..0a175d8 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -44,7 +44,7 @@ void error_printf(const char *fmt, ...)
/*
* Print an error message to current monitor if we have one, else to stderr.
* Appends a newline to the message.
- * It's wrong to call this in a QMP monitor. Use qemu_error_new() there.
+ * It's wrong to call this in a QMP monitor. Use qerror_report() there.
*/
void error_report(const char *fmt, ...)
{
@@ -56,8 +56,8 @@ void error_report(const char *fmt, ...)
error_printf("\n");
}
-void qemu_error_internal(const char *file, int linenr, const char *func,
- const char *fmt, ...)
+void qerror_report_internal(const char *file, int linenr, const char *func,
+ const char *fmt, ...)
{
va_list va;
QError *qerror;
diff --git a/qemu-error.h b/qemu-error.h
index 99dfcce..5b05371 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -16,11 +16,11 @@
void error_vprintf(const char *fmt, va_list ap);
void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
-void qemu_error_internal(const char *file, int linenr, const char *func,
- const char *fmt, ...)
- __attribute__ ((format(printf, 4, 5)));
+void qerror_report_internal(const char *file, int linenr, const char *func,
+ const char *fmt, ...)
+ __attribute__ ((format(printf, 4, 5)));
-#define qemu_error_new(fmt, ...) \
- qemu_error_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
+#define qerror_report(fmt, ...) \
+ qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
#endif
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 18/50] error: Infrastructure to track locations for error reporting
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (16 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 17/50] error: Rename qemu_error_new() to qerror_report() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 19/50] error: Include the program name in error messages to stderr Markus Armbruster
` (32 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
New struct Location holds a location. So far, the only location is
LOC_NONE, so this doesn't do anything useful yet.
Passing the current location all over the place would be too
cumbersome. Hide it away in static cur_loc instead, and provide
accessors. Print it in error_report().
Store it in QError, and print it in qerror_print().
Store it in QemuOpt, for use by qemu_opts_foreach(). This makes
error_report() do the right thing when it runs within
qemu_opts_foreach().
We may still have to store it in other data structures holding user
input for better error messages. Left for another day.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-error.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
qemu-error.h | 16 +++++++++++
qemu-option.c | 7 +++++
qemu-tool.c | 24 ++++++++++++++++
qerror.c | 5 +++-
qerror.h | 4 ++-
6 files changed, 138 insertions(+), 3 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index 0a175d8..0d673c5 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -41,15 +41,98 @@ void error_printf(const char *fmt, ...)
va_end(ap);
}
+static Location std_loc = {
+ .kind = LOC_NONE
+};
+static Location *cur_loc = &std_loc;
+
+/*
+ * Push location saved in LOC onto the location stack, return it.
+ * The top of that stack is the current location.
+ * Needs a matching loc_pop().
+ */
+Location *loc_push_restore(Location *loc)
+{
+ assert(!loc->prev);
+ loc->prev = cur_loc;
+ cur_loc = loc;
+ return loc;
+}
+
+/*
+ * Initialize *LOC to "nowhere", push it onto the location stack.
+ * The top of that stack is the current location.
+ * Needs a matching loc_pop().
+ * Return LOC.
+ */
+Location *loc_push_none(Location *loc)
+{
+ loc->kind = LOC_NONE;
+ loc->prev = NULL;
+ return loc_push_restore(loc);
+}
+
+/*
+ * Pop the location stack.
+ * LOC must be the current location, i.e. the top of the stack.
+ */
+Location *loc_pop(Location *loc)
+{
+ assert(cur_loc == loc && loc->prev);
+ cur_loc = loc->prev;
+ loc->prev = NULL;
+ return loc;
+}
+
+/*
+ * Save the current location in LOC, return LOC.
+ */
+Location *loc_save(Location *loc)
+{
+ *loc = *cur_loc;
+ loc->prev = NULL;
+ return loc;
+}
+
+/*
+ * Change the current location to the one saved in LOC.
+ */
+void loc_restore(Location *loc)
+{
+ Location *prev = cur_loc->prev;
+ assert(!loc->prev);
+ *cur_loc = *loc;
+ cur_loc->prev = prev;
+}
+
+/*
+ * Change the current location to "nowhere in particular".
+ */
+void loc_set_none(void)
+{
+ cur_loc->kind = LOC_NONE;
+}
+
+/*
+ * Print current location to current monitor if we have one, else to stderr.
+ */
+void error_print_loc(void)
+{
+ switch (cur_loc->kind) {
+ default: ;
+ }
+}
+
/*
* Print an error message to current monitor if we have one, else to stderr.
- * Appends a newline to the message.
+ * Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use qerror_report() there.
*/
void error_report(const char *fmt, ...)
{
va_list ap;
+ error_print_loc();
va_start(ap, fmt);
error_vprintf(fmt, ap);
va_end(ap);
diff --git a/qemu-error.h b/qemu-error.h
index 5b05371..a1562aa 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -13,8 +13,24 @@
#ifndef QEMU_ERROR_H
#define QEMU_ERROR_H
+typedef struct Location {
+ /* all members are private to qemu-error.c */
+ enum { LOC_NONE } kind;
+ int num;
+ const void *ptr;
+ struct Location *prev;
+} Location;
+
+Location *loc_push_restore(Location *loc);
+Location *loc_push_none(Location *loc);
+Location *loc_pop(Location *loc);
+Location *loc_save(Location *loc);
+void loc_restore(Location *loc);
+void loc_set_none(void);
+
void error_vprintf(const char *fmt, va_list ap);
void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
+void error_print_loc(void);
void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void qerror_report_internal(const char *file, int linenr, const char *func,
const char *fmt, ...)
diff --git a/qemu-option.c b/qemu-option.c
index de40bff..ab488e4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -27,6 +27,7 @@
#include <string.h>
#include "qemu-common.h"
+#include "qemu-error.h"
#include "qemu-option.h"
/*
@@ -483,6 +484,7 @@ struct QemuOpt {
struct QemuOpts {
char *id;
QemuOptsList *list;
+ Location loc;
QTAILQ_HEAD(QemuOptHead, QemuOpt) head;
QTAILQ_ENTRY(QemuOpts) next;
};
@@ -653,6 +655,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exist
opts->id = qemu_strdup(id);
}
opts->list = list;
+ loc_save(&opts->loc);
QTAILQ_INIT(&opts->head);
QTAILQ_INSERT_TAIL(&list->head, opts, next);
return opts;
@@ -810,13 +813,17 @@ int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
int abort_on_failure)
{
+ Location loc;
QemuOpts *opts;
int rc = 0;
+ loc_push_none(&loc);
QTAILQ_FOREACH(opts, &list->head, next) {
+ loc_restore(&opts->loc);
rc |= func(opts, opaque);
if (abort_on_failure && rc != 0)
break;
}
+ loc_pop(&loc);
return rc;
}
diff --git a/qemu-tool.c b/qemu-tool.c
index 939049a..97ca949 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -104,6 +104,30 @@ int64_t qemu_get_clock(QEMUClock *clock)
return (tv.tv_sec * 1000000000LL + (tv.tv_usec * 1000)) / 1000000;
}
+Location *loc_push_restore(Location *loc)
+{
+ return loc;
+}
+
+Location *loc_push_none(Location *loc)
+{
+ return loc;
+}
+
+Location *loc_pop(Location *loc)
+{
+ return loc;
+}
+
+Location *loc_save(Location *loc)
+{
+ return loc;
+}
+
+void loc_restore(Location *loc)
+{
+}
+
void error_report(const char *fmt, ...)
{
va_list args;
diff --git a/qerror.c b/qerror.c
index a6f0213..b400480 100644
--- a/qerror.c
+++ b/qerror.c
@@ -224,6 +224,7 @@ QError *qerror_from_info(const char *file, int linenr, const char *func,
QError *qerr;
qerr = qerror_new();
+ loc_save(&qerr->loc);
qerr->linenr = linenr;
qerr->file = file;
qerr->func = func;
@@ -321,10 +322,12 @@ QString *qerror_human(const QError *qerror)
* it uses error_report() for this, so that the output is routed to the right
* place (ie. stderr or Monitor's device).
*/
-void qerror_print(const QError *qerror)
+void qerror_print(QError *qerror)
{
QString *qstring = qerror_human(qerror);
+ loc_push_restore(&qerror->loc);
error_report("%s", qstring_get_str(qstring));
+ loc_pop(&qerror->loc);
QDECREF(qstring);
}
diff --git a/qerror.h b/qerror.h
index ee59615..f25be1b 100644
--- a/qerror.h
+++ b/qerror.h
@@ -14,6 +14,7 @@
#include "qdict.h"
#include "qstring.h"
+#include "qemu-error.h"
#include <stdarg.h>
typedef struct QErrorStringTable {
@@ -24,6 +25,7 @@ typedef struct QErrorStringTable {
typedef struct QError {
QObject_HEAD;
QDict *error;
+ Location loc;
int linenr;
const char *file;
const char *func;
@@ -34,7 +36,7 @@ QError *qerror_new(void);
QError *qerror_from_info(const char *file, int linenr, const char *func,
const char *fmt, va_list *va);
QString *qerror_human(const QError *qerror);
-void qerror_print(const QError *qerror);
+void qerror_print(QError *qerror);
QError *qobject_to_qerror(const QObject *obj);
/*
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 19/50] error: Include the program name in error messages to stderr
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (17 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 18/50] error: Infrastructure to track locations for error reporting Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 20/50] error: Track locations in configuration files Markus Armbruster
` (31 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-error.c | 20 +++++++++++++++++++-
qemu-error.h | 1 +
vl.c | 2 ++
3 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index 0d673c5..214e448 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -113,13 +113,31 @@ void loc_set_none(void)
cur_loc->kind = LOC_NONE;
}
+static const char *progname;
+
+/*
+ * Set the program name for error_print_loc().
+ */
+void error_set_progname(const char *argv0)
+{
+ const char *p = strrchr(argv0, '/');
+ progname = p ? p + 1 : argv0;
+}
+
/*
* Print current location to current monitor if we have one, else to stderr.
*/
void error_print_loc(void)
{
+ const char *sep = "";
+
+ if (!cur_mon) {
+ fprintf(stderr, "%s:", progname);
+ sep = " ";
+ }
switch (cur_loc->kind) {
- default: ;
+ default:
+ error_printf(sep);
}
}
diff --git a/qemu-error.h b/qemu-error.h
index a1562aa..204dfb6 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -31,6 +31,7 @@ void loc_set_none(void);
void error_vprintf(const char *fmt, va_list ap);
void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void error_print_loc(void);
+void error_set_progname(const char *argv0);
void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
void qerror_report_internal(const char *file, int linenr, const char *func,
const char *fmt, ...)
diff --git a/vl.c b/vl.c
index e8a368b..2ba8eda 100644
--- a/vl.c
+++ b/vl.c
@@ -4822,6 +4822,8 @@ int main(int argc, char **argv, char **envp)
int show_vnc_port = 0;
int defconfig = 1;
+ error_set_progname(argv[0]);
+
init_clocks();
qemu_cache_utils_init(envp);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 20/50] error: Track locations in configuration files
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (18 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 19/50] error: Include the program name in error messages to stderr Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 21/50] QemuOpts: Fix qemu_config_parse() to catch file read errors Markus Armbruster
` (30 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
New LOC_FILE. Use it for tracking file name and line number in
qemu_config_parse(). We now report errors like
qemu:foo.conf:42: Did not find I2C bus for smbus-eeprom
In particular, gems like this message:
-device: no driver specified
become almost nice now:
qemu:foo.conf:44: -device: no driver specified
(A later commit will get rid of the bogus -device:)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-config.c | 28 +++++++++++++++++-----------
qemu-config.h | 2 +-
qemu-error.c | 20 ++++++++++++++++++++
qemu-error.h | 3 ++-
vl.c | 14 +++++++++-----
5 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 8e06770..2de97cd 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -425,13 +425,17 @@ void qemu_config_write(FILE *fp)
}
}
-int qemu_config_parse(FILE *fp)
+int qemu_config_parse(FILE *fp, const char *fname)
{
char line[1024], group[64], id[64], arg[64], value[1024];
+ Location loc;
QemuOptsList *list = NULL;
QemuOpts *opts = NULL;
+ int res = -1, lno = 0;
+ loc_push_none(&loc);
while (fgets(line, sizeof(line), fp) != NULL) {
+ loc_set_file(fname, ++lno);
if (line[0] == '\n') {
/* skip empty lines */
continue;
@@ -444,7 +448,7 @@ int qemu_config_parse(FILE *fp)
/* group with id */
list = find_list(group);
if (list == NULL)
- return -1;
+ goto out;
opts = qemu_opts_create(list, id, 1);
continue;
}
@@ -452,25 +456,27 @@ int qemu_config_parse(FILE *fp)
/* group without id */
list = find_list(group);
if (list == NULL)
- return -1;
+ goto out;
opts = qemu_opts_create(list, NULL, 0);
continue;
}
if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
/* arg = value */
if (opts == NULL) {
- fprintf(stderr, "no group defined\n");
- return -1;
+ error_report("no group defined");
+ goto out;
}
if (qemu_opt_set(opts, arg, value) != 0) {
- fprintf(stderr, "failed to set \"%s\" for %s\n",
- arg, group);
- return -1;
+ error_report("failed to set \"%s\" for %s", arg, group);
+ goto out;
}
continue;
}
- fprintf(stderr, "parse error: %s\n", line);
- return -1;
+ error_report("parse error");
+ goto out;
}
- return 0;
+ res = 0;
+out:
+ loc_pop(&loc);
+ return res;
}
diff --git a/qemu-config.h b/qemu-config.h
index b335c42..c507687 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -16,6 +16,6 @@ int qemu_global_option(const char *str);
void qemu_add_globals(void);
void qemu_config_write(FILE *fp);
-int qemu_config_parse(FILE *fp);
+int qemu_config_parse(FILE *fp, const char *fname);
#endif /* QEMU_CONFIG_H */
diff --git a/qemu-error.c b/qemu-error.c
index 214e448..23176e1 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -113,6 +113,19 @@ void loc_set_none(void)
cur_loc->kind = LOC_NONE;
}
+/*
+ * Change the current location to file FNAME, line LNO.
+ */
+void loc_set_file(const char *fname, int lno)
+{
+ assert (fname || cur_loc->kind == LOC_FILE);
+ cur_loc->kind = LOC_FILE;
+ cur_loc->num = lno;
+ if (fname) {
+ cur_loc->ptr = fname;
+ }
+}
+
static const char *progname;
/*
@@ -136,6 +149,13 @@ void error_print_loc(void)
sep = " ";
}
switch (cur_loc->kind) {
+ case LOC_FILE:
+ error_printf("%s:", (const char *)cur_loc->ptr);
+ if (cur_loc->num) {
+ error_printf("%d:", cur_loc->num);
+ }
+ error_printf(" ");
+ break;
default:
error_printf(sep);
}
diff --git a/qemu-error.h b/qemu-error.h
index 204dfb6..8f2a140 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -15,7 +15,7 @@
typedef struct Location {
/* all members are private to qemu-error.c */
- enum { LOC_NONE } kind;
+ enum { LOC_NONE, LOC_FILE } kind;
int num;
const void *ptr;
struct Location *prev;
@@ -27,6 +27,7 @@ Location *loc_pop(Location *loc);
Location *loc_save(Location *loc);
void loc_restore(Location *loc);
void loc_set_none(void);
+void loc_set_file(const char *fname, int lno);
void error_vprintf(const char *fmt, va_list ap);
void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
diff --git a/vl.c b/vl.c
index 2ba8eda..397b018 100644
--- a/vl.c
+++ b/vl.c
@@ -4901,18 +4901,22 @@ int main(int argc, char **argv, char **envp)
}
if (defconfig) {
+ const char *fname;
FILE *fp;
- fp = fopen(CONFIG_QEMU_CONFDIR "/qemu.conf", "r");
+
+ fname = CONFIG_QEMU_CONFDIR "/qemu.conf";
+ fp = fopen(fname, "r");
if (fp) {
- if (qemu_config_parse(fp) != 0) {
+ if (qemu_config_parse(fp, fname) != 0) {
exit(1);
}
fclose(fp);
}
- fp = fopen(CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf", "r");
+ fname = CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf";
+ fp = fopen(fname, "r");
if (fp) {
- if (qemu_config_parse(fp) != 0) {
+ if (qemu_config_parse(fp, fname) != 0) {
exit(1);
}
fclose(fp);
@@ -5585,7 +5589,7 @@ int main(int argc, char **argv, char **envp)
fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
exit(1);
}
- if (qemu_config_parse(fp) != 0) {
+ if (qemu_config_parse(fp, optarg) != 0) {
exit(1);
}
fclose(fp);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 21/50] QemuOpts: Fix qemu_config_parse() to catch file read errors
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (19 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 20/50] error: Track locations in configuration files Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 22/50] error: Track locations on command line Markus Armbruster
` (29 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-config.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 2de97cd..ad130fb 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -475,6 +475,10 @@ int qemu_config_parse(FILE *fp, const char *fname)
error_report("parse error");
goto out;
}
+ if (ferror(fp)) {
+ error_report("error reading file");
+ goto out;
+ }
res = 0;
out:
loc_pop(&loc);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 22/50] error: Track locations on command line
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (20 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 21/50] QemuOpts: Fix qemu_config_parse() to catch file read errors Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 23/50] qdev: Fix -device and device_add to handle unsuitable bus gracefully Markus Armbruster
` (28 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
New LOC_CMDLINE. Use it for tracking option with argument in
lookup_opt(). We now report errors like this
qemu: -device smbus-eeprom: Did not find I2C bus for smbus-eeprom
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-error.c | 20 ++++++++++++++++++++
qemu-error.h | 3 ++-
vl.c | 9 +++++----
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index 23176e1..5be6bea 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -114,6 +114,16 @@ void loc_set_none(void)
}
/*
+ * Change the current location to argument ARGV[IDX..IDX+CNT-1].
+ */
+void loc_set_cmdline(char **argv, int idx, int cnt)
+{
+ cur_loc->kind = LOC_CMDLINE;
+ cur_loc->num = cnt;
+ cur_loc->ptr = argv + idx;
+}
+
+/*
* Change the current location to file FNAME, line LNO.
*/
void loc_set_file(const char *fname, int lno)
@@ -143,12 +153,22 @@ void error_set_progname(const char *argv0)
void error_print_loc(void)
{
const char *sep = "";
+ int i;
+ const char *const *argp;
if (!cur_mon) {
fprintf(stderr, "%s:", progname);
sep = " ";
}
switch (cur_loc->kind) {
+ case LOC_CMDLINE:
+ argp = cur_loc->ptr;
+ for (i = 0; i < cur_loc->num; i++) {
+ error_printf("%s%s", sep, argp[i]);
+ sep = " ";
+ }
+ error_printf(": ");
+ break;
case LOC_FILE:
error_printf("%s:", (const char *)cur_loc->ptr);
if (cur_loc->num) {
diff --git a/qemu-error.h b/qemu-error.h
index 8f2a140..88e0b70 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -15,7 +15,7 @@
typedef struct Location {
/* all members are private to qemu-error.c */
- enum { LOC_NONE, LOC_FILE } kind;
+ enum { LOC_NONE, LOC_CMDLINE, LOC_FILE } kind;
int num;
const void *ptr;
struct Location *prev;
@@ -27,6 +27,7 @@ Location *loc_pop(Location *loc);
Location *loc_save(Location *loc);
void loc_restore(Location *loc);
void loc_set_none(void);
+void loc_set_cmdline(char **argv, int idx, int cnt);
void loc_set_file(const char *fname, int lno);
void error_vprintf(const char *fmt, va_list ap);
diff --git a/vl.c b/vl.c
index 397b018..1b58759 100644
--- a/vl.c
+++ b/vl.c
@@ -4756,6 +4756,7 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
char *r = argv[optind];
const char *optarg;
+ loc_set_cmdline(argv, optind, 1);
optind++;
/* Treat --foo the same as -foo. */
if (r[1] == '-')
@@ -4763,8 +4764,7 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
popt = qemu_options;
for(;;) {
if (!popt->name) {
- fprintf(stderr, "%s: invalid option -- '%s'\n",
- argv[0], r);
+ error_report("invalid option");
exit(1);
}
if (!strcmp(popt->name, r + 1))
@@ -4773,11 +4773,11 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
}
if (popt->flags & HAS_ARG) {
if (optind >= argc) {
- fprintf(stderr, "%s: option '%s' requires an argument\n",
- argv[0], r);
+ error_report("requires an argument");
exit(1);
}
optarg = argv[optind++];
+ loc_set_cmdline(argv, optind - 2, 2);
} else {
optarg = NULL;
}
@@ -5614,6 +5614,7 @@ int main(int argc, char **argv, char **envp)
}
}
}
+ loc_set_none();
/* If no data_dir is specified then try to find it relative to the
executable path. */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 23/50] qdev: Fix -device and device_add to handle unsuitable bus gracefully
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (21 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 22/50] error: Track locations on command line Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 24/50] qdev: Factor qdev_create_from_info() out of qdev_create() Markus Armbruster
` (27 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
"device_add isa-serial,bus=pci.0" kills QEMU. Not good.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 25e1703..80e5c36 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -204,6 +204,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
path = qemu_opt_get(opts, "bus");
if (path != NULL) {
bus = qbus_find(path);
+ if (bus && bus->info != info->bus_info) {
+ error_report("Device '%s' can't go on a %s bus",
+ driver, bus->info->name);
+ return NULL;
+ }
} else {
bus = qbus_find_recursive(main_system_bus, NULL, info->bus_info);
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 24/50] qdev: Factor qdev_create_from_info() out of qdev_create()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (22 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 23/50] qdev: Fix -device and device_add to handle unsuitable bus gracefully Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 25/50] qdev: Hide "no_user" devices from users Markus Armbruster
` (26 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
To make it obvious that -device and device_add can't die in
hw_error().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 37 ++++++++++++++++++++++---------------
1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 80e5c36..7bc711b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -78,13 +78,32 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
return NULL;
}
+static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
+{
+ DeviceState *dev;
+
+ assert(bus->info == info->bus_info);
+ dev = qemu_mallocz(info->size);
+ dev->info = info;
+ dev->parent_bus = bus;
+ qdev_prop_set_defaults(dev, dev->info->props);
+ qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
+ qdev_prop_set_globals(dev);
+ QLIST_INSERT_HEAD(&bus->children, dev, sibling);
+ if (qdev_hotplug) {
+ assert(bus->allow_hotplug);
+ dev->hotplugged = 1;
+ }
+ dev->state = DEV_STATE_CREATED;
+ return dev;
+}
+
/* Create a new device. This only initializes the device state structure
and allows properties to be set. qdev_init should be called to
initialize the actual device emulation. */
DeviceState *qdev_create(BusState *bus, const char *name)
{
DeviceInfo *info;
- DeviceState *dev;
if (!bus) {
if (!main_system_bus) {
@@ -98,19 +117,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name);
}
- dev = qemu_mallocz(info->size);
- dev->info = info;
- dev->parent_bus = bus;
- qdev_prop_set_defaults(dev, dev->info->props);
- qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
- qdev_prop_set_globals(dev);
- QLIST_INSERT_HEAD(&bus->children, dev, sibling);
- if (qdev_hotplug) {
- assert(bus->allow_hotplug);
- dev->hotplugged = 1;
- }
- dev->state = DEV_STATE_CREATED;
- return dev;
+ return qdev_create_from_info(bus, info);
}
static void qdev_print_devinfo(DeviceInfo *info)
@@ -224,7 +231,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
}
/* create device, set properties */
- qdev = qdev_create(bus, driver);
+ qdev = qdev_create_from_info(bus, info);
id = qemu_opts_id(opts);
if (id) {
qdev->id = id;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 25/50] qdev: Hide "no_user" devices from users
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (23 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 24/50] qdev: Factor qdev_create_from_info() out of qdev_create() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 26/50] qdev: Hide "ptr" properties " Markus Armbruster
` (25 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Users can't create them, so qdev_device_help() shouldn't list them.
Fix that.
Also make qdev_device_add() pretend they don't exist. Before, it
rejected them with a "can't be added via command line" message, which
wasn't quite right for monitor command device_add.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 7bc711b..5ebf013 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -162,6 +162,9 @@ int qdev_device_help(QemuOpts *opts)
driver = qemu_opt_get(opts, "driver");
if (driver && !strcmp(driver, "?")) {
for (info = device_info_list; info != NULL; info = info->next) {
+ if (info->no_user) {
+ continue; /* not available, don't show */
+ }
qdev_print_devinfo(info);
}
return 1;
@@ -197,15 +200,10 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* find driver */
info = qdev_find_info(NULL, driver);
- if (!info) {
+ if (!info || info->no_user) {
qerror_report(QERR_DEVICE_NOT_FOUND, driver);
return NULL;
}
- if (info->no_user) {
- error_report("device \"%s\" can't be added via command line",
- info->name);
- return NULL;
- }
/* find bus */
path = qemu_opt_get(opts, "bus");
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 26/50] qdev: Hide "ptr" properties from users
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (24 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 25/50] qdev: Hide "no_user" devices from users Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 27/50] monitor: New monitor_cur_is_qmp() Markus Armbruster
` (24 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Users can't set them, so qdev_device_help() shouldn't list them. Fix
that. Also make qdev_prop_parse() hide them instead of printing a
meaningless "has no parser" error message.
Their value means nothing to users, so qdev_print_props() shouldn't
print it. Fix by removing their print method.
Their only use is dirty hacks. Document that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev-properties.c | 21 ++++++++-------------
hw/qdev.c | 15 +++++++++++++++
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 277ff9e..11f7b76 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -399,17 +399,11 @@ PropertyInfo qdev_prop_vlan = {
/* --- pointer --- */
-static int print_ptr(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
- void **ptr = qdev_get_prop_ptr(dev, prop);
- return snprintf(dest, len, "<%p>", *ptr);
-}
-
+/* Not a proper property, just for dirty hacks. TODO Remove it! */
PropertyInfo qdev_prop_ptr = {
.name = "ptr",
.type = PROP_TYPE_PTR,
.size = sizeof(void*),
- .print = print_ptr,
};
/* --- mac address --- */
@@ -543,16 +537,17 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
Property *prop;
prop = qdev_prop_find(dev, name);
- if (!prop) {
+ /*
+ * TODO Properties without a parse method are just for dirty
+ * hacks. qdev_prop_ptr is the only such PropertyInfo. It's
+ * marked for removal. The test !prop->info->parse should be
+ * removed along with it.
+ */
+ if (!prop || !prop->info->parse) {
fprintf(stderr, "property \"%s.%s\" not found\n",
dev->info->name, name);
return -1;
}
- if (!prop->info->parse) {
- fprintf(stderr, "property \"%s.%s\" has no parser\n",
- dev->info->name, name);
- return -1;
- }
if (prop->info->parse(dev, prop, value) != 0) {
fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
dev->info->name, name, value);
diff --git a/hw/qdev.c b/hw/qdev.c
index 5ebf013..1a6f257 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -180,6 +180,15 @@ int qdev_device_help(QemuOpts *opts)
}
for (prop = info->props; prop && prop->name; prop++) {
+ /*
+ * TODO Properties without a parser are just for dirty hacks.
+ * qdev_prop_ptr is the only such PropertyInfo. It's marked
+ * for removal. This conditional should be removed along with
+ * it.
+ */
+ if (!prop->info->parse) {
+ continue; /* no way to set it, don't show */
+ }
error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
}
return 1;
@@ -682,6 +691,12 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
if (!props)
return;
while (props->name) {
+ /*
+ * TODO Properties without a print method are just for dirty
+ * hacks. qdev_prop_ptr is the only such PropertyInfo. It's
+ * marked for removal. The test props->info->print should be
+ * removed along with it.
+ */
if (props->info->print) {
props->info->print(dev, props, buf, sizeof(buf));
qdev_printf("%s-prop: %s = %s\n", prefix, props->name, buf);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 27/50] monitor: New monitor_cur_is_qmp()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (25 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 26/50] qdev: Hide "ptr" properties " Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 28/50] error: Let converted handlers print in human monitor Markus Armbruster
` (23 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor.c | 6 ++++++
monitor.h | 2 ++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/monitor.c b/monitor.c
index 55d78d5..3580d37 100644
--- a/monitor.c
+++ b/monitor.c
@@ -194,6 +194,12 @@ static inline int monitor_ctrl_mode(const Monitor *mon)
return (mon->flags & MONITOR_USE_CONTROL);
}
+/* Return non-zero iff we have a current monitor, and it is in QMP mode. */
+int monitor_cur_is_qmp(void)
+{
+ return cur_mon && monitor_ctrl_mode(cur_mon);
+}
+
static void monitor_read_command(Monitor *mon, int show_prompt)
{
if (!mon->rs)
diff --git a/monitor.h b/monitor.h
index 1222ff0..1f3df9f 100644
--- a/monitor.h
+++ b/monitor.h
@@ -30,6 +30,8 @@ typedef enum MonitorEvent {
QEVENT_MAX,
} MonitorEvent;
+int monitor_cur_is_qmp(void);
+
void monitor_protocol_event(MonitorEvent event, QObject *data);
void monitor_init(CharDriverState *chr, int flags);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (26 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 27/50] monitor: New monitor_cur_is_qmp() Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 20:50 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 15:56 ` [Qemu-devel] [PATCH 29/50] error: Polish human-readable error descriptions Markus Armbruster
` (22 subsequent siblings)
50 siblings, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
While fully converted handlers are not supposed to print anything when
running in a QMP monitor, they are free to print in a human monitor.
For instance, device_add (not yet converted) prints help, and will
continue to do so after conversion.
Moreover, utility functions converted to QError should remain usable
from unconverted handlers.
Two problems:
* handler_audit() complains when a converted handler prints. Limit
that to QMP monitors.
* With QMP, handlers need to pass the error object by way of
monitor_set_error(). However, we do that both for QMP and for the
human monitor. The human monitor prints the error object after the
handler returns. If the handler prints anything else, that output
"overtakes" the error message.
Limit use of monitor_set_error() to QMP monitors. Update
handler_audit() accordingly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor.c | 80 ++++++++++++++++++++++++++-------------------------------
qemu-error.c | 2 +-
2 files changed, 38 insertions(+), 44 deletions(-)
diff --git a/monitor.c b/monitor.c
index 3580d37..dfeb9db 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3873,13 +3873,6 @@ void monitor_set_error(Monitor *mon, QError *qerror)
}
}
-static void monitor_print_error(Monitor *mon)
-{
- qerror_print(mon->error);
- QDECREF(mon->error);
- mon->error = NULL;
-}
-
static int is_async_return(const QObject *data)
{
if (data && qobject_type(data) == QTYPE_QDICT) {
@@ -3891,45 +3884,49 @@ static int is_async_return(const QObject *data)
static void handler_audit(Monitor *mon, const mon_cmd_t *cmd, int ret)
{
- if (ret && !monitor_has_error(mon)) {
- /*
- * If it returns failure, it must have passed on error.
- *
- * Action: Report an internal error to the client if in QMP.
- */
- if (monitor_ctrl_mode(mon)) {
+ if (monitor_ctrl_mode(mon)) {
+ if (ret && !monitor_has_error(mon)) {
+ /*
+ * If it returns failure, it must have passed on error.
+ *
+ * Action: Report an internal error to the client if in QMP.
+ */
qerror_report(QERR_UNDEFINED_ERROR);
+ MON_DEBUG("command '%s' returned failure but did not pass an error\n",
+ cmd->name);
}
- MON_DEBUG("command '%s' returned failure but did not pass an error\n",
- cmd->name);
- }
#ifdef CONFIG_DEBUG_MONITOR
- if (!ret && monitor_has_error(mon)) {
- /*
- * If it returns success, it must not have passed an error.
- *
- * Action: Report the passed error to the client.
- */
- MON_DEBUG("command '%s' returned success but passed an error\n",
- cmd->name);
- }
+ if (!ret && monitor_has_error(mon)) {
+ /*
+ * If it returns success, it must not have passed an error.
+ *
+ * Action: Report the passed error to the client.
+ */
+ MON_DEBUG("command '%s' returned success but passed an error\n",
+ cmd->name);
+ }
- if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
- /*
- * Handlers should not call Monitor print functions.
- *
- * Action: Ignore them in QMP.
- *
- * (XXX: we don't check any 'info' or 'query' command here
- * because the user print function _is_ called by do_info(), hence
- * we will trigger this check. This problem will go away when we
- * make 'query' commands real and kill do_info())
- */
- MON_DEBUG("command '%s' called print functions %d time(s)\n",
- cmd->name, mon_print_count_get(mon));
- }
+ if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
+ /*
+ * Handlers should not call Monitor print functions.
+ *
+ * Action: Ignore them in QMP.
+ *
+ * (XXX: we don't check any 'info' or 'query' command here
+ * because the user print function _is_ called by do_info(), hence
+ * we will trigger this check. This problem will go away when we
+ * make 'query' commands real and kill do_info())
+ */
+ MON_DEBUG("command '%s' called print functions %d time(s)\n",
+ cmd->name, mon_print_count_get(mon));
+ }
#endif
+ } else {
+ assert(!monitor_has_error(mon));
+ QDECREF(mon->error);
+ mon->error = NULL;
+ }
}
static void monitor_call_handler(Monitor *mon, const mon_cmd_t *cmd,
@@ -3982,9 +3979,6 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
cmd->mhandler.cmd(mon, qdict);
}
- if (monitor_has_error(mon))
- monitor_print_error(mon);
-
out:
QDECREF(qdict);
}
diff --git a/qemu-error.c b/qemu-error.c
index 5be6bea..a8c178b 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -207,7 +207,7 @@ void qerror_report_internal(const char *file, int linenr, const char *func,
qerror = qerror_from_info(file, linenr, func, fmt, &va);
va_end(va);
- if (cur_mon) {
+ if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerror);
} else {
qerror_print(qerror);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 29/50] error: Polish human-readable error descriptions
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (27 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 28/50] error: Let converted handlers print in human monitor Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 30/50] error: New QERR_PROPERTY_NOT_FOUND Markus Armbruster
` (21 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Also put error definitions in alphabetical order
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 28 ++++++++++++++--------------
qerror.h | 14 +++++++-------
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/qerror.c b/qerror.c
index b400480..41ff946 100644
--- a/qerror.c
+++ b/qerror.c
@@ -46,43 +46,39 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_DEVICE_ENCRYPTED,
- .desc = "The %(device) is encrypted",
+ .desc = "Device '%(device)' is encrypted",
},
{
.error_fmt = QERR_DEVICE_LOCKED,
- .desc = "Device %(device) is locked",
+ .desc = "Device '%(device)' is locked",
},
{
.error_fmt = QERR_DEVICE_NOT_ACTIVE,
- .desc = "The %(device) device has not been activated by the guest",
+ .desc = "Device '%(device)' has not been activated by the guest",
},
{
.error_fmt = QERR_DEVICE_NOT_FOUND,
- .desc = "The %(device) device has not been found",
+ .desc = "Device '%(device)' not found",
},
{
.error_fmt = QERR_DEVICE_NOT_REMOVABLE,
- .desc = "Device %(device) is not removable",
+ .desc = "Device '%(device)' is not removable",
},
{
.error_fmt = QERR_FD_NOT_FOUND,
- .desc = "Failed to find file descriptor named %(name)",
+ .desc = "File descriptor named '%(name)' not found",
},
{
.error_fmt = QERR_FD_NOT_SUPPLIED,
.desc = "No file descriptor supplied via SCM_RIGHTS",
},
{
- .error_fmt = QERR_OPEN_FILE_FAILED,
- .desc = "Could not open '%(filename)'",
- },
- {
.error_fmt = QERR_INVALID_BLOCK_FORMAT,
- .desc = "Invalid block format %(name)",
+ .desc = "Invalid block format '%(name)'",
},
{
.error_fmt = QERR_INVALID_PARAMETER,
- .desc = "Invalid parameter %(name)",
+ .desc = "Invalid parameter '%(name)'",
},
{
.error_fmt = QERR_INVALID_PARAMETER_TYPE,
@@ -90,7 +86,7 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_INVALID_PASSWORD,
- .desc = "The entered password is invalid",
+ .desc = "Password incorrect",
},
{
.error_fmt = QERR_JSON_PARSING,
@@ -102,7 +98,11 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_MISSING_PARAMETER,
- .desc = "Parameter %(name) is missing",
+ .desc = "Parameter '%(name)' is missing",
+ },
+ {
+ .error_fmt = QERR_OPEN_FILE_FAILED,
+ .desc = "Could not open '%(filename)'",
},
{
.error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
diff --git a/qerror.h b/qerror.h
index f25be1b..8060fe0 100644
--- a/qerror.h
+++ b/qerror.h
@@ -48,7 +48,7 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_ENCRYPTED \
"{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }"
-#define QERR_DEVICE_LOCKED \
+#define QERR_DEVICE_LOCKED \
"{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
#define QERR_DEVICE_NOT_ACTIVE \
@@ -66,9 +66,6 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_FD_NOT_SUPPLIED \
"{ 'class': 'FdNotSupplied', 'data': {} }"
-#define QERR_OPEN_FILE_FAILED \
- "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
-
#define QERR_INVALID_BLOCK_FORMAT \
"{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
@@ -90,18 +87,21 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_MISSING_PARAMETER \
"{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
+#define QERR_OPEN_FILE_FAILED \
+ "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
+
#define QERR_QMP_BAD_INPUT_OBJECT \
"{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
#define QERR_SET_PASSWD_FAILED \
"{ 'class': 'SetPasswdFailed', 'data': {} }"
-#define QERR_UNDEFINED_ERROR \
- "{ 'class': 'UndefinedError', 'data': {} }"
-
#define QERR_TOO_MANY_FILES \
"{ 'class': 'TooManyFiles', 'data': {} }"
+#define QERR_UNDEFINED_ERROR \
+ "{ 'class': 'UndefinedError', 'data': {} }"
+
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 30/50] error: New QERR_PROPERTY_NOT_FOUND
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (28 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 29/50] error: Polish human-readable error descriptions Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 31/50] error: New QERR_PROPERTY_VALUE_BAD Markus Armbruster
` (20 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 41ff946..4de1039 100644
--- a/qerror.c
+++ b/qerror.c
@@ -105,6 +105,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not open '%(filename)'",
},
{
+ .error_fmt = QERR_PROPERTY_NOT_FOUND,
+ .desc = "Property '%(device).%(property)' not found",
+ },
+ {
.error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
.desc = "Bad QMP input object",
},
diff --git a/qerror.h b/qerror.h
index 8060fe0..e36336e 100644
--- a/qerror.h
+++ b/qerror.h
@@ -90,6 +90,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_OPEN_FILE_FAILED \
"{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
+#define QERR_PROPERTY_NOT_FOUND \
+ "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
+
#define QERR_QMP_BAD_INPUT_OBJECT \
"{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 31/50] error: New QERR_PROPERTY_VALUE_BAD
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (29 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 30/50] error: New QERR_PROPERTY_NOT_FOUND Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 32/50] qdev: convert setting device properties to QError Markus Armbruster
` (19 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 4de1039..62bb00f 100644
--- a/qerror.c
+++ b/qerror.c
@@ -109,6 +109,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Property '%(device).%(property)' not found",
},
{
+ .error_fmt = QERR_PROPERTY_VALUE_BAD,
+ .desc = "Property '%(device).%(property)' doesn't take value '%(value)'",
+ },
+ {
.error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
.desc = "Bad QMP input object",
},
diff --git a/qerror.h b/qerror.h
index e36336e..d5f0e9d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -93,6 +93,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_PROPERTY_NOT_FOUND \
"{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
+#define QERR_PROPERTY_VALUE_BAD \
+ "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
+
#define QERR_QMP_BAD_INPUT_OBJECT \
"{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 32/50] qdev: convert setting device properties to QError
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (30 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 31/50] error: New QERR_PROPERTY_VALUE_BAD Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 33/50] qdev: Relax parsing of bus option Markus Armbruster
` (18 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev-properties.c | 6 ++----
hw/qdev.c | 2 --
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 11f7b76..780a9d8 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -544,13 +544,11 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
* removed along with it.
*/
if (!prop || !prop->info->parse) {
- fprintf(stderr, "property \"%s.%s\" not found\n",
- dev->info->name, name);
+ qerror_report(QERR_PROPERTY_NOT_FOUND, dev->info->name, name);
return -1;
}
if (prop->info->parse(dev, prop, value) != 0) {
- fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
- dev->info->name, name, value);
+ qerror_report(QERR_PROPERTY_VALUE_BAD, dev->info->name, name, value);
return -1;
}
return 0;
diff --git a/hw/qdev.c b/hw/qdev.c
index 1a6f257..1de4ff6 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -146,8 +146,6 @@ static int set_property(const char *name, const char *value, void *opaque)
return 0;
if (qdev_prop_parse(dev, name, value) == -1) {
- error_report("can't set property \"%s\" to \"%s\" for \"%s\"",
- name, value, dev->info->name);
return -1;
}
return 0;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 33/50] qdev: Relax parsing of bus option
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (31 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 32/50] qdev: convert setting device properties to QError Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 34/50] error: New QERR_BUS_NOT_FOUND Markus Armbruster
` (17 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Treat multiple successive slashes as a one slash. Ignore trailing
slashes. This is how POSIX pathnames work.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 26 +++++++++++++++++---------
1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 1de4ff6..40b07fb 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -555,8 +555,8 @@ static BusState *qbus_find(const char *path)
pos = 0;
} else {
if (sscanf(path, "%127[^/]%n", elem, &len) != 1) {
- error_report("path parse error (\"%s\")", path);
- return NULL;
+ assert(!path[0]);
+ elem[0] = len = 0;
}
bus = qbus_find_recursive(main_system_bus, elem, NULL);
if (!bus) {
@@ -567,15 +567,18 @@ static BusState *qbus_find(const char *path)
}
for (;;) {
+ assert(path[pos] == '/' || !path[pos]);
+ while (path[pos] == '/') {
+ pos++;
+ }
if (path[pos] == '\0') {
- /* we are done */
return bus;
}
/* find device */
- if (sscanf(path+pos, "/%127[^/]%n", elem, &len) != 1) {
- error_report("path parse error (\"%s\" pos %d)", path, pos);
- return NULL;
+ if (sscanf(path+pos, "%127[^/]%n", elem, &len) != 1) {
+ assert(0);
+ elem[0] = len = 0;
}
pos += len;
dev = qbus_find_dev(bus, elem);
@@ -584,6 +587,11 @@ static BusState *qbus_find(const char *path)
qbus_list_dev(bus);
return NULL;
}
+
+ assert(path[pos] == '/' || !path[pos]);
+ while (path[pos] == '/') {
+ pos++;
+ }
if (path[pos] == '\0') {
/* last specified element is a device. If it has exactly
* one child bus accept it nevertheless */
@@ -601,9 +609,9 @@ static BusState *qbus_find(const char *path)
}
/* find bus */
- if (sscanf(path+pos, "/%127[^/]%n", elem, &len) != 1) {
- error_report("path parse error (\"%s\" pos %d)", path, pos);
- return NULL;
+ if (sscanf(path+pos, "%127[^/]%n", elem, &len) != 1) {
+ assert(0);
+ elem[0] = len = 0;
}
pos += len;
bus = qbus_find_bus(dev, elem);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 34/50] error: New QERR_BUS_NOT_FOUND
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (32 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 33/50] qdev: Relax parsing of bus option Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 35/50] error: New QERR_DEVICE_MULTIPLE_BUSSES Markus Armbruster
` (16 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 62bb00f..e7a485e 100644
--- a/qerror.c
+++ b/qerror.c
@@ -41,6 +41,10 @@ static const QType qerror_type = {
*/
static const QErrorStringTable qerror_table[] = {
{
+ .error_fmt = QERR_BUS_NOT_FOUND,
+ .desc = "Bus '%(bus)' not found",
+ },
+ {
.error_fmt = QERR_COMMAND_NOT_FOUND,
.desc = "The command %(name) has not been found",
},
diff --git a/qerror.h b/qerror.h
index d5f0e9d..2f06dc7 100644
--- a/qerror.h
+++ b/qerror.h
@@ -42,6 +42,9 @@ QError *qobject_to_qerror(const QObject *obj);
/*
* QError class list
*/
+#define QERR_BUS_NOT_FOUND \
+ "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+
#define QERR_COMMAND_NOT_FOUND \
"{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 35/50] error: New QERR_DEVICE_MULTIPLE_BUSSES
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (33 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 34/50] error: New QERR_BUS_NOT_FOUND Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 36/50] error: New QERR_DEVICE_NO_BUS Markus Armbruster
` (15 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index e7a485e..b79b4fa 100644
--- a/qerror.c
+++ b/qerror.c
@@ -57,6 +57,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' is locked",
},
{
+ .error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
+ .desc = "Device '%(device)' has multiple child busses",
+ },
+ {
.error_fmt = QERR_DEVICE_NOT_ACTIVE,
.desc = "Device '%(device)' has not been activated by the guest",
},
diff --git a/qerror.h b/qerror.h
index 2f06dc7..2b31a8d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -54,6 +54,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_LOCKED \
"{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
+#define QERR_DEVICE_MULTIPLE_BUSSES \
+ "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_NOT_ACTIVE \
"{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 36/50] error: New QERR_DEVICE_NO_BUS
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (34 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 35/50] error: New QERR_DEVICE_MULTIPLE_BUSSES Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 37/50] qdev: Convert qbus_find() to QError Markus Armbruster
` (14 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index b79b4fa..733723e 100644
--- a/qerror.c
+++ b/qerror.c
@@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' is not removable",
},
{
+ .error_fmt = QERR_DEVICE_NO_BUS,
+ .desc = "Device '%(device)' has no child bus",
+ },
+ {
.error_fmt = QERR_FD_NOT_FOUND,
.desc = "File descriptor named '%(name)' not found",
},
diff --git a/qerror.h b/qerror.h
index 2b31a8d..e65f1f2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -66,6 +66,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_NOT_REMOVABLE \
"{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
+#define QERR_DEVICE_NO_BUS \
+ "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+
#define QERR_FD_NOT_FOUND \
"{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 37/50] qdev: Convert qbus_find() to QError
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (35 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 36/50] error: New QERR_DEVICE_NO_BUS Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 38/50] error: New error_printf_unless_qmp() Markus Armbruster
` (13 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 25 ++++++++++++++-----------
1 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 40b07fb..bbb1e44 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -216,18 +216,21 @@ DeviceState *qdev_device_add(QemuOpts *opts)
path = qemu_opt_get(opts, "bus");
if (path != NULL) {
bus = qbus_find(path);
- if (bus && bus->info != info->bus_info) {
+ if (!bus) {
+ return NULL;
+ }
+ if (bus->info != info->bus_info) {
error_report("Device '%s' can't go on a %s bus",
driver, bus->info->name);
return NULL;
}
} else {
bus = qbus_find_recursive(main_system_bus, NULL, info->bus_info);
- }
- if (!bus) {
- error_report("Did not find %s bus for %s",
- path ? path : info->bus_info->name, info->name);
- return NULL;
+ if (!bus) {
+ error_report("Did not find %s bus for %s",
+ info->bus_info->name, info->name);
+ return NULL;
+ }
}
if (qdev_hotplug && !bus->allow_hotplug) {
error_report("Bus %s does not support hotplugging",
@@ -560,7 +563,7 @@ static BusState *qbus_find(const char *path)
}
bus = qbus_find_recursive(main_system_bus, elem, NULL);
if (!bus) {
- error_report("bus \"%s\" not found", elem);
+ qerror_report(QERR_BUS_NOT_FOUND, elem);
return NULL;
}
pos = len;
@@ -583,7 +586,7 @@ static BusState *qbus_find(const char *path)
pos += len;
dev = qbus_find_dev(bus, elem);
if (!dev) {
- error_report("device \"%s\" not found", elem);
+ qerror_report(QERR_DEVICE_NOT_FOUND, elem);
qbus_list_dev(bus);
return NULL;
}
@@ -597,12 +600,12 @@ static BusState *qbus_find(const char *path)
* one child bus accept it nevertheless */
switch (dev->num_child_bus) {
case 0:
- error_report("device has no child bus (%s)", path);
+ qerror_report(QERR_DEVICE_NO_BUS, elem);
return NULL;
case 1:
return QLIST_FIRST(&dev->child_bus);
default:
- error_report("device has multiple child busses (%s)", path);
+ qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
qbus_list_bus(dev);
return NULL;
}
@@ -616,7 +619,7 @@ static BusState *qbus_find(const char *path)
pos += len;
bus = qbus_find_bus(dev, elem);
if (!bus) {
- error_report("child bus \"%s\" not found", elem);
+ qerror_report(QERR_BUS_NOT_FOUND, elem);
qbus_list_bus(dev);
return NULL;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 38/50] error: New error_printf_unless_qmp()
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (36 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 37/50] qdev: Convert qbus_find() to QError Markus Armbruster
@ 2010-03-04 15:56 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 39/50] error: New QERR_BAD_BUS_FOR_DEVICE Markus Armbruster
` (12 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-error.c | 11 +++++++++++
qemu-error.h | 2 ++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index a8c178b..5d5fe37 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -41,6 +41,17 @@ void error_printf(const char *fmt, ...)
va_end(ap);
}
+void error_printf_unless_qmp(const char *fmt, ...)
+{
+ va_list ap;
+
+ if (!monitor_cur_is_qmp()) {
+ va_start(ap, fmt);
+ error_vprintf(fmt, ap);
+ va_end(ap);
+ }
+}
+
static Location std_loc = {
.kind = LOC_NONE
};
diff --git a/qemu-error.h b/qemu-error.h
index 88e0b70..e63c6ab 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -32,6 +32,8 @@ void loc_set_file(const char *fname, int lno);
void error_vprintf(const char *fmt, va_list ap);
void error_printf(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
+void error_printf_unless_qmp(const char *fmt, ...)
+ __attribute__ ((format(printf, 1, 2)));
void error_print_loc(void);
void error_set_progname(const char *argv0);
void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 39/50] error: New QERR_BAD_BUS_FOR_DEVICE
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (37 preceding siblings ...)
2010-03-04 15:56 ` [Qemu-devel] [PATCH 38/50] error: New error_printf_unless_qmp() Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 40/50] error: New QERR_BUS_NO_HOTPLUG Markus Armbruster
` (11 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 733723e..68ef78c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -41,6 +41,10 @@ static const QType qerror_type = {
*/
static const QErrorStringTable qerror_table[] = {
{
+ .error_fmt = QERR_BAD_BUS_FOR_DEVICE,
+ .desc = "Device '%(device)' can't go on a %(bad_bus_type) bus",
+ },
+ {
.error_fmt = QERR_BUS_NOT_FOUND,
.desc = "Bus '%(bus)' not found",
},
diff --git a/qerror.h b/qerror.h
index e65f1f2..8433d84 100644
--- a/qerror.h
+++ b/qerror.h
@@ -42,6 +42,9 @@ QError *qobject_to_qerror(const QObject *obj);
/*
* QError class list
*/
+#define QERR_BAD_BUS_FOR_DEVICE \
+ "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
+
#define QERR_BUS_NOT_FOUND \
"{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 40/50] error: New QERR_BUS_NO_HOTPLUG
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (38 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 39/50] error: New QERR_BAD_BUS_FOR_DEVICE Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 41/50] error: New QERR_DEVICE_INIT_FAILED Markus Armbruster
` (10 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 68ef78c..b61cc33 100644
--- a/qerror.c
+++ b/qerror.c
@@ -49,6 +49,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Bus '%(bus)' not found",
},
{
+ .error_fmt = QERR_BUS_NO_HOTPLUG,
+ .desc = "Bus '%(bus)' does not support hotplugging",
+ },
+ {
.error_fmt = QERR_COMMAND_NOT_FOUND,
.desc = "The command %(name) has not been found",
},
diff --git a/qerror.h b/qerror.h
index 8433d84..9db909a 100644
--- a/qerror.h
+++ b/qerror.h
@@ -48,6 +48,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_BUS_NOT_FOUND \
"{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+#define QERR_BUS_NO_HOTPLUG \
+ "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+
#define QERR_COMMAND_NOT_FOUND \
"{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 41/50] error: New QERR_DEVICE_INIT_FAILED
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (39 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 40/50] error: New QERR_BUS_NO_HOTPLUG Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 42/50] error: New QERR_NO_BUS_FOR_DEVICE Markus Armbruster
` (9 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index b61cc33..107161c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -61,6 +61,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' is encrypted",
},
{
+ .error_fmt = QERR_DEVICE_INIT_FAILED,
+ .desc = "Device '%(device)' could not be initialized",
+ },
+ {
.error_fmt = QERR_DEVICE_LOCKED,
.desc = "Device '%(device)' is locked",
},
diff --git a/qerror.h b/qerror.h
index 9db909a..173b9f8 100644
--- a/qerror.h
+++ b/qerror.h
@@ -57,6 +57,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_ENCRYPTED \
"{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }"
+#define QERR_DEVICE_INIT_FAILED \
+ "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_LOCKED \
"{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 42/50] error: New QERR_NO_BUS_FOR_DEVICE
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (40 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 41/50] error: New QERR_DEVICE_INIT_FAILED Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 43/50] Revert "qdev: Use QError for 'device not found' error" Markus Armbruster
` (8 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 107161c..ece51f8 100644
--- a/qerror.c
+++ b/qerror.c
@@ -125,6 +125,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Parameter '%(name)' is missing",
},
{
+ .error_fmt = QERR_NO_BUS_FOR_DEVICE,
+ .desc = "No '%(bus)' bus found for device '%(device)'",
+ },
+ {
.error_fmt = QERR_OPEN_FILE_FAILED,
.desc = "Could not open '%(filename)'",
},
diff --git a/qerror.h b/qerror.h
index 173b9f8..b46d4df 100644
--- a/qerror.h
+++ b/qerror.h
@@ -105,6 +105,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_MISSING_PARAMETER \
"{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
+#define QERR_NO_BUS_FOR_DEVICE \
+ "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
+
#define QERR_OPEN_FILE_FAILED \
"{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 43/50] Revert "qdev: Use QError for 'device not found' error"
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (41 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 42/50] error: New QERR_NO_BUS_FOR_DEVICE Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 44/50] error: Convert do_device_add() to QError Markus Armbruster
` (7 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
This reverts commit 3ced9f7a36189aed94d8bf86f3f5087a53012455.
The next commit will convert all of qdev_device_add() to QError, and
it'll be clearer with this partial conversion reverted.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index bbb1e44..d3204d4 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,7 +29,6 @@
#include "qdev.h"
#include "sysemu.h"
#include "monitor.h"
-#include "qerror.h"
static int qdev_hotplug = 0;
@@ -208,7 +207,8 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* find driver */
info = qdev_find_info(NULL, driver);
if (!info || info->no_user) {
- qerror_report(QERR_DEVICE_NOT_FOUND, driver);
+ error_report("Device \"%s\" not found. Try -device '?' for a list.",
+ driver);
return NULL;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 44/50] error: Convert do_device_add() to QError
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (42 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 43/50] Revert "qdev: Use QError for 'device not found' error" Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 45/50] qemu-option: Functions to convert to/from QDict Markus Armbruster
` (6 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Conversion to QObject is still missing.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 19 +++++++++----------
1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index d3204d4..e3fcc75 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -200,15 +200,15 @@ DeviceState *qdev_device_add(QemuOpts *opts)
driver = qemu_opt_get(opts, "driver");
if (!driver) {
- error_report("-device: no driver specified");
+ qerror_report(QERR_MISSING_PARAMETER, "driver");
return NULL;
}
/* find driver */
info = qdev_find_info(NULL, driver);
if (!info || info->no_user) {
- error_report("Device \"%s\" not found. Try -device '?' for a list.",
- driver);
+ qerror_report(QERR_INVALID_PARAMETER, "driver");
+ error_printf_unless_qmp("Try with argument '?' for a list.\n");
return NULL;
}
@@ -220,21 +220,20 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return NULL;
}
if (bus->info != info->bus_info) {
- error_report("Device '%s' can't go on a %s bus",
- driver, bus->info->name);
+ qerror_report(QERR_BAD_BUS_FOR_DEVICE,
+ driver, bus->info->name);
return NULL;
}
} else {
bus = qbus_find_recursive(main_system_bus, NULL, info->bus_info);
if (!bus) {
- error_report("Did not find %s bus for %s",
- info->bus_info->name, info->name);
+ qerror_report(QERR_NO_BUS_FOR_DEVICE,
+ info->name, info->bus_info->name);
return NULL;
}
}
if (qdev_hotplug && !bus->allow_hotplug) {
- error_report("Bus %s does not support hotplugging",
- bus->name);
+ qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
return NULL;
}
@@ -249,7 +248,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return NULL;
}
if (qdev_init(qdev) < 0) {
- error_report("Error initializing device %s", driver);
+ qerror_report(QERR_DEVICE_INIT_FAILED, driver);
return NULL;
}
qdev->opts = opts;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 45/50] qemu-option: Functions to convert to/from QDict
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (43 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 44/50] error: Convert do_device_add() to QError Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 20:55 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 15:57 ` [Qemu-devel] [PATCH 46/50] qemu-option: Move the implied first name into QemuOptsList Markus Armbruster
` (5 subsequent siblings)
50 siblings, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
The functions are somewhat restricted. Good enough for the job at
hand. We'll extend them when we need more.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-option.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-option.h | 3 ++
2 files changed, 82 insertions(+), 0 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index ab488e4..24bb19b 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -28,6 +28,7 @@
#include "qemu-common.h"
#include "qemu-error.h"
+#include "qemu-objects.h"
#include "qemu-option.h"
/*
@@ -777,6 +778,84 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *fi
return opts;
}
+static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
+{
+ char buf[32];
+ const char *value;
+ int n;
+
+ if (!strcmp(key, "id")) {
+ return;
+ }
+
+ switch (qobject_type(obj)) {
+ case QTYPE_QSTRING:
+ value = qstring_get_str(qobject_to_qstring(obj));
+ break;
+ case QTYPE_QINT:
+ n = snprintf(buf, sizeof(buf), "%" PRId64,
+ qint_get_int(qobject_to_qint(obj)));
+ assert(n < sizeof(buf));
+ value = buf;
+ break;
+ case QTYPE_QFLOAT:
+ n = snprintf(buf, sizeof(buf), "%.17g",
+ qfloat_get_double(qobject_to_qfloat(obj)));
+ assert(n < sizeof(buf));
+ value = buf;
+ break;
+ case QTYPE_QBOOL:
+ strcpy(buf, qbool_get_int(qobject_to_qbool(obj)) ? "on" : "off");
+ value = buf;
+ break;
+ default:
+ return;
+ }
+ qemu_opt_set(opaque, key, value);
+}
+
+/*
+ * Create QemuOpts from a QDict.
+ * Use value of key "id" as ID if it exists and is a QString.
+ * Only QStrings, QInts, QFloats and QBools are copied. Entries with
+ * other types are silently ignored.
+ */
+QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict)
+{
+ QemuOpts *opts;
+
+ opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1);
+ if (opts == NULL)
+ return NULL;
+
+ qdict_iter(qdict, qemu_opts_from_qdict_1, opts);
+ return opts;
+}
+
+/*
+ * Convert from QemuOpts to QDict.
+ * The QDict values are of type QString.
+ * TODO We'll want to use types appropriate for opt->desc->type, but
+ * this is enough for now.
+ */
+QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
+{
+ QemuOpt *opt;
+ QObject *val;
+
+ if (!qdict) {
+ qdict = qdict_new();
+ }
+ if (opts->id) {
+ qdict_put(qdict, "id", qstring_from_str(opts->id));
+ }
+ QTAILQ_FOREACH(opt, &opts->head, next) {
+ val = QOBJECT(qstring_from_str(opt->str));
+ qdict_put_obj(qdict, opt->name, val);
+ }
+ return qdict;
+}
+
/* Validate parsed opts against descriptions where no
* descriptions were provided in the QemuOptsList.
*/
diff --git a/qemu-option.h b/qemu-option.h
index f3f1de7..d735386 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -28,6 +28,7 @@
#include <stdint.h>
#include "qemu-queue.h"
+#include "qdict.h"
enum QEMUOptionParType {
OPT_FLAG,
@@ -118,6 +119,8 @@ void qemu_opts_del(QemuOpts *opts);
int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc);
int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname);
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname);
+QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict);
+QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
int qemu_opts_print(QemuOpts *opts, void *dummy);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 46/50] qemu-option: Move the implied first name into QemuOptsList
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (44 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 45/50] qemu-option: Functions to convert to/from QDict Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 47/50] qemu-option: Rename find_list() to qemu_find_opts() & external linkage Markus Armbruster
` (4 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
We sometimes permit omitting the first option name, for example
-device foo is short for -device driver=foo. The name to use
("driver" in the example) is passed as argument to qemu_opts_parse().
For each QemuOptsList, we use at most one such name.
Move the name into QemuOptsList, and pass whether to permit the
abbreviation. This ensures continued consistency, and simplifies the
commit after next in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci-hotplug.c | 2 +-
hw/qdev.c | 2 +-
hw/usb-net.c | 2 +-
net.c | 4 ++--
qemu-config.c | 5 +++++
qemu-option.c | 7 ++++++-
qemu-option.h | 3 ++-
vl.c | 12 ++++++------
8 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index d608a85..eb3701b 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -54,7 +54,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
return NULL;
}
- opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", NULL);
+ opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", 0);
if (!opts) {
monitor_printf(mon, "parsing network options '%s' failed\n",
opts_str ? opts_str : "");
diff --git a/hw/qdev.c b/hw/qdev.c
index e3fcc75..1edfce8 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -767,7 +767,7 @@ void do_device_add(Monitor *mon, const QDict *qdict)
QemuOpts *opts;
opts = qemu_opts_parse(&qemu_device_opts,
- qdict_get_str(qdict, "config"), "driver");
+ qdict_get_str(qdict, "config"), 1);
if (opts) {
if (qdev_device_help(opts) || qdev_device_add(opts) == NULL) {
qemu_opts_del(opts);
diff --git a/hw/usb-net.c b/hw/usb-net.c
index 6875f11..4c17435 100644
--- a/hw/usb-net.c
+++ b/hw/usb-net.c
@@ -1478,7 +1478,7 @@ static USBDevice *usb_net_init(const char *cmdline)
QemuOpts *opts;
int idx;
- opts = qemu_opts_parse(&qemu_net_opts, cmdline, NULL);
+ opts = qemu_opts_parse(&qemu_net_opts, cmdline, 0);
if (!opts) {
return NULL;
}
diff --git a/net.c b/net.c
index 503af26..f9007cc 100644
--- a/net.c
+++ b/net.c
@@ -1156,7 +1156,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
return;
}
- opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", NULL);
+ opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", 0);
if (!opts) {
monitor_printf(mon, "parsing network options '%s' failed\n",
opts_str ? opts_str : "");
@@ -1361,7 +1361,7 @@ int net_client_parse(QemuOptsList *opts_list, const char *optarg)
}
#endif
- if (!qemu_opts_parse(opts_list, optarg, "type")) {
+ if (!qemu_opts_parse(opts_list, optarg, 1)) {
return -1;
}
diff --git a/qemu-config.c b/qemu-config.c
index ad130fb..a2e0193 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -86,6 +86,7 @@ QemuOptsList qemu_drive_opts = {
QemuOptsList qemu_chardev_opts = {
.name = "chardev",
+ .implied_opt_name = "backend",
.head = QTAILQ_HEAD_INITIALIZER(qemu_chardev_opts.head),
.desc = {
{
@@ -152,6 +153,7 @@ QemuOptsList qemu_chardev_opts = {
QemuOptsList qemu_device_opts = {
.name = "device",
+ .implied_opt_name = "driver",
.head = QTAILQ_HEAD_INITIALIZER(qemu_device_opts.head),
.desc = {
/*
@@ -165,6 +167,7 @@ QemuOptsList qemu_device_opts = {
QemuOptsList qemu_netdev_opts = {
.name = "netdev",
+ .implied_opt_name = "type",
.head = QTAILQ_HEAD_INITIALIZER(qemu_netdev_opts.head),
.desc = {
/*
@@ -177,6 +180,7 @@ QemuOptsList qemu_netdev_opts = {
QemuOptsList qemu_net_opts = {
.name = "net",
+ .implied_opt_name = "type",
.head = QTAILQ_HEAD_INITIALIZER(qemu_net_opts.head),
.desc = {
/*
@@ -227,6 +231,7 @@ QemuOptsList qemu_global_opts = {
QemuOptsList qemu_mon_opts = {
.name = "mon",
+ .implied_opt_name = "chardev",
.head = QTAILQ_HEAD_INITIALIZER(qemu_mon_opts.head),
.desc = {
{
diff --git a/qemu-option.c b/qemu-option.c
index 24bb19b..dc340b8 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -753,12 +753,17 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
return 0;
}
-QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname)
+QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
+ int permit_abbrev)
{
+ const char *firstname;
char value[1024], *id = NULL;
const char *p;
QemuOpts *opts;
+ assert(!permit_abbrev || list->implied_opt_name);
+ firstname = permit_abbrev ? list->implied_opt_name : NULL;
+
if (strncmp(params, "id=", 3) == 0) {
get_opt_value(value, sizeof(value), params+3);
id = qemu_strdup(value);
diff --git a/qemu-option.h b/qemu-option.h
index d735386..58136f3 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -97,6 +97,7 @@ typedef struct QemuOptDesc {
struct QemuOptsList {
const char *name;
+ const char *implied_opt_name;
QTAILQ_HEAD(, QemuOpts) head;
QemuOptDesc desc[];
};
@@ -118,7 +119,7 @@ const char *qemu_opts_id(QemuOpts *opts);
void qemu_opts_del(QemuOpts *opts);
int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc);
int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname);
-QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname);
+QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev);
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict);
QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
diff --git a/vl.c b/vl.c
index 1b58759..d52f5fd 100644
--- a/vl.c
+++ b/vl.c
@@ -1800,7 +1800,7 @@ QemuOpts *drive_add(const char *file, const char *fmt, ...)
vsnprintf(optstr, sizeof(optstr), fmt, ap);
va_end(ap);
- opts = qemu_opts_parse(&qemu_drive_opts, optstr, NULL);
+ opts = qemu_opts_parse(&qemu_drive_opts, optstr, 0);
if (!opts) {
fprintf(stderr, "%s: huh? duplicate? (%s)\n",
__FUNCTION__, optstr);
@@ -4330,7 +4330,7 @@ static int balloon_parse(const char *arg)
if (!strncmp(arg, "virtio", 6)) {
if (arg[6] == ',') {
/* have params -> parse them */
- opts = qemu_opts_parse(&qemu_device_opts, arg+7, NULL);
+ opts = qemu_opts_parse(&qemu_device_opts, arg+7, 0);
if (!opts)
return -1;
} else {
@@ -5317,7 +5317,7 @@ int main(int argc, char **argv, char **envp)
default_monitor = 0;
break;
case QEMU_OPTION_mon:
- opts = qemu_opts_parse(&qemu_mon_opts, optarg, "chardev");
+ opts = qemu_opts_parse(&qemu_mon_opts, optarg, 1);
if (!opts) {
fprintf(stderr, "parse error: %s\n", optarg);
exit(1);
@@ -5325,7 +5325,7 @@ int main(int argc, char **argv, char **envp)
default_monitor = 0;
break;
case QEMU_OPTION_chardev:
- opts = qemu_opts_parse(&qemu_chardev_opts, optarg, "backend");
+ opts = qemu_opts_parse(&qemu_chardev_opts, optarg, 1);
if (!opts) {
fprintf(stderr, "parse error: %s\n", optarg);
exit(1);
@@ -5419,7 +5419,7 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_USB, optarg);
break;
case QEMU_OPTION_device:
- if (!qemu_opts_parse(&qemu_device_opts, optarg, "driver")) {
+ if (!qemu_opts_parse(&qemu_device_opts, optarg, 1)) {
exit(1);
}
break;
@@ -5528,7 +5528,7 @@ int main(int argc, char **argv, char **envp)
configure_rtc_date_offset(optarg, 1);
break;
case QEMU_OPTION_rtc:
- opts = qemu_opts_parse(&qemu_rtc_opts, optarg, NULL);
+ opts = qemu_opts_parse(&qemu_rtc_opts, optarg, 0);
if (!opts) {
fprintf(stderr, "parse error: %s\n", optarg);
exit(1);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 47/50] qemu-option: Rename find_list() to qemu_find_opts() & external linkage
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (45 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 46/50] qemu-option: Move the implied first name into QemuOptsList Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 48/50] monitor: New argument type 'O' Markus Armbruster
` (3 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Next commit wants to use it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qemu-config.c | 8 ++++----
qemu-config.h | 1 +
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index a2e0193..150157c 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -309,7 +309,7 @@ static QemuOptsList *lists[] = {
NULL,
};
-static QemuOptsList *find_list(const char *group)
+QemuOptsList *qemu_find_opts(const char *group)
{
int i;
@@ -336,7 +336,7 @@ int qemu_set_option(const char *str)
return -1;
}
- list = find_list(group);
+ list = qemu_find_opts(group);
if (list == NULL) {
return -1;
}
@@ -451,7 +451,7 @@ int qemu_config_parse(FILE *fp, const char *fname)
}
if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
/* group with id */
- list = find_list(group);
+ list = qemu_find_opts(group);
if (list == NULL)
goto out;
opts = qemu_opts_create(list, id, 1);
@@ -459,7 +459,7 @@ int qemu_config_parse(FILE *fp, const char *fname)
}
if (sscanf(line, "[%63[^]]]", group) == 1) {
/* group without id */
- list = find_list(group);
+ list = qemu_find_opts(group);
if (list == NULL)
goto out;
opts = qemu_opts_create(list, NULL, 0);
diff --git a/qemu-config.h b/qemu-config.h
index c507687..f217c58 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -11,6 +11,7 @@ extern QemuOptsList qemu_global_opts;
extern QemuOptsList qemu_mon_opts;
extern QemuOptsList qemu_cpudef_opts;
+QemuOptsList *qemu_find_opts(const char *group);
int qemu_set_option(const char *str);
int qemu_global_option(const char *str);
void qemu_add_globals(void);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 48/50] monitor: New argument type 'O'
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (46 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 47/50] qemu-option: Rename find_list() to qemu_find_opts() & external linkage Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 49/50] monitor: Use argument type 'O' for device_add Markus Armbruster
` (2 subsequent siblings)
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
In the human monitor, it declares a single optional argument to be
parsed according to the QemuOptsList given by its name.
In QMP, it declares an optional argument for each member of the
QemuOptsList.
Restriction: only lists with empty desc are supported for now. Good
enough for the job at hand. We'll lift the restriction when we need
that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index dfeb9db..895d83b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -67,6 +67,11 @@
* 'F' filename
* 'B' block device name
* 's' string (accept optional quote)
+ * 'O' option string of the form NAME=VALUE,...
+ * parsed according to QemuOptsList given by its name
+ * Example: 'device:O' uses qemu_device_opts.
+ * Restriction: only lists with empty desc are supported
+ * TODO lift the restriction
* 'i' 32 bit integer
* 'l' target long (32 or 64 bit)
* 'M' just like 'l', except in user mode the value is
@@ -3642,6 +3647,31 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
qdict_put(qdict, key, qstring_from_str(buf));
}
break;
+ case 'O':
+ {
+ QemuOptsList *opts_list;
+ QemuOpts *opts;
+
+ opts_list = qemu_find_opts(key);
+ if (!opts_list || opts_list->desc->name) {
+ goto bad_type;
+ }
+ while (qemu_isspace(*p)) {
+ p++;
+ }
+ if (!*p)
+ break;
+ if (get_str(buf, sizeof(buf), &p) < 0) {
+ goto fail;
+ }
+ opts = qemu_opts_parse(opts_list, buf, 1);
+ if (!opts) {
+ goto fail;
+ }
+ qemu_opts_to_qdict(opts, qdict);
+ qemu_opts_del(opts);
+ }
+ break;
case '/':
{
int count, format, size;
@@ -4296,6 +4326,7 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
qint_from_int(qbool_get_int(qobject_to_qbool(value))));
}
break;
+ case 'O':
default:
/* impossible */
abort();
@@ -4310,6 +4341,12 @@ static void cmd_args_init(CmdArgs *cmd_args)
cmd_args->type = cmd_args->flag = cmd_args->optional = 0;
}
+static int check_opts(QemuOptsList *opts_list, QDict *args)
+{
+ assert(!opts_list->desc->name);
+ return 0;
+}
+
/*
* This is not trivial, we have to parse Monitor command's argument
* type syntax to be able to check the arguments provided by clients.
@@ -4322,6 +4359,7 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
int err;
const char *p;
CmdArgs cmd_args;
+ QemuOptsList *opts_list;
if (cmd->args_type == NULL) {
return (qdict_size(args) == 0 ? 0 : -1);
@@ -4329,6 +4367,7 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
err = 0;
cmd_args_init(&cmd_args);
+ opts_list = NULL;
for (p = cmd->args_type;; p++) {
if (*p == ':') {
@@ -4337,16 +4376,23 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
if (cmd_args.type == '-') {
cmd_args.flag = *p++;
cmd_args.optional = 1;
+ } else if (cmd_args.type == 'O') {
+ opts_list = qemu_find_opts(qstring_get_str(cmd_args.name));
+ assert(opts_list);
} else if (*p == '?') {
cmd_args.optional = 1;
p++;
}
assert(*p == ',' || *p == '\0');
- err = check_arg(&cmd_args, args);
-
- QDECREF(cmd_args.name);
- cmd_args_init(&cmd_args);
+ if (opts_list) {
+ err = check_opts(opts_list, args);
+ opts_list = NULL;
+ } else {
+ err = check_arg(&cmd_args, args);
+ QDECREF(cmd_args.name);
+ cmd_args_init(&cmd_args);
+ }
if (err < 0) {
break;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 49/50] monitor: Use argument type 'O' for device_add
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (47 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 48/50] monitor: New argument type 'O' Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 50/50] monitor: convert do_device_add() to QObject Markus Armbruster
2010-03-16 18:31 ` [Qemu-devel] [PULL v2] Convert device_add to QObject / QError Markus Armbruster
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
While there, improve the params help text.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 3 +--
qemu-monitor.hx | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 1edfce8..241f204 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -766,8 +766,7 @@ void do_device_add(Monitor *mon, const QDict *qdict)
{
QemuOpts *opts;
- opts = qemu_opts_parse(&qemu_device_opts,
- qdict_get_str(qdict, "config"), 1);
+ opts = qemu_opts_from_qdict(&qemu_device_opts, qdict);
if (opts) {
if (qdev_device_help(opts) || qdev_device_add(opts) == NULL) {
qemu_opts_del(opts);
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 7f9d261..45e5fd1 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -570,8 +570,8 @@ ETEXI
{
.name = "device_add",
- .args_type = "config:s",
- .params = "device",
+ .args_type = "device:O",
+ .params = "driver[,prop=value][,...]",
.help = "add device, like -device on the command line",
.mhandler.cmd = do_device_add,
},
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] [PATCH 50/50] monitor: convert do_device_add() to QObject
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (48 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 49/50] monitor: Use argument type 'O' for device_add Markus Armbruster
@ 2010-03-04 15:57 ` Markus Armbruster
2010-03-16 18:31 ` [Qemu-devel] [PULL v2] Convert device_add to QObject / QError Markus Armbruster
50 siblings, 0 replies; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/qdev.c | 42 ++++++++++++++++++++++++++++++++++--------
hw/qdev.h | 2 +-
qemu-monitor.hx | 3 ++-
3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 241f204..35e7a88 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -586,7 +586,9 @@ static BusState *qbus_find(const char *path)
dev = qbus_find_dev(bus, elem);
if (!dev) {
qerror_report(QERR_DEVICE_NOT_FOUND, elem);
- qbus_list_dev(bus);
+ if (!monitor_cur_is_qmp()) {
+ qbus_list_dev(bus);
+ }
return NULL;
}
@@ -605,7 +607,9 @@ static BusState *qbus_find(const char *path)
return QLIST_FIRST(&dev->child_bus);
default:
qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
- qbus_list_bus(dev);
+ if (!monitor_cur_is_qmp()) {
+ qbus_list_bus(dev);
+ }
return NULL;
}
}
@@ -619,7 +623,9 @@ static BusState *qbus_find(const char *path)
bus = qbus_find_bus(dev, elem);
if (!bus) {
qerror_report(QERR_BUS_NOT_FOUND, elem);
- qbus_list_bus(dev);
+ if (!monitor_cur_is_qmp()) {
+ qbus_list_bus(dev);
+ }
return NULL;
}
}
@@ -762,16 +768,36 @@ void do_info_qdm(Monitor *mon)
}
}
-void do_device_add(Monitor *mon, const QDict *qdict)
+/**
+ * do_device_add(): Add a device
+ *
+ * Argument qdict contains
+ * - "driver": the name of the new device's driver
+ * - "bus": the device's parent bus (device tree path)
+ * - "id": the device's ID (must be unique)
+ * - device properties
+ *
+ * Example:
+ *
+ * { "driver": "usb-net", "id": "eth1", "netdev": "netdev1" }
+ */
+int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
QemuOpts *opts;
opts = qemu_opts_from_qdict(&qemu_device_opts, qdict);
- if (opts) {
- if (qdev_device_help(opts) || qdev_device_add(opts) == NULL) {
- qemu_opts_del(opts);
- }
+ if (!opts) {
+ return -1;
+ }
+ if (!monitor_cur_is_qmp() && qdev_device_help(opts)) {
+ qemu_opts_del(opts);
+ return 0;
}
+ if (!qdev_device_add(opts)) {
+ qemu_opts_del(opts);
+ return -1;
+ }
+ return 0;
}
void do_device_del(Monitor *mon, const QDict *qdict)
diff --git a/hw/qdev.h b/hw/qdev.h
index 0eb45b0..c86a59d 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -171,7 +171,7 @@ void qbus_free(BusState *bus);
void do_info_qtree(Monitor *mon);
void do_info_qdm(Monitor *mon);
-void do_device_add(Monitor *mon, const QDict *qdict);
+int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
void do_device_del(Monitor *mon, const QDict *qdict);
/*** qdev-properties.c ***/
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 45e5fd1..5308f36 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -573,7 +573,8 @@ ETEXI
.args_type = "device:O",
.params = "driver[,prop=value][,...]",
.help = "add device, like -device on the command line",
- .mhandler.cmd = do_device_add,
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_new = do_device_add,
},
STEXI
--
1.6.6.1
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-04 15:56 ` [Qemu-devel] [PATCH 28/50] error: Let converted handlers print in human monitor Markus Armbruster
@ 2010-03-04 20:50 ` Luiz Capitulino
2010-03-04 21:06 ` Markus Armbruster
2010-03-05 15:43 ` Luiz Capitulino
0 siblings, 2 replies; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-04 20:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Thu, 4 Mar 2010 16:56:49 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> While fully converted handlers are not supposed to print anything when
> running in a QMP monitor, they are free to print in a human monitor.
I disagree.
One of the key decisions behind the new Monitor design is that handlers
are part of common code. User printing is output-specific and should not
be done by handlers.
One of the problems with this layer violation is that the consumer of this
data might not be what you have assumed. For example, it has been suggested
that we could move the qemu shell out of qemu in the future. In this case,
this kind of information _could_ be useful.
Another important problem is that free printing like this is error-prone
as it's not at all clear what could be printed and there's no way to catch
mistakes mechanically. Not to mention that this is for sure going to used
by those who want to ignore QMP completely.
I'm ok with shallow conversion, provided that handler_audit() is there to
warn us about the job to be done.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 45/50] qemu-option: Functions to convert to/from QDict
2010-03-04 15:57 ` [Qemu-devel] [PATCH 45/50] qemu-option: Functions to convert to/from QDict Markus Armbruster
@ 2010-03-04 20:55 ` Luiz Capitulino
2010-03-04 21:12 ` Markus Armbruster
0 siblings, 1 reply; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-04 20:55 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Thu, 4 Mar 2010 16:57:06 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> The functions are somewhat restricted. Good enough for the job at
> hand. We'll extend them when we need more.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qemu-option.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-option.h | 3 ++
> 2 files changed, 82 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index ab488e4..24bb19b 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -28,6 +28,7 @@
>
> #include "qemu-common.h"
> #include "qemu-error.h"
> +#include "qemu-objects.h"
> #include "qemu-option.h"
>
> /*
> @@ -777,6 +778,84 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *fi
> return opts;
> }
>
> +static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
> +{
> + char buf[32];
> + const char *value;
> + int n;
> +
> + if (!strcmp(key, "id")) {
> + return;
> + }
> +
> + switch (qobject_type(obj)) {
> + case QTYPE_QSTRING:
> + value = qstring_get_str(qobject_to_qstring(obj));
> + break;
> + case QTYPE_QINT:
> + n = snprintf(buf, sizeof(buf), "%" PRId64,
> + qint_get_int(qobject_to_qint(obj)));
> + assert(n < sizeof(buf));
> + value = buf;
> + break;
> + case QTYPE_QFLOAT:
> + n = snprintf(buf, sizeof(buf), "%.17g",
> + qfloat_get_double(qobject_to_qfloat(obj)));
> + assert(n < sizeof(buf));
> + value = buf;
> + break;
> + case QTYPE_QBOOL:
> + strcpy(buf, qbool_get_int(qobject_to_qbool(obj)) ? "on" : "off");
> + value = buf;
> + break;
> + default:
> + return;
> + }
> + qemu_opt_set(opaque, key, value);
> +}
> +
> +/*
> + * Create QemuOpts from a QDict.
> + * Use value of key "id" as ID if it exists and is a QString.
> + * Only QStrings, QInts, QFloats and QBools are copied. Entries with
> + * other types are silently ignored.
> + */
> +QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict)
> +{
> + QemuOpts *opts;
> +
> + opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1);
> + if (opts == NULL)
> + return NULL;
> +
> + qdict_iter(qdict, qemu_opts_from_qdict_1, opts);
> + return opts;
> +}
> +
> +/*
> + * Convert from QemuOpts to QDict.
> + * The QDict values are of type QString.
> + * TODO We'll want to use types appropriate for opt->desc->type, but
> + * this is enough for now.
> + */
> +QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
> +{
> + QemuOpt *opt;
> + QObject *val;
> +
> + if (!qdict) {
> + qdict = qdict_new();
> + }
> + if (opts->id) {
> + qdict_put(qdict, "id", qstring_from_str(opts->id));
> + }
> + QTAILQ_FOREACH(opt, &opts->head, next) {
> + val = QOBJECT(qstring_from_str(opt->str));
> + qdict_put_obj(qdict, opt->name, val);
> + }
Why not just do:
qdict_put(qdict, opt->name, qstring_from_str(opt->str));
> + return qdict;
> +}
> +
> /* Validate parsed opts against descriptions where no
> * descriptions were provided in the QemuOptsList.
> */
> diff --git a/qemu-option.h b/qemu-option.h
> index f3f1de7..d735386 100644
> --- a/qemu-option.h
> +++ b/qemu-option.h
> @@ -28,6 +28,7 @@
>
> #include <stdint.h>
> #include "qemu-queue.h"
> +#include "qdict.h"
>
> enum QEMUOptionParType {
> OPT_FLAG,
> @@ -118,6 +119,8 @@ void qemu_opts_del(QemuOpts *opts);
> int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc);
> int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname);
> QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname);
> +QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict);
> +QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
>
> typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
> int qemu_opts_print(QemuOpts *opts, void *dummy);
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-04 20:50 ` [Qemu-devel] " Luiz Capitulino
@ 2010-03-04 21:06 ` Markus Armbruster
2010-03-04 21:14 ` Luiz Capitulino
2010-03-05 15:43 ` Luiz Capitulino
1 sibling, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 21:06 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 4 Mar 2010 16:56:49 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> While fully converted handlers are not supposed to print anything when
>> running in a QMP monitor, they are free to print in a human monitor.
>
> I disagree.
>
> One of the key decisions behind the new Monitor design is that handlers
> are part of common code. User printing is output-specific and should not
> be done by handlers.
>
> One of the problems with this layer violation is that the consumer of this
> data might not be what you have assumed. For example, it has been suggested
> that we could move the qemu shell out of qemu in the future. In this case,
> this kind of information _could_ be useful.
How do you intend to solve the problem of printing help then?
The perfect is the enemy of the good.
> Another important problem is that free printing like this is error-prone
> as it's not at all clear what could be printed and there's no way to catch
> mistakes mechanically. Not to mention that this is for sure going to used
> by those who want to ignore QMP completely.
This paragraph I don't get.
> I'm ok with shallow conversion, provided that handler_audit() is there to
> warn us about the job to be done.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 45/50] qemu-option: Functions to convert to/from QDict
2010-03-04 20:55 ` [Qemu-devel] " Luiz Capitulino
@ 2010-03-04 21:12 ` Markus Armbruster
2010-03-04 21:17 ` Luiz Capitulino
0 siblings, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-04 21:12 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 4 Mar 2010 16:57:06 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> The functions are somewhat restricted. Good enough for the job at
>> hand. We'll extend them when we need more.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qemu-option.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> qemu-option.h | 3 ++
>> 2 files changed, 82 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-option.c b/qemu-option.c
>> index ab488e4..24bb19b 100644
>> --- a/qemu-option.c
>> +++ b/qemu-option.c
[...]
>> +/*
>> + * Convert from QemuOpts to QDict.
>> + * The QDict values are of type QString.
>> + * TODO We'll want to use types appropriate for opt->desc->type, but
>> + * this is enough for now.
>> + */
>> +QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
>> +{
>> + QemuOpt *opt;
>> + QObject *val;
>> +
>> + if (!qdict) {
>> + qdict = qdict_new();
>> + }
>> + if (opts->id) {
>> + qdict_put(qdict, "id", qstring_from_str(opts->id));
>> + }
>> + QTAILQ_FOREACH(opt, &opts->head, next) {
>> + val = QOBJECT(qstring_from_str(opt->str));
>> + qdict_put_obj(qdict, opt->name, val);
>> + }
>
> Why not just do:
>
> qdict_put(qdict, opt->name, qstring_from_str(opt->str));
I got a version without the TODO. Looks like this:
QTAILQ_FOREACH(opt, &opts->head, next) {
switch (opt->desc ? opt->desc->type : QEMU_OPT_STRING) {
case QEMU_OPT_BOOL:
val = QOBJECT(qbool_from_int(opt->value.boolean));
break;
case QEMU_OPT_NUMBER:
case QEMU_OPT_SIZE:
// FIXME opts only uint64_t, qint only int64_t
val = QOBJECT(qint_from_int(opt->value.uint));
break;
default:
val = QOBJECT(qstring_from_str(opt->str));
break;
}
qdict_put_obj(qdict, opt->name, val);
}
I dumbed it down to keep this series as simple as possible, and missed
the simplification opportunity. Okay to leave it as is?
[...]
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-04 21:06 ` Markus Armbruster
@ 2010-03-04 21:14 ` Luiz Capitulino
0 siblings, 0 replies; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-04 21:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Thu, 04 Mar 2010 22:06:42 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > On Thu, 4 Mar 2010 16:56:49 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> While fully converted handlers are not supposed to print anything when
> >> running in a QMP monitor, they are free to print in a human monitor.
> >
> > I disagree.
> >
> > One of the key decisions behind the new Monitor design is that handlers
> > are part of common code. User printing is output-specific and should not
> > be done by handlers.
> >
> > One of the problems with this layer violation is that the consumer of this
> > data might not be what you have assumed. For example, it has been suggested
> > that we could move the qemu shell out of qemu in the future. In this case,
> > this kind of information _could_ be useful.
>
> How do you intend to solve the problem of printing help then?
Doesn't a shallow conversion takes care of it for now?
> The perfect is the enemy of the good.
>
> > Another important problem is that free printing like this is error-prone
> > as it's not at all clear what could be printed and there's no way to catch
> > mistakes mechanically. Not to mention that this is for sure going to used
> > by those who want to ignore QMP completely.
>
> This paragraph I don't get.
If something that should be available under qmp is printed by using
monitor_printf(), we loose big time.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 45/50] qemu-option: Functions to convert to/from QDict
2010-03-04 21:12 ` Markus Armbruster
@ 2010-03-04 21:17 ` Luiz Capitulino
0 siblings, 0 replies; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-04 21:17 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Thu, 04 Mar 2010 22:12:01 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > On Thu, 4 Mar 2010 16:57:06 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> The functions are somewhat restricted. Good enough for the job at
> >> hand. We'll extend them when we need more.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> qemu-option.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> qemu-option.h | 3 ++
> >> 2 files changed, 82 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/qemu-option.c b/qemu-option.c
> >> index ab488e4..24bb19b 100644
> >> --- a/qemu-option.c
> >> +++ b/qemu-option.c
> [...]
> >> +/*
> >> + * Convert from QemuOpts to QDict.
> >> + * The QDict values are of type QString.
> >> + * TODO We'll want to use types appropriate for opt->desc->type, but
> >> + * this is enough for now.
> >> + */
> >> +QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
> >> +{
> >> + QemuOpt *opt;
> >> + QObject *val;
> >> +
> >> + if (!qdict) {
> >> + qdict = qdict_new();
> >> + }
> >> + if (opts->id) {
> >> + qdict_put(qdict, "id", qstring_from_str(opts->id));
> >> + }
> >> + QTAILQ_FOREACH(opt, &opts->head, next) {
> >> + val = QOBJECT(qstring_from_str(opt->str));
> >> + qdict_put_obj(qdict, opt->name, val);
> >> + }
> >
> > Why not just do:
> >
> > qdict_put(qdict, opt->name, qstring_from_str(opt->str));
>
> I got a version without the TODO. Looks like this:
>
> QTAILQ_FOREACH(opt, &opts->head, next) {
> switch (opt->desc ? opt->desc->type : QEMU_OPT_STRING) {
> case QEMU_OPT_BOOL:
> val = QOBJECT(qbool_from_int(opt->value.boolean));
> break;
> case QEMU_OPT_NUMBER:
> case QEMU_OPT_SIZE:
> // FIXME opts only uint64_t, qint only int64_t
> val = QOBJECT(qint_from_int(opt->value.uint));
> break;
> default:
> val = QOBJECT(qstring_from_str(opt->str));
> break;
> }
> qdict_put_obj(qdict, opt->name, val);
> }
>
> I dumbed it down to keep this series as simple as possible, and missed
> the simplification opportunity. Okay to leave it as is?
Yes, although it's worth changing in case you resend the series with
other modifications (assuming you'll cleanup soon, of course).
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-04 20:50 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 21:06 ` Markus Armbruster
@ 2010-03-05 15:43 ` Luiz Capitulino
2010-03-05 16:43 ` Markus Armbruster
1 sibling, 1 reply; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-05 15:43 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Thu, 4 Mar 2010 17:50:20 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Thu, 4 Mar 2010 16:56:49 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
> > While fully converted handlers are not supposed to print anything when
> > running in a QMP monitor, they are free to print in a human monitor.
>
> I disagree.
I've talked to Markus by irc about this one and he convinced me that
this is the best solution for the immediate term.
Actually, I found out that only error is printed in the human monitor
(through qerror_report()) so this is not as serious as I thought at first.
So, I won't nack it and the bigger mid-term discussion we should have
is whether or not it's ok to mix qerror_report(), error_printf() &
friends in handlers.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-05 15:43 ` Luiz Capitulino
@ 2010-03-05 16:43 ` Markus Armbruster
2010-03-08 13:41 ` Luiz Capitulino
0 siblings, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-05 16:43 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 4 Mar 2010 17:50:20 -0300
> Luiz Capitulino <lcapitulino@redhat.com> wrote:
>
>> On Thu, 4 Mar 2010 16:56:49 +0100
>> Markus Armbruster <armbru@redhat.com> wrote:
>>
>> > While fully converted handlers are not supposed to print anything when
>> > running in a QMP monitor, they are free to print in a human monitor.
>>
>> I disagree.
>
> I've talked to Markus by irc about this one and he convinced me that
> this is the best solution for the immediate term.
>
> Actually, I found out that only error is printed in the human monitor
> (through qerror_report()) so this is not as serious as I thought at first.
In case I confused not just you: the patch does not affect QMP monitors
at all.
I wish I had explained this better. Let me retry: the patch provides
for partially converted handlers. Without it, CONFIG_DEBUG_MONITOR
cries bloody murder, and converted errors get delayed until after all
output from the not-yet-converted parts of the handler.
For instance, it takes me four steps to fully convert do_device_add().
I want all the partially converted intermediate versions to work.
Even more compelling is the case of utility functions: I want to be able
to convert a function to QError without having to convert all the
handlers using it to QError / QObject in the same commit, because doing
that would result in an unreviewable hairball of a patch.
> So, I won't nack it and the bigger mid-term discussion we should have
> is whether or not it's ok to mix qerror_report(), error_printf() &
> friends in handlers.
I think mixing is both unavoidable and harmless in intermediate
conversion steps.
Whether we want to allow stuff like error_printf_unless_qmp() in the
final state is a separate and valid question. error_printf_unless_qmp()
solved the problem at hand for me, but of course I'm open to other
ideas. I don't think delaying this series could help with that, though.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 28/50] error: Let converted handlers print in human monitor
2010-03-05 16:43 ` Markus Armbruster
@ 2010-03-08 13:41 ` Luiz Capitulino
0 siblings, 0 replies; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-08 13:41 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Fri, 05 Mar 2010 17:43:40 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
> >
> > So, I won't nack it and the bigger mid-term discussion we should have
> > is whether or not it's ok to mix qerror_report(), error_printf() &
> > friends in handlers.
>
> I think mixing is both unavoidable and harmless in intermediate
> conversion steps.
I agree.
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] Re: [PATCH 16/50] error: Replace qemu_error() by error_report()
2010-03-04 15:56 ` [Qemu-devel] [PATCH 16/50] error: Replace qemu_error() by error_report() Markus Armbruster
@ 2010-03-13 2:34 ` Luiz Capitulino
0 siblings, 0 replies; 63+ messages in thread
From: Luiz Capitulino @ 2010-03-13 2:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Thu, 4 Mar 2010 16:56:37 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> error_report() terminates the message with a newline. Strip it it
> from its arguments.
>
> This fixes a few error messages lacking a newline:
> net_handle_fd_param()'s "No file descriptor named %s found", and
> tap_open()'s "vnet_hdr=1 requested, but no kernel support for
> IFF_VNET_HDR available" (all three versions).
>
> There's one place that passes arguments without newlines
> intentionally: load_vmstate(). Fix it up.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
I'm starting to queue QMP related patches, so that we test them early,
and this one doesn't apply anymore against HEAD. Didn't investigate much,
though.
If you decide to rebase, remember that we have patches from Shahar
that modify qerror.[hc], my queue is at:
git://repo.or.cz/qemu/qmp-unstable.git (master)
^ permalink raw reply [flat|nested] 63+ messages in thread
* [Qemu-devel] [PULL v2] Convert device_add to QObject / QError
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
` (49 preceding siblings ...)
2010-03-04 15:57 ` [Qemu-devel] [PATCH 50/50] monitor: convert do_device_add() to QObject Markus Armbruster
@ 2010-03-16 18:31 ` Markus Armbruster
2010-03-17 15:28 ` Anthony Liguori
50 siblings, 1 reply; 63+ messages in thread
From: Markus Armbruster @ 2010-03-16 18:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino
Anthony ran into conflicts and asked me to rebase and send out a pull
request.
Complete list of conflicts:
* qdev: Improve diagnostics for bad property values
commit 6bf38816df80a3b50529119c5458b151b3e2c728
Adds two new errors to qdev_prop_parse(), which need conversion to
QError. Resolution straighforward, just needs new
QERR_PROPERTY_VALUE_IN_USE, QERR_PROPERTY_VALUE_NOT_FOUND.
* scsi: Make device scsi-disk reject /dev/sg*
commit 32bb404a6a4d726dfd691f75704f08257ce65ffe
Adds a qemu_error() use, which needs to be changed to error_report().
* slirp: check system() success
commit 24ac07dec7f23c58dc48aa7754f872781b386d46
Context changed. Resolution trivial.
The following changes since commit 0aef4261ac0ec9089ade0e3a92f986cb4ba7317e:
Aurelien Jarno (1):
target-ppc: fix evsrwu and evsrws (second try)
are available in the git repository at:
git://repo.or.cz/qemu/armbru.git qerror
Markus Armbruster (52):
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
error: New QERR_PROPERTY_VALUE_IN_USE
error: New QERR_PROPERTY_VALUE_NOT_FOUND
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 | 36 ++---
hw/qdev.c | 236 ++++++++++++++++++++--------------
hw/qdev.h | 2 +-
hw/scsi-bus.c | 4 +-
hw/scsi-disk.c | 7 +-
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 | 83 +++++++++---
qerror.h | 51 ++++++-
savevm.c | 27 ++--
slirp/misc.c | 2 +-
sysemu.h | 13 +--
usb-linux.c | 8 -
vl.c | 44 ++++---
vnc.c | 5 +-
45 files changed, 1007 insertions(+), 533 deletions(-)
create mode 100644 qemu-error.c
create mode 100644 qemu-error.h
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [Qemu-devel] [PULL v2] Convert device_add to QObject / QError
2010-03-16 18:31 ` [Qemu-devel] [PULL v2] Convert device_add to QObject / QError Markus Armbruster
@ 2010-03-17 15:28 ` Anthony Liguori
0 siblings, 0 replies; 63+ messages in thread
From: Anthony Liguori @ 2010-03-17 15:28 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Luiz Capitulino
On 03/16/2010 01:31 PM, Markus Armbruster wrote:
> Anthony ran into conflicts and asked me to rebase and send out a pull
> request.
>
> Complete list of conflicts:
>
> * qdev: Improve diagnostics for bad property values
> commit 6bf38816df80a3b50529119c5458b151b3e2c728
>
> Adds two new errors to qdev_prop_parse(), which need conversion to
> QError. Resolution straighforward, just needs new
> QERR_PROPERTY_VALUE_IN_USE, QERR_PROPERTY_VALUE_NOT_FOUND.
>
> * scsi: Make device scsi-disk reject /dev/sg*
> commit 32bb404a6a4d726dfd691f75704f08257ce65ffe
>
> Adds a qemu_error() use, which needs to be changed to error_report().
>
> * slirp: check system() success
> commit 24ac07dec7f23c58dc48aa7754f872781b386d46
>
> Context changed. Resolution trivial.
>
>
> The following changes since commit 0aef4261ac0ec9089ade0e3a92f986cb4ba7317e:
> Aurelien Jarno (1):
> target-ppc: fix evsrwu and evsrws (second try)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/armbru.git qerror
>
Applied all. Thanks.
Regards,
Anthony Liguori
> Markus Armbruster (52):
> 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
> error: New QERR_PROPERTY_VALUE_IN_USE
> error: New QERR_PROPERTY_VALUE_NOT_FOUND
> 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 | 36 ++---
> hw/qdev.c | 236 ++++++++++++++++++++--------------
> hw/qdev.h | 2 +-
> hw/scsi-bus.c | 4 +-
> hw/scsi-disk.c | 7 +-
> 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 | 83 +++++++++---
> qerror.h | 51 ++++++-
> savevm.c | 27 ++--
> slirp/misc.c | 2 +-
> sysemu.h | 13 +--
> usb-linux.c | 8 -
> vl.c | 44 ++++---
> vnc.c | 5 +-
> 45 files changed, 1007 insertions(+), 533 deletions(-)
> create mode 100644 qemu-error.c
> create mode 100644 qemu-error.h
>
>
>
^ permalink raw reply [flat|nested] 63+ messages in thread
end of thread, other threads:[~2010-03-17 15:28 UTC | newest]
Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-04 15:56 [Qemu-devel] [PATCH 00/50] Convert device_add to QObject / QError Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 01/50] usb: Remove disabled monitor_printf() in usb_read_file() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 02/50] savevm: Fix -loadvm to report errors to stderr, not the monitor Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 03/50] pc: Fix error reporting for -boot once Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 04/50] pc: Factor common code out of pc_boot_set() and cmos_init() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 05/50] tools: Remove unused cur_mon from qemu-tool.c Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 06/50] monitor: Separate "default monitor" and "current monitor" cleanly Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 07/50] block: Simplify usb_msd_initfn() test for "can read bdrv key" Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 08/50] monitor: Factor monitor_set_error() out of qemu_error_internal() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 09/50] error: Move qemu_error() & friends from monitor.c to own file Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 10/50] error: Simplify error sink setup Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 11/50] error: Move qemu_error & friends into their own header Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 12/50] error: New error_printf() and error_vprintf() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 13/50] error: Don't abuse qemu_error() for non-error in qdev_device_help() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 14/50] error: Don't abuse qemu_error() for non-error in qbus_find() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 15/50] error: Don't abuse qemu_error() for non-error in scsi_hot_add() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 16/50] error: Replace qemu_error() by error_report() Markus Armbruster
2010-03-13 2:34 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 15:56 ` [Qemu-devel] [PATCH 17/50] error: Rename qemu_error_new() to qerror_report() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 18/50] error: Infrastructure to track locations for error reporting Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 19/50] error: Include the program name in error messages to stderr Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 20/50] error: Track locations in configuration files Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 21/50] QemuOpts: Fix qemu_config_parse() to catch file read errors Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 22/50] error: Track locations on command line Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 23/50] qdev: Fix -device and device_add to handle unsuitable bus gracefully Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 24/50] qdev: Factor qdev_create_from_info() out of qdev_create() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 25/50] qdev: Hide "no_user" devices from users Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 26/50] qdev: Hide "ptr" properties " Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 27/50] monitor: New monitor_cur_is_qmp() Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 28/50] error: Let converted handlers print in human monitor Markus Armbruster
2010-03-04 20:50 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 21:06 ` Markus Armbruster
2010-03-04 21:14 ` Luiz Capitulino
2010-03-05 15:43 ` Luiz Capitulino
2010-03-05 16:43 ` Markus Armbruster
2010-03-08 13:41 ` Luiz Capitulino
2010-03-04 15:56 ` [Qemu-devel] [PATCH 29/50] error: Polish human-readable error descriptions Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 30/50] error: New QERR_PROPERTY_NOT_FOUND Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 31/50] error: New QERR_PROPERTY_VALUE_BAD Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 32/50] qdev: convert setting device properties to QError Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 33/50] qdev: Relax parsing of bus option Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 34/50] error: New QERR_BUS_NOT_FOUND Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 35/50] error: New QERR_DEVICE_MULTIPLE_BUSSES Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 36/50] error: New QERR_DEVICE_NO_BUS Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 37/50] qdev: Convert qbus_find() to QError Markus Armbruster
2010-03-04 15:56 ` [Qemu-devel] [PATCH 38/50] error: New error_printf_unless_qmp() Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 39/50] error: New QERR_BAD_BUS_FOR_DEVICE Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 40/50] error: New QERR_BUS_NO_HOTPLUG Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 41/50] error: New QERR_DEVICE_INIT_FAILED Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 42/50] error: New QERR_NO_BUS_FOR_DEVICE Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 43/50] Revert "qdev: Use QError for 'device not found' error" Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 44/50] error: Convert do_device_add() to QError Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 45/50] qemu-option: Functions to convert to/from QDict Markus Armbruster
2010-03-04 20:55 ` [Qemu-devel] " Luiz Capitulino
2010-03-04 21:12 ` Markus Armbruster
2010-03-04 21:17 ` Luiz Capitulino
2010-03-04 15:57 ` [Qemu-devel] [PATCH 46/50] qemu-option: Move the implied first name into QemuOptsList Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 47/50] qemu-option: Rename find_list() to qemu_find_opts() & external linkage Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 48/50] monitor: New argument type 'O' Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 49/50] monitor: Use argument type 'O' for device_add Markus Armbruster
2010-03-04 15:57 ` [Qemu-devel] [PATCH 50/50] monitor: convert do_device_add() to QObject Markus Armbruster
2010-03-16 18:31 ` [Qemu-devel] [PULL v2] Convert device_add to QObject / QError Markus Armbruster
2010-03-17 15:28 ` Anthony Liguori
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.