* [Qemu-devel] [PULL 01/41] error: Document how to accumulate multiple errors
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
@ 2016-01-13 15:42 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 02/41] Use error_fatal to simplify obvious fatal errors Markus Armbruster
` (40 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:42 UTC (permalink / raw)
To: qemu-devel
Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447776349-2344-1-git-send-email-armbru@redhat.com>
---
include/qapi/error.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 6285cf5..1480f59 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -76,6 +76,23 @@
* But when all you do with the error is pass it on, please use
* foo(arg, errp);
* for readability.
+ *
+ * Receive and accumulate multiple errors (first one wins):
+ * Error *err = NULL, *local_err = NULL;
+ * foo(arg, &err);
+ * bar(arg, &local_err);
+ * error_propagate(&err, local_err);
+ * if (err) {
+ * handle the error...
+ * }
+ *
+ * Do *not* "optimize" this to
+ * foo(arg, &err);
+ * bar(arg, &err); // WRONG!
+ * if (err) {
+ * handle the error...
+ * }
+ * because this may pass a non-null err to bar().
*/
#ifndef ERROR_H
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 02/41] Use error_fatal to simplify obvious fatal errors
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
2016-01-13 15:42 ` [Qemu-devel] [PULL 01/41] error: Document how to accumulate multiple errors Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 03/41] hw: Inline the qdev_prop_set_drive_nofail() wrapper Markus Armbruster
` (39 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, qemu-arm, Eduardo Habkost, Michael S. Tsirkin
Done with this Coccinelle semantic patch:
@@
type T;
identifier FUN, RET;
expression list ARGS;
expression ERR, EC;
@@
(
- T RET = FUN(ARGS, &ERR);
+ T RET = FUN(ARGS, &error_fatal);
|
- RET = FUN(ARGS, &ERR);
+ RET = FUN(ARGS, &error_fatal);
|
- FUN(ARGS, &ERR);
+ FUN(ARGS, &error_fatal);
)
- if (ERR != NULL) {
- error_report_err(ERR);
- exit(EC);
- }
This is actually a more elegant version of my initial semantic patch
by courtesy of Eduardo.
It leaves dead Error * variables behind, cleaned up manually.
Cc: qemu-arm@nongnu.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/arm/exynos4210.c | 13 ++----------
hw/arm/highbank.c | 7 +------
hw/arm/integratorcp.c | 13 ++----------
hw/arm/realview.c | 20 ++++---------------
hw/arm/versatilepb.c | 13 ++----------
hw/arm/vexpress.c | 7 +------
hw/arm/xilinx_zynq.c | 28 ++++++--------------------
hw/char/serial.c | 14 ++-----------
hw/core/qdev-properties-system.c | 8 +-------
hw/i386/pc.c | 14 ++-----------
hw/smbios/smbios.c | 43 +++++++---------------------------------
numa.c | 8 ++------
vl.c | 21 +++-----------------
13 files changed, 35 insertions(+), 174 deletions(-)
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index d934980..79b7c5a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -150,27 +150,18 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
for (n = 0; n < EXYNOS4210_NCPUS; n++) {
Object *cpuobj = object_new(object_class_get_name(cpu_oc));
- Error *err = NULL;
/* By default A9 CPUs have EL3 enabled. This board does not currently
* support EL3 so the CPU EL3 property is disabled before realization.
*/
if (object_property_find(cpuobj, "has_el3", NULL)) {
- object_property_set_bool(cpuobj, false, "has_el3", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
}
s->cpu[n] = ARM_CPU(cpuobj);
object_property_set_int(cpuobj, EXYNOS4210_SMP_PRIVATE_BASE_ADDR,
"reset-cbar", &error_abort);
- object_property_set_bool(cpuobj, true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
}
/*** IRQs ***/
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 85ae69e..a0a5a06 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -279,7 +279,6 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
Object *cpuobj;
ARMCPU *cpu;
- Error *err = NULL;
cpuobj = object_new(object_class_get_name(oc));
cpu = ARM_CPU(cpuobj);
@@ -297,11 +296,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
object_property_set_int(cpuobj, MPCORE_PERIPHBASE,
"reset-cbar", &error_abort);
}
- object_property_set_bool(cpuobj, true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
cpu_fiq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ);
}
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 421bde9..96dedce 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -533,7 +533,6 @@ static void integratorcp_init(MachineState *machine)
qemu_irq pic[32];
DeviceState *dev, *sic, *icp;
int i;
- Error *err = NULL;
if (!cpu_model) {
cpu_model = "arm926";
@@ -552,18 +551,10 @@ static void integratorcp_init(MachineState *machine)
* realization.
*/
if (object_property_find(cpuobj, "has_el3", NULL)) {
- object_property_set_bool(cpuobj, false, "has_el3", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
}
- object_property_set_bool(cpuobj, true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
cpu = ARM_CPU(cpuobj);
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index e14828d..2d6952c 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -99,33 +99,21 @@ static void realview_init(MachineState *machine,
for (n = 0; n < smp_cpus; n++) {
Object *cpuobj = object_new(object_class_get_name(cpu_oc));
- Error *err = NULL;
/* By default A9,A15 and ARM1176 CPUs have EL3 enabled. This board
* does not currently support EL3 so the CPU EL3 property is disabled
* before realization.
*/
if (object_property_find(cpuobj, "has_el3", NULL)) {
- object_property_set_bool(cpuobj, false, "has_el3", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
}
if (is_pb && is_mpcore) {
- object_property_set_int(cpuobj, periphbase, "reset-cbar", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_int(cpuobj, periphbase, "reset-cbar",
+ &error_fatal);
}
- object_property_set_bool(cpuobj, true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpuobj), ARM_CPU_IRQ);
}
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 912c290..70eefe9 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -192,7 +192,6 @@ static void versatile_init(MachineState *machine, int board_id)
int n;
int done_smc = 0;
DriveInfo *dinfo;
- Error *err = NULL;
if (!machine->cpu_model) {
machine->cpu_model = "arm926";
@@ -211,18 +210,10 @@ static void versatile_init(MachineState *machine, int board_id)
* realization.
*/
if (object_property_find(cpuobj, "has_el3", NULL)) {
- object_property_set_bool(cpuobj, false, "has_el3", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, false, "has_el3", &error_fatal);
}
- object_property_set_bool(cpuobj, true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
cpu = ARM_CPU(cpuobj);
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 058abbd..ea9a984 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -211,7 +211,6 @@ static void init_cpus(const char *cpu_model, const char *privdev,
/* Create the actual CPUs */
for (n = 0; n < smp_cpus; n++) {
Object *cpuobj = object_new(object_class_get_name(cpu_oc));
- Error *err = NULL;
if (!secure) {
object_property_set_bool(cpuobj, false, "has_el3", NULL);
@@ -221,11 +220,7 @@ static void init_cpus(const char *cpu_model, const char *privdev,
object_property_set_int(cpuobj, periphbase,
"reset-cbar", &error_abort);
}
- object_property_set_bool(cpuobj, true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(cpuobj, true, "realized", &error_fatal);
}
/* Create the private peripheral devices (including the GIC);
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1c1a445..65e92e1 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -156,7 +156,6 @@ static void zynq_init(MachineState *machine)
DeviceState *dev;
SysBusDevice *busdev;
qemu_irq pic[64];
- Error *err = NULL;
int n;
if (!cpu_model) {
@@ -171,29 +170,14 @@ static void zynq_init(MachineState *machine)
* realization.
*/
if (object_property_find(OBJECT(cpu), "has_el3", NULL)) {
- object_property_set_bool(OBJECT(cpu), false, "has_el3", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(OBJECT(cpu), false, "has_el3", &error_fatal);
}
- object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
-
- object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
- object_property_set_bool(OBJECT(cpu), true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr",
+ &error_fatal);
+ object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar",
+ &error_fatal);
+ object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
/* max 2GB ram */
if (ram_size > 0x80000000) {
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 513d73c..566e9ef 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -888,18 +888,13 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
CharDriverState *chr, MemoryRegion *system_io)
{
SerialState *s;
- Error *err = NULL;
s = g_malloc0(sizeof(SerialState));
s->irq = irq;
s->baudbase = baudbase;
s->chr = chr;
- serial_realize_core(s, &err);
- if (err != NULL) {
- error_report_err(err);
- exit(1);
- }
+ serial_realize_core(s, &error_fatal);
vmstate_register(NULL, base, &vmstate_serial, s);
@@ -949,7 +944,6 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
CharDriverState *chr, enum device_endian end)
{
SerialState *s;
- Error *err = NULL;
s = g_malloc0(sizeof(SerialState));
@@ -958,11 +952,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
s->baudbase = baudbase;
s->chr = chr;
- serial_realize_core(s, &err);
- if (err != NULL) {
- error_report_err(err);
- exit(1);
- }
+ serial_realize_core(s, &error_fatal);
vmstate_register(NULL, base, &vmstate_serial, s);
memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 921e799..d515e99 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -367,13 +367,7 @@ void qdev_prop_set_drive(DeviceState *dev, const char *name,
void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
BlockBackend *value)
{
- Error *err = NULL;
-
- qdev_prop_set_drive(dev, name, value, &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ qdev_prop_set_drive(dev, name, value, &error_fatal);
}
void qdev_prop_set_chr(DeviceState *dev, const char *name,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c36b8cf..166e8e2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -433,7 +433,6 @@ void pc_cmos_init(PCMachineState *pcms,
{
int val;
static pc_cmos_init_late_arg arg;
- Error *local_err = NULL;
/* various important CMOS locations needed by PC/Bochs bios */
@@ -481,11 +480,7 @@ void pc_cmos_init(PCMachineState *pcms,
object_property_set_link(OBJECT(pcms), OBJECT(s),
"rtc_state", &error_abort);
- set_boot_dev(s, MACHINE(pcms)->boot_order, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ set_boot_dev(s, MACHINE(pcms)->boot_order, &error_fatal);
val = 0;
val |= 0x02; /* FPU is there */
@@ -1123,7 +1118,6 @@ void pc_cpus_init(PCMachineState *pcms)
int i;
X86CPU *cpu = NULL;
MachineState *machine = MACHINE(pcms);
- Error *error = NULL;
unsigned long apic_id_limit;
/* init CPUs */
@@ -1144,11 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms)
for (i = 0; i < smp_cpus; i++) {
cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
- &error);
- if (error) {
- error_report_err(error);
- exit(1);
- }
+ &error_fatal);
object_unref(OBJECT(cpu));
}
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index b81a1d3..a3e575a 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -937,7 +937,6 @@ static void save_opt(const char **dest, QemuOpts *opts, const char *name)
void smbios_entry_add(QemuOpts *opts)
{
- Error *local_err = NULL;
const char *val;
assert(!smbios_immutable);
@@ -948,11 +947,7 @@ void smbios_entry_add(QemuOpts *opts)
int size;
struct smbios_table *table; /* legacy mode only */
- qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_file_opts, &error_fatal);
size = get_image_size(val);
if (size == -1 || size < sizeof(struct smbios_structure_header)) {
@@ -1034,11 +1029,7 @@ void smbios_entry_add(QemuOpts *opts)
switch (type) {
case 0:
- qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_type0_opts, &error_fatal);
save_opt(&type0.vendor, opts, "vendor");
save_opt(&type0.version, opts, "version");
save_opt(&type0.date, opts, "date");
@@ -1054,11 +1045,7 @@ void smbios_entry_add(QemuOpts *opts)
}
return;
case 1:
- qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_type1_opts, &error_fatal);
save_opt(&type1.manufacturer, opts, "manufacturer");
save_opt(&type1.product, opts, "product");
save_opt(&type1.version, opts, "version");
@@ -1076,11 +1063,7 @@ void smbios_entry_add(QemuOpts *opts)
}
return;
case 2:
- qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_type2_opts, &error_fatal);
save_opt(&type2.manufacturer, opts, "manufacturer");
save_opt(&type2.product, opts, "product");
save_opt(&type2.version, opts, "version");
@@ -1089,11 +1072,7 @@ void smbios_entry_add(QemuOpts *opts)
save_opt(&type2.location, opts, "location");
return;
case 3:
- qemu_opts_validate(opts, qemu_smbios_type3_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_type3_opts, &error_fatal);
save_opt(&type3.manufacturer, opts, "manufacturer");
save_opt(&type3.version, opts, "version");
save_opt(&type3.serial, opts, "serial");
@@ -1101,11 +1080,7 @@ void smbios_entry_add(QemuOpts *opts)
save_opt(&type3.sku, opts, "sku");
return;
case 4:
- qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_type4_opts, &error_fatal);
save_opt(&type4.sock_pfx, opts, "sock_pfx");
save_opt(&type4.manufacturer, opts, "manufacturer");
save_opt(&type4.version, opts, "version");
@@ -1114,11 +1089,7 @@ void smbios_entry_add(QemuOpts *opts)
save_opt(&type4.part, opts, "part");
return;
case 17:
- qemu_opts_validate(opts, qemu_smbios_type17_opts, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_opts_validate(opts, qemu_smbios_type17_opts, &error_fatal);
save_opt(&type17.loc_pfx, opts, "loc_pfx");
save_opt(&type17.bank, opts, "bank");
save_opt(&type17.manufacturer, opts, "manufacturer");
diff --git a/numa.c b/numa.c
index 1710946..425ef8d 100644
--- a/numa.c
+++ b/numa.c
@@ -450,17 +450,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
memory_region_init(mr, owner, name, ram_size);
for (i = 0; i < MAX_NODES; i++) {
- Error *local_err = NULL;
uint64_t size = numa_info[i].node_mem;
HostMemoryBackend *backend = numa_info[i].node_memdev;
if (!backend) {
continue;
}
- MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ MemoryRegion *seg = host_memory_backend_get_memory(backend,
+ &error_fatal);
if (memory_region_is_mapped(seg)) {
char *path = object_get_canonical_path_component(OBJECT(backend));
diff --git a/vl.c b/vl.c
index 5aaea77..6c2add9 100644
--- a/vl.c
+++ b/vl.c
@@ -4329,12 +4329,7 @@ int main(int argc, char **argv, char **envp)
configure_accelerator(current_machine);
if (qtest_chrdev) {
- Error *local_err = NULL;
- qtest_init(qtest_chrdev, qtest_log, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qtest_init(qtest_chrdev, qtest_log, &error_fatal);
}
machine_opts = qemu_get_machine_opts();
@@ -4345,24 +4340,14 @@ int main(int argc, char **argv, char **envp)
opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
if (opts) {
- Error *local_err = NULL;
-
boot_order = qemu_opt_get(opts, "order");
if (boot_order) {
- validate_bootdevices(boot_order, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ validate_bootdevices(boot_order, &error_fatal);
}
boot_once = qemu_opt_get(opts, "once");
if (boot_once) {
- validate_bootdevices(boot_once, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ validate_bootdevices(boot_once, &error_fatal);
}
boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 03/41] hw: Inline the qdev_prop_set_drive_nofail() wrapper
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
2016-01-13 15:42 ` [Qemu-devel] [PULL 01/41] error: Document how to accumulate multiple errors Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 02/41] Use error_fatal to simplify obvious fatal errors Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 04/41] hw: Don't use hw_error() for machine initialization errors Markus Armbruster
` (38 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1449764955-10741-3-git-send-email-armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/nseries.c | 4 ++--
hw/block/fdc.c | 15 ++++++++++-----
hw/block/nand.c | 2 +-
hw/core/qdev-properties-system.c | 6 ------
hw/ide/qdev.c | 3 ++-
hw/isa/pc87312.c | 8 ++++----
hw/ppc/spapr.c | 3 ++-
include/hw/qdev-properties.h | 2 --
8 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 2a8835e..57170ae 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -172,8 +172,8 @@ static void n8x0_nand_setup(struct n800_s *s)
qdev_prop_set_int32(s->nand, "shift", 1);
dinfo = drive_get(IF_MTD, 0, 0);
if (dinfo) {
- qdev_prop_set_drive_nofail(s->nand, "drive",
- blk_by_legacy_dinfo(dinfo));
+ qdev_prop_set_drive(s->nand, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_init_nofail(s->nand);
sysbus_connect_irq(SYS_BUS_DEVICE(s->nand), 0,
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 4292ece..858f5f7 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2245,10 +2245,12 @@ ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
dev = DEVICE(isadev);
if (fds[0]) {
- qdev_prop_set_drive_nofail(dev, "driveA", blk_by_legacy_dinfo(fds[0]));
+ qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fds[0]),
+ &error_fatal);
}
if (fds[1]) {
- qdev_prop_set_drive_nofail(dev, "driveB", blk_by_legacy_dinfo(fds[1]));
+ qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fds[1]),
+ &error_fatal);
}
qdev_init_nofail(dev);
@@ -2268,10 +2270,12 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
fdctrl = &sys->state;
fdctrl->dma_chann = dma_chann; /* FIXME */
if (fds[0]) {
- qdev_prop_set_drive_nofail(dev, "driveA", blk_by_legacy_dinfo(fds[0]));
+ qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fds[0]),
+ &error_fatal);
}
if (fds[1]) {
- qdev_prop_set_drive_nofail(dev, "driveB", blk_by_legacy_dinfo(fds[1]));
+ qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fds[1]),
+ &error_fatal);
}
qdev_init_nofail(dev);
sbd = SYS_BUS_DEVICE(dev);
@@ -2287,7 +2291,8 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
dev = qdev_create(NULL, "SUNW,fdtwo");
if (fds[0]) {
- qdev_prop_set_drive_nofail(dev, "drive", blk_by_legacy_dinfo(fds[0]));
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(fds[0]),
+ &error_fatal);
}
qdev_init_nofail(dev);
sys = SYSBUS_FDC(dev);
diff --git a/hw/block/nand.c b/hw/block/nand.c
index f0e3413..478e1a6 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -635,7 +635,7 @@ DeviceState *nand_init(BlockBackend *blk, int manf_id, int chip_id)
qdev_prop_set_uint8(dev, "manufacturer_id", manf_id);
qdev_prop_set_uint8(dev, "chip_id", chip_id);
if (blk) {
- qdev_prop_set_drive_nofail(dev, "drive", blk);
+ qdev_prop_set_drive(dev, "drive", blk, &error_fatal);
}
qdev_init_nofail(dev);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index d515e99..1589aba 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -364,12 +364,6 @@ void qdev_prop_set_drive(DeviceState *dev, const char *name,
name, errp);
}
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
- BlockBackend *value)
-{
- qdev_prop_set_drive(dev, name, value, &error_fatal);
-}
-
void qdev_prop_set_chr(DeviceState *dev, const char *name,
CharDriverState *value)
{
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 788b361..1f83109 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -118,7 +118,8 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
dev = qdev_create(&bus->qbus, drive->media_cd ? "ide-cd" : "ide-hd");
qdev_prop_set_uint32(dev, "unit", unit);
- qdev_prop_set_drive_nofail(dev, "drive", blk_by_legacy_dinfo(drive));
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(drive),
+ &error_fatal);
qdev_init_nofail(dev);
return DO_UPCAST(IDEDevice, qdev, dev);
}
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 3b1fcec..3803065 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -324,14 +324,14 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
/* FIXME use a qdev drive property instead of drive_get() */
drive = drive_get(IF_FLOPPY, 0, 0);
if (drive != NULL) {
- qdev_prop_set_drive_nofail(d, "driveA",
- blk_by_legacy_dinfo(drive));
+ qdev_prop_set_drive(d, "driveA", blk_by_legacy_dinfo(drive),
+ &error_fatal);
}
/* FIXME use a qdev drive property instead of drive_get() */
drive = drive_get(IF_FLOPPY, 0, 1);
if (drive != NULL) {
- qdev_prop_set_drive_nofail(d, "driveB",
- blk_by_legacy_dinfo(drive));
+ qdev_prop_set_drive(d, "driveB", blk_by_legacy_dinfo(drive),
+ &error_fatal);
}
qdev_init_nofail(d);
s->fdc.dev = isa;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 414e0f9b..0ca0176 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1216,7 +1216,8 @@ static void spapr_create_nvram(sPAPRMachineState *spapr)
DriveInfo *dinfo = drive_get(IF_PFLASH, 0, 0);
if (dinfo) {
- qdev_prop_set_drive_nofail(dev, "drive", blk_by_legacy_dinfo(dinfo));
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_init_nofail(dev);
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 77538a8..254afd8 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -180,8 +180,6 @@ void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *valu
void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value);
void qdev_prop_set_drive(DeviceState *dev, const char *name,
BlockBackend *value, Error **errp);
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
- BlockBackend *value);
void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
/* FIXME: Remove opaque pointer properties. */
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 04/41] hw: Don't use hw_error() for machine initialization errors
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (2 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 03/41] hw: Inline the qdev_prop_set_drive_nofail() wrapper Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 05/41] omap: Don't use hw_error() in device init() methods Markus Armbruster
` (37 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Cc: Guan Xuetao, qemu-arm, qemu-ppc, Markus Armbruster,
Richard Henderson
Printing CPU registers is not helpful during machine initialization.
Moreover, these are straightforward configuration or "can get
resources" errors, so dumping core isn't appropriate either. Replace
hw_error() by error_report(); exit(1). Matches how we report these
errors in other machine initializations.
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1450370121-5768-2-git-send-email-armbru@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
hw/alpha/dp264.c | 11 ++++++-----
hw/arm/highbank.c | 6 ++++--
hw/char/exynos4210_uart.c | 9 ++++++---
hw/m68k/an5206.c | 4 +++-
hw/ppc/mac_newworld.c | 11 ++++++-----
hw/ppc/mac_oldworld.c | 16 +++++++++-------
hw/ppc/prep.c | 11 +++++++----
hw/unicore32/puv3.c | 10 +++++++---
8 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 27bdaa1..38b85ba 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -11,6 +11,7 @@
#include "hw/loader.h"
#include "hw/boards.h"
#include "alpha_sys.h"
+#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
#include "hw/timer/mc146818rtc.h"
#include "hw/ide.h"
@@ -104,14 +105,14 @@ static void clipper_init(MachineState *machine)
palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS,
bios_name ? bios_name : "palcode-clipper");
if (palcode_filename == NULL) {
- hw_error("no palcode provided\n");
+ error_report("no palcode provided");
exit(1);
}
size = load_elf(palcode_filename, cpu_alpha_superpage_to_phys,
NULL, &palcode_entry, &palcode_low, &palcode_high,
0, EM_ALPHA, 0);
if (size < 0) {
- hw_error("could not load palcode '%s'\n", palcode_filename);
+ error_report("could not load palcode '%s'", palcode_filename);
exit(1);
}
g_free(palcode_filename);
@@ -131,7 +132,7 @@ static void clipper_init(MachineState *machine)
NULL, &kernel_entry, &kernel_low, &kernel_high,
0, EM_ALPHA, 0);
if (size < 0) {
- hw_error("could not load kernel '%s'\n", kernel_filename);
+ error_report("could not load kernel '%s'", kernel_filename);
exit(1);
}
@@ -148,8 +149,8 @@ static void clipper_init(MachineState *machine)
initrd_size = get_image_size(initrd_filename);
if (initrd_size < 0) {
- hw_error("could not load initial ram disk '%s'\n",
- initrd_filename);
+ error_report("could not load initial ram disk '%s'",
+ initrd_filename);
exit(1);
}
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index a0a5a06..cb9926e 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -315,11 +315,13 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
if (sysboot_filename != NULL) {
if (load_image_targphys(sysboot_filename, 0xfff88000, 0x8000) < 0) {
- hw_error("Unable to load %s\n", bios_name);
+ error_report("Unable to load %s", bios_name);
+ exit(1);
}
g_free(sysboot_filename);
} else {
- hw_error("Unable to find %s\n", bios_name);
+ error_report("Unable to find %s", bios_name);
+ exit(1);
}
}
diff --git a/hw/char/exynos4210_uart.c b/hw/char/exynos4210_uart.c
index 215f962..2736b37 100644
--- a/hw/char/exynos4210_uart.c
+++ b/hw/char/exynos4210_uart.c
@@ -20,6 +20,7 @@
*/
#include "hw/sysbus.h"
+#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
#include "sysemu/char.h"
@@ -595,15 +596,17 @@ DeviceState *exynos4210_uart_create(hwaddr addr,
if (!chr) {
if (channel >= MAX_SERIAL_PORTS) {
- hw_error("Only %d serial ports are supported by QEMU.\n",
- MAX_SERIAL_PORTS);
+ error_report("Only %d serial ports are supported by QEMU",
+ MAX_SERIAL_PORTS);
+ exit(1);
}
chr = serial_hds[channel];
if (!chr) {
snprintf(label, ARRAY_SIZE(label), "%s%d", chr_name, channel);
chr = qemu_chr_new(label, "null", NULL);
if (!(chr)) {
- hw_error("Can't assign serial port to UART%d.\n", channel);
+ error_report("Can't assign serial port to UART%d", channel);
+ exit(1);
}
}
}
diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index c1dea17..8d9ccaa 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -12,6 +12,7 @@
#include "hw/loader.h"
#include "elf.h"
#include "exec/address-spaces.h"
+#include "qemu/error-report.h"
#include "sysemu/qtest.h"
#define KERNEL_LOAD_ADDR 0x10000
@@ -39,7 +40,8 @@ static void an5206_init(MachineState *machine)
}
cpu = cpu_m68k_init(cpu_model);
if (!cpu) {
- hw_error("Unable to find m68k CPU definition\n");
+ error_report("Unable to find m68k CPU definition");
+ exit(1);
}
env = &cpu->env;
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 1b9a573..ca3d6a8 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -62,6 +62,7 @@
#include "hw/ide.h"
#include "hw/loader.h"
#include "elf.h"
+#include "qemu/error-report.h"
#include "sysemu/kvm.h"
#include "kvm_ppc.h"
#include "hw/usb.h"
@@ -226,7 +227,7 @@ static void ppc_core99_init(MachineState *machine)
bios_size = -1;
}
if (bios_size < 0 || bios_size > BIOS_SIZE) {
- hw_error("qemu: could not load PowerPC bios '%s'\n", bios_name);
+ error_report("could not load PowerPC bios '%s'", bios_name);
exit(1);
}
@@ -252,7 +253,7 @@ static void ppc_core99_init(MachineState *machine)
kernel_base,
ram_size - kernel_base);
if (kernel_size < 0) {
- hw_error("qemu: could not load kernel '%s'\n", kernel_filename);
+ error_report("could not load kernel '%s'", kernel_filename);
exit(1);
}
/* load initrd */
@@ -261,8 +262,8 @@ static void ppc_core99_init(MachineState *machine)
initrd_size = load_image_targphys(initrd_filename, initrd_base,
ram_size - initrd_base);
if (initrd_size < 0) {
- hw_error("qemu: could not load initial ram disk '%s'\n",
- initrd_filename);
+ error_report("could not load initial ram disk '%s'",
+ initrd_filename);
exit(1);
}
cmdline_base = round_page(initrd_base + initrd_size);
@@ -344,7 +345,7 @@ static void ppc_core99_init(MachineState *machine)
break;
#endif /* defined(TARGET_PPC64) */
default:
- hw_error("Bus model not supported on mac99 machine\n");
+ error_report("Bus model not supported on mac99 machine");
exit(1);
}
}
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 21eaf0e..c3f9fe3 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -38,6 +38,7 @@
#include "hw/ide.h"
#include "hw/loader.h"
#include "elf.h"
+#include "qemu/error-report.h"
#include "sysemu/kvm.h"
#include "kvm_ppc.h"
#include "sysemu/block-backend.h"
@@ -153,7 +154,7 @@ static void ppc_heathrow_init(MachineState *machine)
bios_size = -1;
}
if (bios_size < 0 || bios_size > BIOS_SIZE) {
- hw_error("qemu: could not load PowerPC bios '%s'\n", bios_name);
+ error_report("could not load PowerPC bios '%s'", bios_name);
exit(1);
}
@@ -178,8 +179,7 @@ static void ppc_heathrow_init(MachineState *machine)
kernel_base,
ram_size - kernel_base);
if (kernel_size < 0) {
- hw_error("qemu: could not load kernel '%s'\n",
- kernel_filename);
+ error_report("could not load kernel '%s'", kernel_filename);
exit(1);
}
/* load initrd */
@@ -188,8 +188,8 @@ static void ppc_heathrow_init(MachineState *machine)
initrd_size = load_image_targphys(initrd_filename, initrd_base,
ram_size - initrd_base);
if (initrd_size < 0) {
- hw_error("qemu: could not load initial ram disk '%s'\n",
- initrd_filename);
+ error_report("could not load initial ram disk '%s'",
+ initrd_filename);
exit(1);
}
cmdline_base = round_page(initrd_base + initrd_size);
@@ -246,7 +246,8 @@ static void ppc_heathrow_init(MachineState *machine)
((qemu_irq *)env->irq_inputs)[PPC6xx_INPUT_INT];
break;
default:
- hw_error("Bus model not supported on OldWorld Mac machine\n");
+ error_report("Bus model not supported on OldWorld Mac machine");
+ exit(1);
}
}
@@ -259,7 +260,8 @@ static void ppc_heathrow_init(MachineState *machine)
/* init basic PC hardware */
if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
- hw_error("Only 6xx bus is supported on heathrow machine\n");
+ error_report("Only 6xx bus is supported on heathrow machine");
+ exit(1);
}
pic = heathrow_pic_init(&pic_mem, 1, heathrow_irqs);
pci_bus = pci_grackle_init(0xfec00000, pic,
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 5ad28f7..0e102fc 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -33,6 +33,7 @@
#include "hw/pci/pci_host.h"
#include "hw/ppc/ppc.h"
#include "hw/boards.h"
+#include "qemu/error-report.h"
#include "qemu/log.h"
#include "hw/ide.h"
#include "hw/loader.h"
@@ -532,7 +533,7 @@ static void ppc_prep_init(MachineState *machine)
kernel_size = load_image_targphys(kernel_filename, kernel_base,
ram_size - kernel_base);
if (kernel_size < 0) {
- hw_error("qemu: could not load kernel '%s'\n", kernel_filename);
+ error_report("could not load kernel '%s'", kernel_filename);
exit(1);
}
/* load initrd */
@@ -541,8 +542,9 @@ static void ppc_prep_init(MachineState *machine)
initrd_size = load_image_targphys(initrd_filename, initrd_base,
ram_size - initrd_base);
if (initrd_size < 0) {
- hw_error("qemu: could not load initial ram disk '%s'\n",
- initrd_filename);
+ error_report("could not load initial ram disk '%s'",
+ initrd_filename);
+ exit(1);
}
} else {
initrd_base = 0;
@@ -569,7 +571,8 @@ static void ppc_prep_init(MachineState *machine)
}
if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
- hw_error("Only 6xx bus is supported on PREP machine\n");
+ error_report("Only 6xx bus is supported on PREP machine");
+ exit(1);
}
dev = qdev_create(NULL, "raven-pcihost");
diff --git a/hw/unicore32/puv3.c b/hw/unicore32/puv3.c
index 91117b2..1968202 100644
--- a/hw/unicore32/puv3.c
+++ b/hw/unicore32/puv3.c
@@ -17,6 +17,7 @@
#include "hw/boards.h"
#include "hw/loader.h"
#include "hw/i386/pc.h"
+#include "qemu/error-report.h"
#include "sysemu/qtest.h"
#undef DEBUG_PUV3
@@ -95,7 +96,8 @@ static void puv3_load_kernel(const char *kernel_filename)
size = load_image_targphys(kernel_filename, KERNEL_LOAD_ADDR,
KERNEL_MAX_SIZE);
if (size < 0) {
- hw_error("Load kernel error: '%s'\n", kernel_filename);
+ error_report("Load kernel error: '%s'", kernel_filename);
+ exit(1);
}
/* cheat curses that we have a graphic console, only under ocd console */
@@ -112,7 +114,8 @@ static void puv3_init(MachineState *machine)
UniCore32CPU *cpu;
if (initrd_filename) {
- hw_error("Please use kernel built-in initramdisk.\n");
+ error_report("Please use kernel built-in initramdisk");
+ exit(1);
}
if (!cpu_model) {
@@ -121,7 +124,8 @@ static void puv3_init(MachineState *machine)
cpu = uc32_cpu_init(cpu_model);
if (!cpu) {
- hw_error("Unable to find CPU definition\n");
+ error_report("Unable to find CPU definition");
+ exit(1);
}
env = &cpu->env;
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 05/41] omap: Don't use hw_error() in device init() methods
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (3 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 04/41] hw: Don't use hw_error() for machine initialization errors Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 06/41] arm_mptimer: Don't use hw_error() in realize() method Markus Armbruster
` (36 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Markus Armbruster
Device init() methods aren't supposed to call hw_error(), they should
report the error and fail cleanly. Do that.
The errors are all device misconfiguration. All callers use
qdev_init_nofail(), so this patch merely converts hw_error() crashes
into &error_abort crashes. Improvement, because now it crashes closer
to where the misconfiguration bug would be, and a few more bad
examples of hw_error() use are gone.
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <1450370121-5768-3-git-send-email-armbru@redhat.com>
---
hw/gpio/omap_gpio.c | 29 +++++++++++++++++++++--------
hw/i2c/omap_i2c.c | 8 ++++++--
hw/intc/omap_intc.c | 10 +++++++---
3 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/hw/gpio/omap_gpio.c b/hw/gpio/omap_gpio.c
index 3c53898..63d8b42 100644
--- a/hw/gpio/omap_gpio.c
+++ b/hw/gpio/omap_gpio.c
@@ -21,6 +21,7 @@
#include "hw/hw.h"
#include "hw/arm/omap.h"
#include "hw/sysbus.h"
+#include "qemu/error-report.h"
struct omap_gpio_s {
qemu_irq irq;
@@ -682,7 +683,8 @@ static int omap_gpio_init(SysBusDevice *sbd)
struct omap_gpif_s *s = OMAP1_GPIO(dev);
if (!s->clk) {
- hw_error("omap-gpio: clk not connected\n");
+ error_report("omap-gpio: clk not connected");
+ return -1;
}
qdev_init_gpio_in(dev, omap_gpio_set, 16);
qdev_init_gpio_out(dev, s->omap1.handler, 16);
@@ -700,25 +702,35 @@ static int omap2_gpio_init(SysBusDevice *sbd)
int i;
if (!s->iclk) {
- hw_error("omap2-gpio: iclk not connected\n");
+ error_report("omap2-gpio: iclk not connected");
+ return -1;
}
+
+ s->modulecount = s->mpu_model < omap2430 ? 4
+ : s->mpu_model < omap3430 ? 5
+ : 6;
+
+ for (i = 0; i < s->modulecount; i++) {
+ if (!s->fclk[i]) {
+ error_report("omap2-gpio: fclk%d not connected", i);
+ return -1;
+ }
+ }
+
if (s->mpu_model < omap3430) {
- s->modulecount = (s->mpu_model < omap2430) ? 4 : 5;
memory_region_init_io(&s->iomem, OBJECT(s), &omap2_gpif_top_ops, s,
"omap2.gpio", 0x1000);
sysbus_init_mmio(sbd, &s->iomem);
- } else {
- s->modulecount = 6;
}
+
s->modules = g_new0(struct omap2_gpio_s, s->modulecount);
s->handler = g_new0(qemu_irq, s->modulecount * 32);
qdev_init_gpio_in(dev, omap2_gpio_set, s->modulecount * 32);
qdev_init_gpio_out(dev, s->handler, s->modulecount * 32);
+
for (i = 0; i < s->modulecount; i++) {
struct omap2_gpio_s *m = &s->modules[i];
- if (!s->fclk[i]) {
- hw_error("omap2-gpio: fclk%d not connected\n", i);
- }
+
m->revision = (s->mpu_model < omap3430) ? 0x18 : 0x25;
m->handler = &s->handler[i * 32];
sysbus_init_irq(sbd, &m->irq[0]); /* mpu irq */
@@ -728,6 +740,7 @@ static int omap2_gpio_init(SysBusDevice *sbd)
"omap.gpio-module", 0x1000);
sysbus_init_mmio(sbd, &m->iomem);
}
+
return 0;
}
diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c
index b6f544a..8b0b146 100644
--- a/hw/i2c/omap_i2c.c
+++ b/hw/i2c/omap_i2c.c
@@ -20,6 +20,7 @@
#include "hw/i2c/i2c.h"
#include "hw/arm/omap.h"
#include "hw/sysbus.h"
+#include "qemu/error-report.h"
#define TYPE_OMAP_I2C "omap_i2c"
#define OMAP_I2C(obj) OBJECT_CHECK(OMAPI2CState, (obj), TYPE_OMAP_I2C)
@@ -449,12 +450,15 @@ static int omap_i2c_init(SysBusDevice *sbd)
OMAPI2CState *s = OMAP_I2C(dev);
if (!s->fclk) {
- hw_error("omap_i2c: fclk not connected\n");
+ error_report("omap_i2c: fclk not connected");
+ return -1;
}
if (s->revision >= OMAP2_INTR_REV && !s->iclk) {
/* Note that OMAP1 doesn't have a separate interface clock */
- hw_error("omap_i2c: iclk not connected\n");
+ error_report("omap_i2c: iclk not connected");
+ return -1;
}
+
sysbus_init_irq(sbd, &s->irq);
sysbus_init_irq(sbd, &s->drq[0]);
sysbus_init_irq(sbd, &s->drq[1]);
diff --git a/hw/intc/omap_intc.c b/hw/intc/omap_intc.c
index e9b38a3..07b6272 100644
--- a/hw/intc/omap_intc.c
+++ b/hw/intc/omap_intc.c
@@ -20,6 +20,7 @@
#include "hw/hw.h"
#include "hw/arm/omap.h"
#include "hw/sysbus.h"
+#include "qemu/error-report.h"
/* Interrupt Handlers */
struct omap_intr_handler_bank_s {
@@ -367,7 +368,8 @@ static int omap_intc_init(SysBusDevice *sbd)
struct omap_intr_handler_s *s = OMAP_INTC(dev);
if (!s->iclk) {
- hw_error("omap-intc: clk not connected\n");
+ error_report("omap-intc: clk not connected");
+ return -1;
}
s->nbanks = 1;
sysbus_init_irq(sbd, &s->parent_intr[0]);
@@ -608,10 +610,12 @@ static int omap2_intc_init(SysBusDevice *sbd)
struct omap_intr_handler_s *s = OMAP_INTC(dev);
if (!s->iclk) {
- hw_error("omap2-intc: iclk not connected\n");
+ error_report("omap2-intc: iclk not connected");
+ return -1;
}
if (!s->fclk) {
- hw_error("omap2-intc: fclk not connected\n");
+ error_report("omap2-intc: fclk not connected");
+ return -1;
}
s->level_only = 1;
s->nbanks = 3;
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 06/41] arm_mptimer: Don't use hw_error() in realize() method
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (4 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 05/41] omap: Don't use hw_error() in device init() methods Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 07/41] etraxfs_eth: Don't use hw_error() in init() method Markus Armbruster
` (35 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm
Device realize() methods aren't supposed to call hw_error(), they
should set an error and fail cleanly. Do that.
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <1450370121-5768-4-git-send-email-armbru@redhat.com>
---
hw/timer/arm_mptimer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 3e59c2a..5dfab66 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -220,8 +220,9 @@ static void arm_mptimer_realize(DeviceState *dev, Error **errp)
int i;
if (s->num_cpu < 1 || s->num_cpu > ARM_MPTIMER_MAX_CPUS) {
- hw_error("%s: num-cpu must be between 1 and %d\n",
- __func__, ARM_MPTIMER_MAX_CPUS);
+ error_setg(errp, "num-cpu must be between 1 and %d",
+ ARM_MPTIMER_MAX_CPUS);
+ return;
}
/* We implement one timer block per CPU, and expose multiple MMIO regions:
* * region 0 is "timer for this core"
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 07/41] etraxfs_eth: Don't use hw_error() in init() method
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (5 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 06/41] arm_mptimer: Don't use hw_error() in realize() method Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 08/41] raven: Mark use of hw_error() in realize() FIXME Markus Armbruster
` (34 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Edgar E. Iglesias, Markus Armbruster
Device init() methods aren't supposed to call hw_error(), they should
report the error and fail cleanly. Do that.
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Message-Id: <1450370121-5768-5-git-send-email-armbru@redhat.com>
---
hw/net/etraxfs_eth.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index d600275..b562ac9 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -26,6 +26,7 @@
#include "hw/sysbus.h"
#include "net/net.h"
#include "hw/cris/etraxfs.h"
+#include "qemu/error-report.h"
#define D(x)
@@ -589,7 +590,8 @@ static int fs_eth_init(SysBusDevice *sbd)
ETRAXFSEthState *s = ETRAX_FS_ETH(dev);
if (!s->dma_out || !s->dma_in) {
- hw_error("Unconnected ETRAX-FS Ethernet MAC.\n");
+ error_report("Unconnected ETRAX-FS Ethernet MAC");
+ return -1;
}
s->dma_out->client.push = eth_tx_push;
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 08/41] raven: Mark use of hw_error() in realize() FIXME
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (6 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 07/41] etraxfs_eth: Don't use hw_error() in init() method Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 09/41] error: Don't append a newline when printing the error hint Markus Armbruster
` (33 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Andreas Färber, Markus Armbruster, qemu-ppc
Device realize() methods aren't supposed to call hw_error(), they
should set an error and fail cleanly. Blindly doing that would be
easy enough, but then realize() would fail without undoing its side
effects. Just mark it FIXME for now.
Cc: "Andreas Färber" <andreas.faerber@web.de>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1450370121-5768-6-git-send-email-armbru@redhat.com>
---
hw/pci-host/prep.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index da88cb3..f434596 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -326,6 +326,7 @@ static void raven_realize(PCIDevice *d, Error **errp)
}
}
if (bios_size < 0 || bios_size > BIOS_SIZE) {
+ /* FIXME should error_setg() */
hw_error("qemu: could not load bios image '%s'\n", s->bios_name);
}
g_free(filename);
@@ -355,8 +356,9 @@ static void raven_class_init(ObjectClass *klass, void *data)
dc->desc = "PReP Host Bridge - Motorola Raven";
dc->vmsd = &vmstate_raven;
/*
- * PCI-facing part of the host bridge, not usable without the
- * host-facing part, which can't be device_add'ed, yet.
+ * Reason: PCI-facing part of the host bridge, not usable without
+ * the host-facing part, which can't be device_add'ed, yet.
+ * Reason: realize() method uses hw_error().
*/
dc->cannot_instantiate_with_device_add_yet = true;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 09/41] error: Don't append a newline when printing the error hint
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (7 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 08/41] raven: Mark use of hw_error() in realize() FIXME Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 10/41] hw/arm/virt: Fix property "gic-version" error handling Markus Armbruster
` (32 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Since commit 50b7b00, we have error_append_hint() to conveniently
accumulate Error member @hint. error_report_err() prints it with a
newline appended. Consequently, users of error_append_hint() need to
know whether theirs is the final line of the hint to decide whether it
needs a newline. Not a nice interface.
Change error_report_err() to print just the hint, and the (still few)
users of error_append_hint() to add the required newline.
Cc: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450370121-5768-7-git-send-email-armbru@redhat.com>
---
qdev-monitor.c | 2 ++
util/error.c | 2 +-
util/qemu-option.c | 4 ++--
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a35098f..30936df 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -304,6 +304,7 @@ static void qbus_list_bus(DeviceState *dev, Error **errp)
error_append_hint(errp, "%s\"%s\"", sep, child->name);
sep = ", ";
}
+ error_append_hint(errp, "\n");
}
static void qbus_list_dev(BusState *bus, Error **errp)
@@ -321,6 +322,7 @@ static void qbus_list_dev(BusState *bus, Error **errp)
}
sep = ", ";
}
+ error_append_hint(errp, "\n");
}
static BusState *qbus_find_bus(DeviceState *dev, char *elem)
diff --git a/util/error.c b/util/error.c
index 80c89a2..9b27c45 100644
--- a/util/error.c
+++ b/util/error.c
@@ -204,7 +204,7 @@ void error_report_err(Error *err)
{
error_report("%s", error_get_pretty(err));
if (err->hint) {
- error_printf_unless_qmp("%s\n", err->hint->str);
+ error_printf_unless_qmp("%s", err->hint->str);
}
error_free(err);
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a50ecea..a2d593a 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -206,7 +206,7 @@ void parse_option_size(const char *name, const char *value,
default:
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
error_append_hint(errp, "You may use k, M, G or T suffixes for "
- "kilobytes, megabytes, gigabytes and terabytes.");
+ "kilobytes, megabytes, gigabytes and terabytes.\n");
return;
}
} else {
@@ -647,7 +647,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id",
"an identifier");
error_append_hint(errp, "Identifiers consist of letters, digits, "
- "'-', '.', '_', starting with a letter.");
+ "'-', '.', '_', starting with a letter.\n");
return NULL;
}
opts = qemu_opts_find(list, id);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 10/41] hw/arm/virt: Fix property "gic-version" error handling
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (8 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 09/41] error: Don't append a newline when printing the error hint Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 11/41] sysbus: Don't use hw_error() in machine_init_done_notifiers Markus Armbruster
` (31 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm
virt_set_gic_version() calls exit(1) when passed an invalid property
value. Property setters are not supposed to do that. Screwed up in
commit b92ad39. Harmless, because the property belongs to a machine.
Set an error object instead.
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index fd52b76..92dcd02 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1200,9 +1200,8 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
} else if (!strcmp(value, "host")) {
vms->gic_version = 0; /* Will probe later */
} else {
- error_report("Invalid gic-version option value");
- error_printf("Allowed gic-version values are: 3, 2, host\n");
- exit(1);
+ error_setg(errp, "Invalid gic-version value");
+ error_append_hint(errp, "Valid values are 3, 2, host.\n");
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 11/41] sysbus: Don't use hw_error() in machine_init_done_notifiers
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (9 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 10/41] hw/arm/virt: Fix property "gic-version" error handling Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 12/41] isa: Trivially convert remaining PCI-ISA bridges to realize() Markus Armbruster
` (30 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Graf
platform_bus_map_irq() and platform_bus_map_mmio() use hw_error() to
fail. They run in machine_init_done_notifiers, via
platform_bus_init_notify() and link_sysbus_device(). Printing CPU
registers is not helpful there.
Replace hw_error() by error_report(); exit(1). If these are
programming errors, it should be replaced by an assertion instead.
While there, observe that both functions always return 0, and
link_sysbus_device() ignores the return value. Change them to void.
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1450370121-5768-9-git-send-email-armbru@redhat.com>
---
hw/core/platform-bus.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/hw/core/platform-bus.c b/hw/core/platform-bus.c
index 70e0518..aa55d01 100644
--- a/hw/core/platform-bus.c
+++ b/hw/core/platform-bus.c
@@ -21,6 +21,7 @@
#include "hw/platform-bus.h"
#include "exec/address-spaces.h"
+#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
@@ -106,31 +107,29 @@ static void plaform_bus_refresh_irqs(PlatformBusDevice *pbus)
pbus->done_gathering = true;
}
-static int platform_bus_map_irq(PlatformBusDevice *pbus, SysBusDevice *sbdev,
- int n)
+static void platform_bus_map_irq(PlatformBusDevice *pbus, SysBusDevice *sbdev,
+ int n)
{
int max_irqs = pbus->num_irqs;
int irqn;
if (sysbus_is_irq_connected(sbdev, n)) {
/* IRQ is already mapped, nothing to do */
- return 0;
+ return;
}
irqn = find_first_zero_bit(pbus->used_irqs, max_irqs);
if (irqn >= max_irqs) {
- hw_error("Platform Bus: Can not fit IRQ line");
- return -1;
+ error_report("Platform Bus: Can not fit IRQ line");
+ exit(1);
}
set_bit(irqn, pbus->used_irqs);
sysbus_connect_irq(sbdev, n, pbus->irqs[irqn]);
-
- return 0;
}
-static int platform_bus_map_mmio(PlatformBusDevice *pbus, SysBusDevice *sbdev,
- int n)
+static void platform_bus_map_mmio(PlatformBusDevice *pbus, SysBusDevice *sbdev,
+ int n)
{
MemoryRegion *sbdev_mr = sysbus_mmio_get_region(sbdev, n);
uint64_t size = memory_region_size(sbdev_mr);
@@ -140,7 +139,7 @@ static int platform_bus_map_mmio(PlatformBusDevice *pbus, SysBusDevice *sbdev,
if (memory_region_is_mapped(sbdev_mr)) {
/* Region is already mapped, nothing to do */
- return 0;
+ return;
}
/*
@@ -155,13 +154,13 @@ static int platform_bus_map_mmio(PlatformBusDevice *pbus, SysBusDevice *sbdev,
}
if (!found_region) {
- hw_error("Platform Bus: Can not fit MMIO region of size %"PRIx64, size);
+ error_report("Platform Bus: Can not fit MMIO region of size %"PRIx64,
+ size);
+ exit(1);
}
/* Map the device's region into our Platform Bus MMIO space */
memory_region_add_subregion(&pbus->mmio, off, sbdev_mr);
-
- return 0;
}
/*
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 12/41] isa: Trivially convert remaining PCI-ISA bridges to realize()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (10 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 11/41] sysbus: Don't use hw_error() in machine_init_done_notifiers Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 13/41] isa: Clean up error handling around isa_bus_new() Markus Armbruster
` (29 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Mark Cave-Ayland, Michael S. Tsirkin
These are "ICH9-LPC" and "ebus".
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <1450370121-5768-10-git-send-email-armbru@redhat.com>
---
hw/isa/lpc_ich9.c | 5 ++---
hw/sparc64/sun4u.c | 6 ++----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 1ffc803..8e58449 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -602,7 +602,7 @@ static void ich9_lpc_initfn(Object *obj)
ich9_lpc_add_properties(lpc);
}
-static int ich9_lpc_init(PCIDevice *d)
+static void ich9_lpc_realize(PCIDevice *d, Error **errp)
{
ICH9LPCState *lpc = ICH9_LPC_DEVICE(d);
ISABus *isa_bus;
@@ -628,7 +628,6 @@ static int ich9_lpc_init(PCIDevice *d)
memory_region_add_subregion_overlap(pci_address_space_io(d),
ICH9_RST_CNT_IOPORT, &lpc->rst_cnt_mem,
1);
- return 0;
}
static void ich9_device_plug_cb(HotplugHandler *hotplug_dev,
@@ -706,7 +705,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data)
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
dc->reset = ich9_lpc_reset;
- k->init = ich9_lpc_init;
+ k->realize = ich9_lpc_realize;
dc->vmsd = &vmstate_ich9_lpc;
dc->props = ich9_lpc_properties;
k->config_write = ich9_lpc_config_write;
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 7a433d3..8058aac 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -600,8 +600,7 @@ pci_ebus_init(PCIBus *bus, int devfn, qemu_irq *irqs)
return isa_bus;
}
-static int
-pci_ebus_init1(PCIDevice *pci_dev)
+static void pci_ebus_realize(PCIDevice *pci_dev, Error **errp)
{
EbusState *s = DO_UPCAST(EbusState, pci_dev, pci_dev);
@@ -621,14 +620,13 @@ pci_ebus_init1(PCIDevice *pci_dev)
memory_region_init_alias(&s->bar1, OBJECT(s), "bar1", get_system_io(),
0, 0x4000);
pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar1);
- return 0;
}
static void ebus_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = pci_ebus_init1;
+ k->realize = pci_ebus_realize;
k->vendor_id = PCI_VENDOR_ID_SUN;
k->device_id = PCI_DEVICE_ID_SUN_EBUS;
k->revision = 0x01;
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 13/41] isa: Clean up error handling around isa_bus_new()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (11 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 12/41] isa: Trivially convert remaining PCI-ISA bridges to realize() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 14/41] isa: Clean up inappropriate hw_error() Markus Armbruster
` (28 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Mark Cave-Ayland,
Hervé Poussineau, Aurelien Jarno, Richard Henderson
We can have at most one ISA bus. If you try to create another one,
isa_bus_new() complains to stderr and returns null.
isa_bus_new() is called in two contexts, machine's init() and device's
realize() methods. Since complaining to stderr is not proper in the
latter context, convert isa_bus_new() to Error.
Machine's init():
* mips_jazz_init(), called from the init() methods of machines
"magnum" and "pica"
* mips_r4k_init(), the init() method of machine "mips"
* pc_init1() called from the init() methods of non-q35 PC machines
* typhoon_init(), called from clipper_init(), the init() method of
machine "clipper"
These callers always create the first ISA bus, hence isa_bus_new()
can't fail. Simply pass &error_abort.
Device's realize():
* i82378_realize(), of PCI device "i82378"
* ich9_lpc_realize(), of PCI device "ICH9-LPC"
* pci_ebus_realize(), of PCI device "ebus"
* piix3_realize(), of PCI device "pci-piix3", abstract parent of
"PIIX3" and "PIIX3-xen"
* piix4_realize(), of PCI device "PIIX4"
* vt82c686b_realize(), of PCI device "VT82C686B"
Propagate the error. Note that these devices are typically created
only by machine init() methods with qdev_init_nofail() or similar. If
we screwed up and created an ISA bus before that call, we now give up
right away. Before, we'd hobble on, and typically die in
isa_bus_irqs(). Similar if someone finds a way to hot-plug one of
these critters.
Cc: Richard Henderson <rth@twiddle.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <1450370121-5768-11-git-send-email-armbru@redhat.com>
---
hw/alpha/typhoon.c | 3 ++-
hw/i386/pc_piix.c | 3 ++-
hw/isa/i82378.c | 5 ++++-
hw/isa/isa-bus.c | 4 ++--
hw/isa/lpc_ich9.c | 6 +++++-
hw/isa/piix4.c | 6 ++++--
hw/isa/vt82c686.c | 5 ++++-
hw/mips/mips_jazz.c | 2 +-
hw/mips/mips_r4k.c | 2 +-
hw/pci-host/piix.c | 6 ++++--
hw/sparc64/sun4u.c | 6 ++++--
include/hw/isa/isa.h | 2 +-
12 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 421162e..35dc8a5 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -920,7 +920,8 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
{
qemu_irq *isa_irqs;
- *isa_bus = isa_bus_new(NULL, get_system_memory(), &s->pchip.reg_io);
+ *isa_bus = isa_bus_new(NULL, get_system_memory(), &s->pchip.reg_io,
+ &error_abort);
isa_irqs = i8259_init(*isa_bus,
qemu_allocate_irq(typhoon_set_isa_irq, s, 0));
isa_bus_irqs(*isa_bus, isa_irqs);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 438cdae..df2b824 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -189,7 +189,8 @@ static void pc_init1(MachineState *machine,
} else {
pci_bus = NULL;
i440fx_state = NULL;
- isa_bus = isa_bus_new(NULL, get_system_memory(), system_io);
+ isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
+ &error_abort);
no_hpet = 1;
}
isa_bus_irqs(isa_bus, gsi);
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index d4c8306..3793c6f 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -75,7 +75,10 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
pci_config_set_interrupt_pin(pci_conf, 1); /* interrupt pin 0 */
isabus = isa_bus_new(dev, get_system_memory(),
- pci_address_space_io(pci));
+ pci_address_space_io(pci), errp);
+ if (!isabus) {
+ return;
+ }
/* This device has:
2 82C59 (irq)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 43e0cd8..af6ffd6 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -44,10 +44,10 @@ static const TypeInfo isa_bus_info = {
};
ISABus *isa_bus_new(DeviceState *dev, MemoryRegion* address_space,
- MemoryRegion *address_space_io)
+ MemoryRegion *address_space_io, Error **errp)
{
if (isabus) {
- fprintf(stderr, "Can't create a second ISA bus\n");
+ error_setg(errp, "Can't create a second ISA bus");
return NULL;
}
if (!dev) {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 8e58449..ed9907d 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -607,7 +607,11 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
ICH9LPCState *lpc = ICH9_LPC_DEVICE(d);
ISABus *isa_bus;
- isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), get_system_io());
+ isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), get_system_io(),
+ errp);
+ if (!isa_bus) {
+ return;
+ }
pci_set_long(d->wmask + ICH9_LPC_PMBASE,
ICH9_LPC_PMBASE_BASE_ADDRESS_MASK);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 2c59e91..644cfd9 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -90,8 +90,10 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
{
PIIX4State *d = PIIX4_PCI_DEVICE(dev);
- isa_bus_new(DEVICE(d), pci_address_space(dev),
- pci_address_space_io(dev));
+ if (!isa_bus_new(DEVICE(d), pci_address_space(dev),
+ pci_address_space_io(dev), errp)) {
+ return;
+ }
piix4_dev = &d->dev;
qemu_register_reset(piix4_reset, d);
}
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 252e1d7..6c2190b 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -440,7 +440,10 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
int i;
isa_bus = isa_bus_new(DEVICE(d), get_system_memory(),
- pci_address_space_io(d));
+ pci_address_space_io(d), errp);
+ if (!isa_bus) {
+ return;
+ }
pci_conf = d->config;
pci_config_set_prog_interface(pci_conf, 0x0);
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 1ab885b..1cfbaa6 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -219,7 +219,7 @@ static void mips_jazz_init(MachineState *machine,
memory_region_init(isa_mem, NULL, "isa-mem", 0x01000000);
memory_region_add_subregion(address_space, 0x90000000, isa_io);
memory_region_add_subregion(address_space, 0x91000000, isa_mem);
- isa_bus = isa_bus_new(NULL, isa_mem, isa_io);
+ isa_bus = isa_bus_new(NULL, isa_mem, isa_io, &error_abort);
/* ISA devices */
i8259 = i8259_init(isa_bus, env->irq[4]);
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index af10da1..2d4e038 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -272,7 +272,7 @@ void mips_r4k_init(MachineState *machine)
memory_region_init(isa_mem, NULL, "isa-mem", 0x01000000);
memory_region_add_subregion(get_system_memory(), 0x14000000, isa_io);
memory_region_add_subregion(get_system_memory(), 0x10000000, isa_mem);
- isa_bus = isa_bus_new(NULL, isa_mem, get_system_io());
+ isa_bus = isa_bus_new(NULL, isa_mem, get_system_io(), &error_abort);
/* The PIC is attached to the MIPS CPU INT0 pin */
i8259 = i8259_init(isa_bus, env->irq[2]);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 924f0fa..b0d7e31 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -651,8 +651,10 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
{
PIIX3State *d = PIIX3_PCI_DEVICE(dev);
- isa_bus_new(DEVICE(d), get_system_memory(),
- pci_address_space_io(dev));
+ if (!isa_bus_new(DEVICE(d), get_system_memory(),
+ pci_address_space_io(dev), errp)) {
+ return;
+ }
memory_region_init_io(&d->rcr_mem, OBJECT(dev), &rcr_ops, d,
"piix3-reset-control", 1);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 8058aac..1925a1c 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -604,8 +604,10 @@ static void pci_ebus_realize(PCIDevice *pci_dev, Error **errp)
{
EbusState *s = DO_UPCAST(EbusState, pci_dev, pci_dev);
- isa_bus_new(DEVICE(pci_dev), get_system_memory(),
- pci_address_space_io(pci_dev));
+ if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
+ pci_address_space_io(pci_dev), errp)) {
+ return;
+ }
pci_dev->config[0x04] = 0x06; // command = bus master, pci mem
pci_dev->config[0x05] = 0x00;
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index d758b39..de3cd3d 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -59,7 +59,7 @@ struct ISADevice {
};
ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space,
- MemoryRegion *address_space_io);
+ MemoryRegion *address_space_io, Error **errp);
void isa_bus_irqs(ISABus *bus, qemu_irq *irqs);
qemu_irq isa_get_irq(ISADevice *dev, int isairq);
void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 14/41] isa: Clean up inappropriate hw_error()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (12 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 13/41] isa: Clean up error handling around isa_bus_new() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 15/41] audio: Clean up inappropriate and unreachable use of hw_error() Markus Armbruster
` (27 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Mark Cave-Ayland,
Hervé Poussineau, Aurelien Jarno, Richard Henderson
isa_bus_irqs(), isa_create() and isa_try_create() call hw_error() when
passed a null bus. Use of hw_error() has always been questionable,
because these are used only during machine initialization, and
printing CPU registers isn't useful there.
Since the previous commit, passing a null bus is a programming error.
Drop the hw_error() and simply let it crash.
Cc: Richard Henderson <rth@twiddle.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
Message-Id: <1450354795-31608-12-git-send-email-armbru@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
hw/isa/isa-bus.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index af6ffd6..630054c 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -63,9 +63,6 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion* address_space,
void isa_bus_irqs(ISABus *bus, qemu_irq *irqs)
{
- if (!bus) {
- hw_error("Can't set isa irqs with no isa bus present.");
- }
bus->irqs = irqs;
}
@@ -137,10 +134,6 @@ ISADevice *isa_create(ISABus *bus, const char *name)
{
DeviceState *dev;
- if (!bus) {
- hw_error("Tried to create isa device %s with no isa bus present.",
- name);
- }
dev = qdev_create(BUS(bus), name);
return ISA_DEVICE(dev);
}
@@ -149,10 +142,6 @@ ISADevice *isa_try_create(ISABus *bus, const char *name)
{
DeviceState *dev;
- if (!bus) {
- hw_error("Tried to create isa device %s with no isa bus present.",
- name);
- }
dev = qdev_try_create(BUS(bus), name);
return ISA_DEVICE(dev);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 15/41] audio: Clean up inappropriate and unreachable use of hw_error()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (13 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 14/41] isa: Clean up inappropriate hw_error() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 16/41] xen-hvm: Mark inappropriate error handling FIXME Markus Armbruster
` (26 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Gerd Hoffmann
audio_init() should not use hw_error(), because dumping CPU registers
is unhelpful there, and aborting is wrong, because it can be called
called from an audio device's realize() method.
The two uses of hw_error() come from commit 0d9acba:
* When qemu_new_timer() fails. It couldn't fail back then, and it
can't fail now. Drop the unreachable error handling.
* When no_audio_driver can't be initialized. It couldn't fail back
then, and it can't fail now. Replace the error handling by an
assertion.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
audio/audio.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 5be4b15..a0fc8b3 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1806,9 +1806,6 @@ static void audio_init (void)
atexit (audio_atexit);
s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
- if (!s->ts) {
- hw_error("Could not create audio timer\n");
- }
audio_process_options ("AUDIO", audio_options);
@@ -1859,12 +1856,8 @@ static void audio_init (void)
if (!done) {
done = !audio_driver_init (s, &no_audio_driver);
- if (!done) {
- hw_error("Could not initialize audio subsystem\n");
- }
- else {
- dolog ("warning: Using timer based audio emulation\n");
- }
+ assert(done);
+ dolog("warning: Using timer based audio emulation\n");
}
if (conf.period.hertz <= 0) {
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 16/41] xen-hvm: Mark inappropriate error handling FIXME
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (14 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 15/41] audio: Clean up inappropriate and unreachable use of hw_error() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 17/41] qemu-nbd: Replace BSDism <err.h> by error_report() Markus Armbruster
` (25 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: xen-devel, Stefano Stabellini
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: xen-devel@lists.xensource.com
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1450370121-5768-14-git-send-email-armbru@redhat.com>
---
xen-hvm.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/xen-hvm.c b/xen-hvm.c
index 3d78a0c..2a93390 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -240,6 +240,7 @@ static void xen_ram_init(PCMachineState *pcms,
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
{
+ /* FIXME caller ram_block_add() wants error_setg() on failure */
unsigned long nr_pfn;
xen_pfn_t *pfn_list;
int i;
@@ -1192,6 +1193,12 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
int xen_hvm_init(PCMachineState *pcms,
MemoryRegion **ram_memory)
{
+ /*
+ * FIXME Returns -1 without cleaning up on some errors (harmless
+ * as long as the caller exit()s on error), dies with hw_error()
+ * on others. hw_error() isn't approprate here. Should probably
+ * simply exit() on all errors.
+ */
int i, rc;
xen_pfn_t ioreq_pfn;
xen_pfn_t bufioreq_pfn;
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 17/41] qemu-nbd: Replace BSDism <err.h> by error_report()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (15 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 16/41] xen-hvm: Mark inappropriate error handling FIXME Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 18/41] error: Use error_report_err() where appropriate (again) Markus Armbruster
` (24 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Coccinelle semantic patch
@@
expression E;
expression list ARGS;
@@
- errx(E, ARGS);
+ error_report(ARGS);
+ exit(E);
@@
expression E, FMT;
expression list ARGS;
@@
- err(E, FMT, ARGS);
+ error_report(FMT /*": %s"*/, ARGS, strerror(errno));
+ exit(E);
followed by a replace of '"/*": %s"*/' by ' : %s"', because I can't
figure out how to make Coccinelle transform strings.
A few of the error messages touched have trailing newlines. They'll
be stripped later in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-2-git-send-email-armbru@redhat.com>
---
qemu-nbd.c | 121 +++++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 77 insertions(+), 44 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 65dc30c..d5c32de 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -30,7 +30,6 @@
#include <stdarg.h>
#include <stdio.h>
#include <getopt.h>
-#include <err.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
@@ -158,7 +157,8 @@ static int find_partition(BlockBackend *blk, int partition,
if ((ret = blk_read(blk, 0, data, 1)) < 0) {
errno = -ret;
- err(EXIT_FAILURE, "error while reading");
+ error_report("error while reading: %s", strerror(errno));
+ exit(EXIT_FAILURE);
}
if (data[510] != 0x55 || data[511] != 0xaa) {
@@ -179,7 +179,8 @@ static int find_partition(BlockBackend *blk, int partition,
if ((ret = blk_read(blk, mbr[i].start_sector_abs, data1, 1)) < 0) {
errno = -ret;
- err(EXIT_FAILURE, "error while reading");
+ error_report("error while reading: %s", strerror(errno));
+ exit(EXIT_FAILURE);
}
for (j = 0; j < 4; j++) {
@@ -454,16 +455,19 @@ int main(int argc, char **argv)
/* fallthrough */
case QEMU_NBD_OPT_CACHE:
if (seen_cache) {
- errx(EXIT_FAILURE, "-n and --cache can only be specified once");
+ error_report("-n and --cache can only be specified once");
+ exit(EXIT_FAILURE);
}
seen_cache = true;
if (bdrv_parse_cache_flags(optarg, &flags) == -1) {
- errx(EXIT_FAILURE, "Invalid cache mode `%s'", optarg);
+ error_report("Invalid cache mode `%s'", optarg);
+ exit(EXIT_FAILURE);
}
break;
case QEMU_NBD_OPT_AIO:
if (seen_aio) {
- errx(EXIT_FAILURE, "--aio can only be specified once");
+ error_report("--aio can only be specified once");
+ exit(EXIT_FAILURE);
}
seen_aio = true;
if (!strcmp(optarg, "native")) {
@@ -471,16 +475,19 @@ int main(int argc, char **argv)
} else if (!strcmp(optarg, "threads")) {
/* this is the default */
} else {
- errx(EXIT_FAILURE, "invalid aio mode `%s'", optarg);
+ error_report("invalid aio mode `%s'", optarg);
+ exit(EXIT_FAILURE);
}
break;
case QEMU_NBD_OPT_DISCARD:
if (seen_discard) {
- errx(EXIT_FAILURE, "--discard can only be specified once");
+ error_report("--discard can only be specified once");
+ exit(EXIT_FAILURE);
}
seen_discard = true;
if (bdrv_parse_discard_flags(optarg, &flags) == -1) {
- errx(EXIT_FAILURE, "Invalid discard mode `%s'", optarg);
+ error_report("Invalid discard mode `%s'", optarg);
+ exit(EXIT_FAILURE);
}
break;
case QEMU_NBD_OPT_DETECT_ZEROES:
@@ -491,13 +498,15 @@ int main(int argc, char **argv)
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
&local_err);
if (local_err) {
- errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode: %s",
- error_get_pretty(local_err));
+ error_report("Failed to parse detect_zeroes mode: %s",
+ error_get_pretty(local_err));
+ exit(EXIT_FAILURE);
}
if (detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
!(flags & BDRV_O_UNMAP)) {
- errx(EXIT_FAILURE, "setting detect-zeroes to unmap is not allowed "
- "without setting discard operation to unmap");
+ error_report("setting detect-zeroes to unmap is not allowed "
+ "without setting discard operation to unmap");
+ exit(EXIT_FAILURE);
}
break;
case 'b':
@@ -509,10 +518,12 @@ int main(int argc, char **argv)
case 'o':
dev_offset = strtoll (optarg, &end, 0);
if (*end) {
- errx(EXIT_FAILURE, "Invalid offset `%s'", optarg);
+ error_report("Invalid offset `%s'", optarg);
+ exit(EXIT_FAILURE);
}
if (dev_offset < 0) {
- errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
+ error_report("Offset must be positive `%s'", optarg);
+ exit(EXIT_FAILURE);
}
break;
case 'l':
@@ -520,8 +531,9 @@ int main(int argc, char **argv)
sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
optarg, false);
if (!sn_opts) {
- errx(EXIT_FAILURE, "Failed in parsing snapshot param `%s'",
- optarg);
+ error_report("Failed in parsing snapshot param `%s'",
+ optarg);
+ exit(EXIT_FAILURE);
}
} else {
sn_id_or_name = optarg;
@@ -534,16 +546,19 @@ int main(int argc, char **argv)
case 'P':
partition = strtol(optarg, &end, 0);
if (*end) {
- errx(EXIT_FAILURE, "Invalid partition `%s'", optarg);
+ error_report("Invalid partition `%s'", optarg);
+ exit(EXIT_FAILURE);
}
if (partition < 1 || partition > 8) {
- errx(EXIT_FAILURE, "Invalid partition %d", partition);
+ error_report("Invalid partition %d", partition);
+ exit(EXIT_FAILURE);
}
break;
case 'k':
sockpath = optarg;
if (sockpath[0] != '/') {
- errx(EXIT_FAILURE, "socket path must be absolute\n");
+ error_report("socket path must be absolute\n");
+ exit(EXIT_FAILURE);
}
break;
case 'd':
@@ -555,10 +570,12 @@ int main(int argc, char **argv)
case 'e':
shared = strtol(optarg, &end, 0);
if (*end) {
- errx(EXIT_FAILURE, "Invalid shared device number '%s'", optarg);
+ error_report("Invalid shared device number '%s'", optarg);
+ exit(EXIT_FAILURE);
}
if (shared < 1) {
- errx(EXIT_FAILURE, "Shared device number must be greater than 0\n");
+ error_report("Shared device number must be greater than 0\n");
+ exit(EXIT_FAILURE);
}
break;
case 'f':
@@ -579,21 +596,24 @@ int main(int argc, char **argv)
exit(0);
break;
case '?':
- errx(EXIT_FAILURE, "Try `%s --help' for more information.",
- argv[0]);
+ error_report("Try `%s --help' for more information.", argv[0]);
+ exit(EXIT_FAILURE);
}
}
if ((argc - optind) != 1) {
- errx(EXIT_FAILURE, "Invalid number of argument.\n"
- "Try `%s --help' for more information.",
- argv[0]);
+ error_report("Invalid number of argument.\n"
+ "Try `%s --help' for more information.",
+ argv[0]);
+ exit(EXIT_FAILURE);
}
if (disconnect) {
fd = open(argv[optind], O_RDWR);
if (fd < 0) {
- err(EXIT_FAILURE, "Cannot open %s", argv[optind]);
+ error_report("Cannot open %s: %s", argv[optind],
+ strerror(errno));
+ exit(EXIT_FAILURE);
}
nbd_disconnect(fd);
@@ -610,7 +630,9 @@ int main(int argc, char **argv)
int ret;
if (qemu_pipe(stderr_fd) < 0) {
- err(EXIT_FAILURE, "Error setting up communication pipe");
+ error_report("Error setting up communication pipe: %s",
+ strerror(errno));
+ exit(EXIT_FAILURE);
}
/* Now daemonize, but keep a communication channel open to
@@ -618,7 +640,8 @@ int main(int argc, char **argv)
*/
pid = fork();
if (pid < 0) {
- err(EXIT_FAILURE, "Failed to fork");
+ error_report("Failed to fork: %s", strerror(errno));
+ exit(EXIT_FAILURE);
} else if (pid == 0) {
close(stderr_fd[0]);
ret = qemu_daemon(1, 0);
@@ -626,7 +649,8 @@ int main(int argc, char **argv)
/* Temporarily redirect stderr to the parent's pipe... */
dup2(stderr_fd[1], STDERR_FILENO);
if (ret < 0) {
- err(EXIT_FAILURE, "Failed to daemonize");
+ error_report("Failed to daemonize: %s", strerror(errno));
+ exit(EXIT_FAILURE);
}
/* ... close the descriptor we inherited and go on. */
@@ -648,7 +672,9 @@ int main(int argc, char **argv)
}
}
if (ret < 0) {
- err(EXIT_FAILURE, "Cannot read from daemon");
+ error_report("Cannot read from daemon: %s",
+ strerror(errno));
+ exit(EXIT_FAILURE);
}
/* Usually the daemon should not print any message.
@@ -680,8 +706,9 @@ int main(int argc, char **argv)
srcpath = argv[optind];
blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
if (!blk) {
- errx(EXIT_FAILURE, "Failed to blk_new_open '%s': %s", argv[optind],
- error_get_pretty(local_err));
+ error_report("Failed to blk_new_open '%s': %s", argv[optind],
+ error_get_pretty(local_err));
+ exit(EXIT_FAILURE);
}
bs = blk_bs(blk);
@@ -696,30 +723,34 @@ int main(int argc, char **argv)
}
if (ret < 0) {
errno = -ret;
- err(EXIT_FAILURE,
- "Failed to load snapshot: %s",
- error_get_pretty(local_err));
+ error_report("Failed to load snapshot: %s: %s",
+ error_get_pretty(local_err), strerror(errno));
+ exit(EXIT_FAILURE);
}
bs->detect_zeroes = detect_zeroes;
fd_size = blk_getlength(blk);
if (fd_size < 0) {
- errx(EXIT_FAILURE, "Failed to determine the image length: %s",
- strerror(-fd_size));
+ error_report("Failed to determine the image length: %s",
+ strerror(-fd_size));
+ exit(EXIT_FAILURE);
}
if (partition != -1) {
ret = find_partition(blk, partition, &dev_offset, &fd_size);
if (ret < 0) {
errno = -ret;
- err(EXIT_FAILURE, "Could not find partition %d", partition);
+ error_report("Could not find partition %d: %s", partition,
+ strerror(errno));
+ exit(EXIT_FAILURE);
}
}
exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed,
&local_err);
if (!exp) {
- errx(EXIT_FAILURE, "%s", error_get_pretty(local_err));
+ error_report("%s", error_get_pretty(local_err));
+ exit(EXIT_FAILURE);
}
fd = socket_listen(saddr, &local_err);
@@ -733,8 +764,8 @@ int main(int argc, char **argv)
ret = pthread_create(&client_thread, NULL, nbd_client_thread, device);
if (ret != 0) {
- errx(EXIT_FAILURE, "Failed to create client thread: %s",
- strerror(ret));
+ error_report("Failed to create client thread: %s", strerror(ret));
+ exit(EXIT_FAILURE);
}
} else {
/* Shut up GCC warnings. */
@@ -747,7 +778,9 @@ int main(int argc, char **argv)
/* now when the initialization is (almost) complete, chdir("/")
* to free any busy filesystems */
if (chdir("/") < 0) {
- err(EXIT_FAILURE, "Could not chdir to root directory");
+ error_report("Could not chdir to root directory: %s",
+ strerror(errno));
+ exit(EXIT_FAILURE);
}
state = RUNNING;
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 18/41] error: Use error_report_err() where appropriate (again)
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (16 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 17/41] qemu-nbd: Replace BSDism <err.h> by error_report() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 19/41] error: Use error_report_err() instead of monitor_printf() Markus Armbruster
` (23 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Same Coccinelle semantic patch as in commit 565f65d.
We now use the original error whole instead of just its message
obtained with error_get_pretty(). This avoids suppressing its hint
(see commit 50b7b00), but I don't think the errors touched in this
commit can come with hints.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-3-git-send-email-armbru@redhat.com>
---
block/sheepdog.c | 3 +--
hw/arm/imx25_pdk.c | 2 +-
hw/arm/kzm.c | 2 +-
hw/arm/netduino2.c | 2 +-
hw/arm/xlnx-ep108.c | 2 +-
hw/ppc/spapr_drc.c | 6 ++----
qemu-nbd.c | 2 +-
vl.c | 2 +-
8 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d80e4ed..dd8301b 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1861,8 +1861,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- error_report("%s", error_get_pretty(local_err));
- error_free(local_err);
+ error_report_err(local_err);
ret = -EIO;
goto out;
}
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index 59a4c11..039f0eb 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -75,7 +75,7 @@ static void imx25_pdk_init(MachineState *machine)
object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
if (err != NULL) {
- error_report("%s", error_get_pretty(err));
+ error_report_err(err);
exit(1);
}
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index eff6f46..f4b463a 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -74,7 +74,7 @@ static void kzm_init(MachineState *machine)
object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
if (err != NULL) {
- error_report("%s", error_get_pretty(err));
+ error_report_err(err);
exit(1);
}
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index a3b9e82..3ab83a1 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -38,7 +38,7 @@ static void netduino2_init(MachineState *machine)
qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err != NULL) {
- error_report("%s", error_get_pretty(err));
+ error_report_err(err);
exit(1);
}
}
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 85b978f..73e6087 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -41,7 +41,7 @@ static void xlnx_ep108_init(MachineState *machine)
object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
if (err) {
- error_report("%s", error_get_pretty(err));
+ error_report_err(err);
exit(1);
}
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 8be62c3..4fb86a6 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -465,8 +465,7 @@ static void realize(DeviceState *d, Error **errp)
object_property_add_alias(root_container, link_name,
drc->owner, child_name, &err);
if (err) {
- error_report("%s", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
object_unref(OBJECT(drc));
}
g_free(child_name);
@@ -486,8 +485,7 @@ static void unrealize(DeviceState *d, Error **errp)
snprintf(name, sizeof(name), "%x", drck->get_index(drc));
object_property_del(root_container, name, &err);
if (err) {
- error_report("%s", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
object_unref(OBJECT(drc));
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d5c32de..65c0ebd 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -749,7 +749,7 @@ int main(int argc, char **argv)
exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed,
&local_err);
if (!exp) {
- error_report("%s", error_get_pretty(local_err));
+ error_report_err(local_err);
exit(EXIT_FAILURE);
}
diff --git a/vl.c b/vl.c
index 6c2add9..7548fa2 100644
--- a/vl.c
+++ b/vl.c
@@ -4553,7 +4553,7 @@ int main(int argc, char **argv, char **envp)
Error *local_err = NULL;
qemu_boot_set(boot_once, &local_err);
if (local_err) {
- error_report("%s", error_get_pretty(local_err));
+ error_report_err(local_err);
exit(1);
}
qemu_register_reset(restore_boot_order, g_strdup(boot_order));
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 19/41] error: Use error_report_err() instead of monitor_printf()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (17 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 18/41] error: Use error_report_err() where appropriate (again) Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 20/41] error: Use error_report_err() instead of ad hoc prints Markus Armbruster
` (22 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Both error_report_err() and monitor_printf() print to the same
destination when monitor_printf() is used correctly, i.e. within an
HMP monitor. Elsewhere, monitor_printf() does nothing, while
error_report_err() reports to stderr.
Most changed functions are HMP command handlers. These should only
run within an HMP monitor. The one exception is bdrv_password_cb(),
which should also only run within an HMP monitor.
Four command handlers prefix the error message with the command name:
balloon, migrate_set_capability, migrate_set_parameter, migrate.
Pointless, drop.
Unlike monitor_printf(), error_report_err() uses the error whole
instead of just its message obtained with error_get_pretty(). This
avoids suppressing its hint (see commit 50b7b00). Example:
(qemu) device_add ivshmem,id=666
Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.
Try "help device_add" for more information
The "Identifiers consist of..." line is new with this patch.
Coccinelle semantic patch:
@@
expression M, E;
@@
- monitor_printf(M, "%s\n", error_get_pretty(E));
- error_free(E);
+ error_report_err(E);
@r1@
expression M, E;
format F;
position p;
@@
- monitor_printf(M, "...%@F@\n", error_get_pretty(E));@p
- error_free(E);
+ error_report_err(E);
@script:python@
p << r1.p;
@@
print "%s:%s:%s: prefix dropped" % (p[0].file, p[0].line, p[0].column)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-4-git-send-email-armbru@redhat.com>
---
hmp.c | 29 +++++++++--------------------
hw/s390x/s390-skeys.c | 3 +--
migration/savevm.c | 3 +--
monitor.c | 6 ++----
4 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/hmp.c b/hmp.c
index c2b2c16..9723397 100644
--- a/hmp.c
+++ b/hmp.c
@@ -41,8 +41,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
{
assert(errp);
if (*errp) {
- monitor_printf(mon, "%s\n", error_get_pretty(*errp));
- error_free(*errp);
+ error_report_err(*errp);
}
}
@@ -556,8 +555,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
info = qmp_query_vnc(&err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
return;
}
@@ -679,8 +677,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
info = qmp_query_balloon(&err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
return;
}
@@ -948,8 +945,7 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
data = qmp_ringbuf_read(chardev, size, false, 0, &err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
return;
}
@@ -1042,8 +1038,7 @@ void hmp_balloon(Monitor *mon, const QDict *qdict)
qmp_balloon(value, &err);
if (err) {
- monitor_printf(mon, "balloon: %s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
}
}
@@ -1191,8 +1186,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
qmp_migrate_set_cache_size(value, &err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
return;
}
}
@@ -1229,9 +1223,7 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
qapi_free_MigrationCapabilityStatusList(caps);
if (err) {
- monitor_printf(mon, "migrate_set_capability: %s\n",
- error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
}
}
@@ -1281,9 +1273,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
}
if (err) {
- monitor_printf(mon, "migrate_set_parameter: %s\n",
- error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
}
}
@@ -1544,8 +1534,7 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
if (err) {
- monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
return;
}
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 539ef6d..4af1558 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -100,8 +100,7 @@ void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
qmp_dump_skeys(filename, &err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
}
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..e277b72 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1984,8 +1984,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
vm_state_size = qemu_ftell(f);
qemu_fclose(f);
if (ret < 0) {
- monitor_printf(mon, "%s\n", error_get_pretty(local_err));
- error_free(local_err);
+ error_report_err(local_err);
goto the_end;
}
diff --git a/monitor.c b/monitor.c
index e7e7ae2..c53a453 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1464,8 +1464,7 @@ static void hmp_boot_set(Monitor *mon, const QDict *qdict)
qemu_boot_set(bootdevice, &local_err);
if (local_err) {
- monitor_printf(mon, "%s\n", error_get_pretty(local_err));
- error_free(local_err);
+ error_report_err(local_err);
} else {
monitor_printf(mon, "boot device list now set to %s\n", bootdevice);
}
@@ -4149,8 +4148,7 @@ static void bdrv_password_cb(void *opaque, const char *password,
bdrv_add_key(bs, password, &local_err);
if (local_err) {
- monitor_printf(mon, "%s\n", error_get_pretty(local_err));
- error_free(local_err);
+ error_report_err(local_err);
ret = -EPERM;
}
if (mon->password_completion_cb)
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 20/41] error: Use error_report_err() instead of ad hoc prints
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (18 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 19/41] error: Use error_report_err() instead of monitor_printf() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 21/41] error: Improve documentation Markus Armbruster
` (21 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Unlike ad hoc prints, error_report_err() uses the error whole instead
of just its message obtained with error_get_pretty(). This avoids
suppressing its hint (see commit 50b7b00). Example:
$ bld/ivshmem-server -l 42@
Parameter 'shm_size' expects a size
You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes.
The last line is new with this patch.
While there, drop a "cannot parse shm size: " message prefix; it's
redundant, because the error message proper is always of the form
"Parameter 'shm_size' expects ...".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-5-git-send-email-armbru@redhat.com>
---
contrib/ivshmem-server/main.c | 4 +---
qdev-monitor.c | 3 +--
qemu-nbd.c | 3 +--
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index 54ff001..00508b5 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -106,9 +106,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int argc, char *argv[])
case 'l': /* shm_size */
parse_option_size("shm_size", optarg, &args->shm_size, &errp);
if (errp) {
- fprintf(stderr, "cannot parse shm size: %s\n",
- error_get_pretty(errp));
- error_free(errp);
+ error_report_err(errp);
ivshmem_server_usage(argv[0], 1);
}
break;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 30936df..3ce4710 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -266,8 +266,7 @@ int qdev_device_help(QemuOpts *opts)
return 1;
error:
- error_printf("%s\n", error_get_pretty(local_err));
- error_free(local_err);
+ error_report_err(local_err);
return 1;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 65c0ebd..706552e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -251,8 +251,7 @@ static void *nbd_client_thread(void *arg)
&size, &local_error);
if (ret < 0) {
if (local_error) {
- fprintf(stderr, "%s\n", error_get_pretty(local_error));
- error_free(local_error);
+ error_report_err(local_error);
}
goto out_socket;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 21/41] error: Improve documentation
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (19 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 20/41] error: Use error_report_err() instead of ad hoc prints Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 22/41] block: Clean up "Could not create temporary overlay" error message Markus Armbruster
` (20 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
While there, tighten error_append_hint()'s assertion.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1450452927-8346-6-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/qapi/error.h | 20 ++++++++++++++++++--
util/error.c | 2 +-
util/qemu-error.c | 8 ++++----
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 1480f59..b18a608 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -18,6 +18,15 @@
* Create an error:
* error_setg(&err, "situation normal, all fouled up");
*
+ * Create an error and add additional explanation:
+ * error_setg(&err, "invalid quark");
+ * error_append_hint(&err, "Valid quarks are up, down, strange, "
+ * "charm, top, bottom.\n");
+ *
+ * Do *not* contract this to
+ * error_setg(&err, "invalid quark\n"
+ * "Valid quarks are up, down, strange, charm, top, bottom.");
+ *
* Report an error to stderr:
* error_report_err(err);
* This frees the error object.
@@ -26,6 +35,7 @@
* const char *msg = error_get_pretty(err);
* do with msg what needs to be done...
* error_free(err);
+ * Note that this loses hints added with error_append_hint().
*
* Handle an error without reporting it (just for completeness):
* error_free(err);
@@ -142,6 +152,8 @@ ErrorClass error_get_class(const Error *err);
* If @errp is anything else, *@errp must be NULL.
* The new error's class is ERROR_CLASS_GENERIC_ERROR, and its
* human-readable error message is made from printf-style @fmt, ...
+ * The resulting message should be a single phrase, with no newline or
+ * trailing punctuation.
*/
#define error_setg(errp, fmt, ...) \
error_setg_internal((errp), __FILE__, __LINE__, __func__, \
@@ -198,7 +210,11 @@ void error_propagate(Error **dst_errp, Error *local_err);
/**
* Append a printf-style human-readable explanation to an existing error.
- * May be called multiple times, and safe if @errp is NULL.
+ * @errp may be NULL, but not &error_fatal or &error_abort.
+ * Trivially the case if you call it only after error_setg() or
+ * error_propagate().
+ * May be called multiple times. The resulting hint should end with a
+ * newline.
*/
void error_append_hint(Error **errp, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
@@ -232,7 +248,7 @@ void error_free_or_abort(Error **errp);
/*
* Convenience function to error_report() and free @err.
*/
-void error_report_err(Error *);
+void error_report_err(Error *err);
/*
* Just like error_setg(), except you get to specify the error class.
diff --git a/util/error.c b/util/error.c
index 9b27c45..ebfb74b 100644
--- a/util/error.c
+++ b/util/error.c
@@ -132,7 +132,7 @@ void error_append_hint(Error **errp, const char *fmt, ...)
return;
}
err = *errp;
- assert(err && errp != &error_abort);
+ assert(err && errp != &error_abort && errp != &error_fatal);
if (!err->hint) {
err->hint = g_string_new(NULL);
diff --git a/util/qemu-error.c b/util/qemu-error.c
index c1574bb..ecf5708 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -200,8 +200,8 @@ static void error_print_loc(void)
bool enable_timestamp_msg;
/*
* Print an error message to current monitor if we have one, else to stderr.
- * Format arguments like vsprintf(). The result should not contain
- * newlines.
+ * Format arguments like vsprintf(). The resulting message should be
+ * a single phrase, with no newline or trailing punctuation.
* Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use error_setg() there.
*/
@@ -224,8 +224,8 @@ void error_vreport(const char *fmt, va_list ap)
/*
* Print an error message to current monitor if we have one, else to stderr.
- * Format arguments like sprintf(). The result should not contain
- * newlines.
+ * Format arguments like sprintf(). The resulting message should be a
+ * single phrase, with no newline or trailing punctuation.
* Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use error_setg() there.
*/
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 22/41] block: Clean up "Could not create temporary overlay" error message
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (20 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 21/41] error: Improve documentation Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 23/41] qemu-nbd: Clean up "Failed to load snapshot" " Markus Armbruster
` (19 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
bdrv_create() sets an error and returns -errno on failure. When the
latter is interesting, the error is created with error_setg_errno().
bdrv_append_temp_snapshot() uses the error's message to create a new
one with error_setg_errno(). This adds a strerror() that is either
uninteresting or duplicate. Use error_setg() instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-7-git-send-email-armbru@redhat.com>
---
block.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 01655de..b2bdff9 100644
--- a/block.c
+++ b/block.c
@@ -1463,9 +1463,8 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
ret = bdrv_create(&bdrv_qcow2, tmp_filename, opts, &local_err);
qemu_opts_del(opts);
if (ret < 0) {
- error_setg_errno(errp, -ret, "Could not create temporary overlay "
- "'%s': %s", tmp_filename,
- error_get_pretty(local_err));
+ error_setg(errp, "Could not create temporary overlay '%s': %s",
+ tmp_filename, error_get_pretty(local_err));
error_free(local_err);
goto out;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 23/41] qemu-nbd: Clean up "Failed to load snapshot" error message
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (21 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 22/41] block: Clean up "Could not create temporary overlay" error message Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 24/41] test-throttle: Simplify qemu_init_main_loop() error handling Markus Armbruster
` (18 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
bdrv_snapshot_load_tmp() sets an error and returns -errno on failure.
We report both even though the error message is self-contained. Drop
the redundant strerror().
While there: setting errno right before exit() is pointless, so drop
that, too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-8-git-send-email-armbru@redhat.com>
---
qemu-nbd.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 706552e..b8be3bc 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -156,8 +156,7 @@ static int find_partition(BlockBackend *blk, int partition,
int ret;
if ((ret = blk_read(blk, 0, data, 1)) < 0) {
- errno = -ret;
- error_report("error while reading: %s", strerror(errno));
+ error_report("error while reading: %s", strerror(-ret));
exit(EXIT_FAILURE);
}
@@ -178,8 +177,7 @@ static int find_partition(BlockBackend *blk, int partition,
int j;
if ((ret = blk_read(blk, mbr[i].start_sector_abs, data1, 1)) < 0) {
- errno = -ret;
- error_report("error while reading: %s", strerror(errno));
+ error_report("error while reading: %s", strerror(-ret));
exit(EXIT_FAILURE);
}
@@ -721,9 +719,8 @@ int main(int argc, char **argv)
&local_err);
}
if (ret < 0) {
- errno = -ret;
- error_report("Failed to load snapshot: %s: %s",
- error_get_pretty(local_err), strerror(errno));
+ error_report("Failed to load snapshot: %s",
+ error_get_pretty(local_err));
exit(EXIT_FAILURE);
}
@@ -738,9 +735,8 @@ int main(int argc, char **argv)
if (partition != -1) {
ret = find_partition(blk, partition, &dev_offset, &fd_size);
if (ret < 0) {
- errno = -ret;
error_report("Could not find partition %d: %s", partition,
- strerror(errno));
+ strerror(-ret));
exit(EXIT_FAILURE);
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 24/41] test-throttle: Simplify qemu_init_main_loop() error handling
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (22 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 23/41] qemu-nbd: Clean up "Failed to load snapshot" " Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 25/41] error: New error_prepend(), error_reportf_err() Markus Armbruster
` (17 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
The code looks like it tries to check for both qemu_init_main_loop()
and qemu_get_aio_context() failure in one conditional. In fact,
qemu_get_aio_context() can fail only after qemu_init_main_loop()
failed.
Simplify accordingly: check for qemu_init_main_loop() error directly,
without bothering to improve its error message. Call
qemu_get_aio_context() only when qemu_get_aio_context() succeeded. It
can't fail then, so no need to check.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-9-git-send-email-armbru@redhat.com>
---
tests/test-throttle.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 85c9b6c..a95039f 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -581,21 +581,8 @@ static void test_groups(void)
int main(int argc, char **argv)
{
- Error *local_error = NULL;
-
- qemu_init_main_loop(&local_error);
+ qemu_init_main_loop(&error_fatal);
ctx = qemu_get_aio_context();
-
- if (!ctx) {
- error_report("Failed to create AIO Context: '%s'",
- local_error ? error_get_pretty(local_error) :
- "Failed to initialize the QEMU main loop");
- if (local_error) {
- error_free(local_error);
- }
- exit(1);
- }
-
bdrv_init();
do {} while (g_main_context_iteration(NULL, false));
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 25/41] error: New error_prepend(), error_reportf_err()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (23 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 24/41] test-throttle: Simplify qemu_init_main_loop() error handling Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 26/41] error: Don't decorate original error message when adding to it Markus Armbruster
` (16 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Instead of simply propagating an error verbatim, we sometimes want to
add to its message, like this:
frobnicate(arg, &err);
error_setg(errp, "Can't frobnicate %s: %s",
arg, error_get_pretty(err));
error_free(err);
This is suboptimal, because it loses err's hint (if any). Moreover,
when errp is &error_abort or is subsequently propagated to
&error_abort, the abort message points to the place where we last
added to the error, not to the place where it originated.
To avoid these issues, provide means to add to an error's message in
place:
frobnicate(arg, errp);
error_prepend(errp, "Can't frobnicate %s: ", arg);
Likewise, reporting an error like
frobnicate(arg, &err);
error_report("Can't frobnicate %s: %s", arg, error_get_pretty(err));
can lose err's hint. To avoid:
error_reportf_err(err, "Can't frobnicate %s: ", arg);
The next commits will put these functions to use.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-10-git-send-email-armbru@redhat.com>
---
include/qapi/error.h | 31 +++++++++++++++++++++++++++++--
util/error.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index b18a608..45d6c72 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -31,6 +31,9 @@
* error_report_err(err);
* This frees the error object.
*
+ * Report an error to stderr with additional text prepended:
+ * error_reportf_err(err, "Could not frobnicate '%s': ", name);
+ *
* Report an error somewhere else:
* const char *msg = error_get_pretty(err);
* do with msg what needs to be done...
@@ -48,6 +51,10 @@
* error_propagate(errp, err);
* where Error **errp is a parameter, by convention the last one.
*
+ * Pass an existing error to the caller with the message modified:
+ * error_propagate(errp, err);
+ * error_prepend(errp, "Could not frobnicate '%s': ", name);
+ *
* Create a new error and pass it to the caller:
* error_setg(errp, "situation normal, all fouled up");
*
@@ -108,9 +115,10 @@
#ifndef ERROR_H
#define ERROR_H
+#include <stdarg.h>
+#include <stdbool.h>
#include "qemu/compiler.h"
#include "qapi-types.h"
-#include <stdbool.h>
/*
* Opaque error object.
@@ -208,7 +216,20 @@ void error_setg_win32_internal(Error **errp,
*/
void error_propagate(Error **dst_errp, Error *local_err);
-/**
+/*
+ * Prepend some text to @errp's human-readable error message.
+ * The text is made by formatting @fmt, @ap like vprintf().
+ */
+void error_vprepend(Error **errp, const char *fmt, va_list ap);
+
+/*
+ * Prepend some text to @errp's human-readable error message.
+ * The text is made by formatting @fmt, ... like printf().
+ */
+void error_prepend(Error **errp, const char *fmt, ...)
+ GCC_FMT_ATTR(2, 3);
+
+/*
* Append a printf-style human-readable explanation to an existing error.
* @errp may be NULL, but not &error_fatal or &error_abort.
* Trivially the case if you call it only after error_setg() or
@@ -251,6 +272,12 @@ void error_free_or_abort(Error **errp);
void error_report_err(Error *err);
/*
+ * Convenience function to error_prepend(), error_report() and free @err.
+ */
+void error_reportf_err(Error *err, const char *fmt, ...)
+ GCC_FMT_ATTR(2, 3);
+
+/*
* Just like error_setg(), except you get to specify the error class.
* Note: use of error classes other than ERROR_CLASS_GENERIC_ERROR is
* strongly discouraged.
diff --git a/util/error.c b/util/error.c
index ebfb74b..57303fd 100644
--- a/util/error.c
+++ b/util/error.c
@@ -122,6 +122,29 @@ void error_setg_file_open_internal(Error **errp,
"Could not open '%s'", filename);
}
+void error_vprepend(Error **errp, const char *fmt, va_list ap)
+{
+ GString *newmsg;
+
+ if (!errp) {
+ return;
+ }
+
+ newmsg = g_string_new(NULL);
+ g_string_vprintf(newmsg, fmt, ap);
+ g_string_append(newmsg, (*errp)->msg);
+ (*errp)->msg = g_string_free(newmsg, 0);
+}
+
+void error_prepend(Error **errp, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ error_vprepend(errp, fmt, ap);
+ va_end(ap);
+}
+
void error_append_hint(Error **errp, const char *fmt, ...)
{
va_list ap;
@@ -209,6 +232,16 @@ void error_report_err(Error *err)
error_free(err);
}
+void error_reportf_err(Error *err, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ error_vprepend(&err, fmt, ap);
+ va_end(ap);
+ error_report_err(err);
+}
+
void error_free(Error *err)
{
if (err) {
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 26/41] error: Don't decorate original error message when adding to it
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (24 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 25/41] error: New error_prepend(), error_reportf_err() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 27/41] error: Use error_reportf_err() where it makes obvious sense Markus Armbruster
` (15 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Prepend the additional information, colon, space to the original
message without enclosing it in parenthesis or quotes, like we do
elsewhere.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-11-git-send-email-armbru@redhat.com>
---
hw/core/qdev-properties.c | 2 +-
qemu-img.c | 2 +-
tests/test-aio.c | 2 +-
tests/test-thread-pool.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 33e245e..fffb58e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1063,7 +1063,7 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev,
object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
if (err != NULL) {
assert(prop->user_provided);
- error_report("Warning: global %s.%s=%s ignored (%s)",
+ error_report("Warning: global %s.%s=%s ignored: %s",
prop->driver, prop->property, prop->value,
error_get_pretty(err));
error_free(err);
diff --git a/qemu-img.c b/qemu-img.c
index 3d48b4f..f4f5540 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2439,7 +2439,7 @@ static int img_snapshot(int argc, char **argv)
case SNAPSHOT_DELETE:
bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
if (err) {
- error_report("Could not delete snapshot '%s': (%s)",
+ error_report("Could not delete snapshot '%s': %s",
snapshot_name, error_get_pretty(err));
error_free(err);
ret = 1;
diff --git a/tests/test-aio.c b/tests/test-aio.c
index e188d8c..f0b447e 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -832,7 +832,7 @@ int main(int argc, char **argv)
ctx = aio_context_new(&local_error);
if (!ctx) {
- error_report("Failed to create AIO Context: '%s'",
+ error_report("Failed to create AIO Context: %s",
error_get_pretty(local_error));
error_free(local_error);
exit(1);
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index 6a0b981..153b8f5 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -229,7 +229,7 @@ int main(int argc, char **argv)
ctx = aio_context_new(&local_error);
if (!ctx) {
- error_report("Failed to create AIO Context: '%s'",
+ error_report("Failed to create AIO Context: %s",
error_get_pretty(local_error));
error_free(local_error);
exit(1);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 27/41] error: Use error_reportf_err() where it makes obvious sense
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (25 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 26/41] error: Don't decorate original error message when adding to it Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 28/41] error: Use error_prepend() " Markus Armbruster
` (14 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Done with this Coccinelle semantic patch
@@
expression FMT, E, S;
expression list ARGS;
@@
- error_report(FMT, ARGS, error_get_pretty(E));
+ error_reportf_err(E, FMT/*@@@*/, ARGS);
(
- error_free(E);
|
exit(S);
|
abort();
)
followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
because I can't figure out how to make Coccinelle transform strings.
We now use the error whole instead of just its message obtained with
error_get_pretty(). This avoids suppressing its hint (see commit
50b7b00), but I can't see how the errors touched in this commit could
come with hints.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1450452927-8346-12-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
arch_init.c | 4 +---
block/sheepdog.c | 5 ++---
blockdev.c | 12 +++++-------
hw/arm/cubieboard.c | 9 ++++-----
hw/arm/digic_boards.c | 3 +--
hw/core/qdev-properties.c | 6 ++----
hw/core/qdev.c | 5 ++---
hw/i386/pc.c | 5 ++---
hw/ppc/e500.c | 4 ++--
hw/usb/bus.c | 5 ++---
qemu-img.c | 33 +++++++++++++--------------------
qemu-nbd.c | 11 +++++------
replay/replay.c | 3 +--
tests/test-aio.c | 4 +---
tests/test-thread-pool.c | 4 +---
ui/vnc.c | 4 +---
vl.c | 6 ++----
17 files changed, 47 insertions(+), 76 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 38f5fb9..d1383b3 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -258,9 +258,7 @@ void do_acpitable_option(const QemuOpts *opts)
acpi_table_add(opts, &err);
if (err) {
- error_report("Wrong acpi table provided: %s",
- error_get_pretty(err));
- error_free(err);
+ error_reportf_err(err, "Wrong acpi table provided: ");
exit(1);
}
#endif
diff --git a/block/sheepdog.c b/block/sheepdog.c
index dd8301b..6986be8 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2405,9 +2405,8 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
ret = do_sd_create(s, &new_vid, 1, &local_err);
if (ret < 0) {
- error_report("failed to create inode for snapshot: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err,
+ "failed to create inode for snapshot: ");
goto cleanup;
}
diff --git a/blockdev.c b/blockdev.c
index 2df0c6d..1392fff 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1582,13 +1582,11 @@ static void internal_snapshot_abort(BlkActionState *common)
}
if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
- error_report("Failed to delete snapshot with id '%s' and name '%s' on "
- "device '%s' in abort: %s",
- sn->id_str,
- sn->name,
- bdrv_get_device_name(bs),
- error_get_pretty(local_error));
- error_free(local_error);
+ error_reportf_err(local_error,
+ "Failed to delete snapshot with id '%s' and "
+ "name '%s' on device '%s' in abort: ",
+ sn->id_str, sn->name,
+ bdrv_get_device_name(bs));
}
}
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index bf068cd..a71e43c 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -39,27 +39,26 @@ static void cubieboard_init(MachineState *machine)
object_property_set_int(OBJECT(&s->a10->emac), 1, "phy-addr", &err);
if (err != NULL) {
- error_report("Couldn't set phy address: %s", error_get_pretty(err));
+ error_reportf_err(err, "Couldn't set phy address: ");
exit(1);
}
object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", &err);
if (err != NULL) {
- error_report("Couldn't set clk0 frequency: %s", error_get_pretty(err));
+ error_reportf_err(err, "Couldn't set clk0 frequency: ");
exit(1);
}
object_property_set_int(OBJECT(&s->a10->timer), 24000000, "clk1-freq",
&err);
if (err != NULL) {
- error_report("Couldn't set clk1 frequency: %s", error_get_pretty(err));
+ error_reportf_err(err, "Couldn't set clk1 frequency: ");
exit(1);
}
object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
if (err != NULL) {
- error_report("Couldn't realize Allwinner A10: %s",
- error_get_pretty(err));
+ error_reportf_err(err, "Couldn't realize Allwinner A10: ");
exit(1);
}
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 710045a..dfaed25 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -64,8 +64,7 @@ static void digic4_board_init(DigicBoard *board)
s->digic = DIGIC(object_new(TYPE_DIGIC));
object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
if (err != NULL) {
- error_report("Couldn't realize DIGIC SoC: %s",
- error_get_pretty(err));
+ error_reportf_err(err, "Couldn't realize DIGIC SoC: ");
exit(1);
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index fffb58e..3572810 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1063,10 +1063,8 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev,
object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
if (err != NULL) {
assert(prop->user_provided);
- error_report("Warning: global %s.%s=%s ignored: %s",
- prop->driver, prop->property, prop->value,
- error_get_pretty(err));
- error_free(err);
+ error_reportf_err(err, "Warning: global %s.%s=%s ignored: ",
+ prop->driver, prop->property, prop->value);
return;
}
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4e3173d..2c7101d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -370,9 +370,8 @@ void qdev_init_nofail(DeviceState *dev)
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
- error_report("Initialization of device %s failed: %s",
- object_get_typename(OBJECT(dev)),
- error_get_pretty(err));
+ error_reportf_err(err, "Initialization of device %s failed: ",
+ object_get_typename(OBJECT(dev)));
exit(1);
}
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 166e8e2..0e5c86a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1253,9 +1253,8 @@ void pc_acpi_init(const char *default_dsdt)
acpi_table_add_builtin(opts, &err);
if (err) {
- error_report("WARNING: failed to load %s: %s", filename,
- error_get_pretty(err));
- error_free(err);
+ error_reportf_err(err, "WARNING: failed to load %s: ",
+ filename);
}
g_free(filename);
}
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index b3418db..bd7da47 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -751,8 +751,8 @@ static qemu_irq *ppce500_init_mpic(MachineState *machine, PPCE500Params *params,
dev = ppce500_init_mpic_kvm(params, irqs, &err);
}
if (machine_kernel_irqchip_required(machine) && !dev) {
- error_report("kernel_irqchip requested but unavailable: %s",
- error_get_pretty(err));
+ error_reportf_err(err,
+ "kernel_irqchip requested but unavailable: ");
exit(1);
}
}
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index ee6b43a..26ab67f 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -725,9 +725,8 @@ USBDevice *usbdevice_create(const char *cmdline)
}
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
- error_report("Failed to initialize USB device '%s': %s",
- f->name, error_get_pretty(err));
- error_free(err);
+ error_reportf_err(err, "Failed to initialize USB device '%s': ",
+ f->name);
object_unparent(OBJECT(dev));
return NULL;
}
diff --git a/qemu-img.c b/qemu-img.c
index f4f5540..a5949e6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -213,9 +213,7 @@ static BlockBackend *img_open(const char *id, const char *filename,
blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
if (!blk) {
- error_report("Could not open '%s': %s", filename,
- error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "Could not open '%s': ", filename);
goto fail;
}
@@ -360,8 +358,7 @@ static int img_create(int argc, char **argv)
bdrv_img_create(filename, fmt, base_filename, base_fmt,
options, img_size, BDRV_O_FLAGS, &local_err, quiet);
if (local_err) {
- error_report("%s: %s", filename, error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "%s: ", filename);
goto fail;
}
@@ -1711,9 +1708,7 @@ static int img_convert(int argc, char **argv)
bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
}
if (local_err) {
- error_report("Failed to load snapshot: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "Failed to load snapshot: ");
ret = -1;
goto out;
}
@@ -1809,9 +1804,8 @@ static int img_convert(int argc, char **argv)
/* Create the new image */
ret = bdrv_create(drv, out_filename, opts, &local_err);
if (ret < 0) {
- error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "%s: error while converting %s: ",
+ out_filename, out_fmt);
goto out;
}
}
@@ -2439,9 +2433,8 @@ static int img_snapshot(int argc, char **argv)
case SNAPSHOT_DELETE:
bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
if (err) {
- error_report("Could not delete snapshot '%s': %s",
- snapshot_name, error_get_pretty(err));
- error_free(err);
+ error_reportf_err(err, "Could not delete snapshot '%s': ",
+ snapshot_name);
ret = 1;
}
break;
@@ -2574,9 +2567,9 @@ static int img_rebase(int argc, char **argv)
blk_old_backing = blk_new_open("old_backing", backing_name, NULL,
options, src_flags, &local_err);
if (!blk_old_backing) {
- error_report("Could not open old backing file '%s': %s",
- backing_name, error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err,
+ "Could not open old backing file '%s': ",
+ backing_name);
goto out;
}
@@ -2591,9 +2584,9 @@ static int img_rebase(int argc, char **argv)
blk_new_backing = blk_new_open("new_backing", out_baseimg, NULL,
options, src_flags, &local_err);
if (!blk_new_backing) {
- error_report("Could not open new backing file '%s': %s",
- out_baseimg, error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err,
+ "Could not open new backing file '%s': ",
+ out_baseimg);
goto out;
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index b8be3bc..f9fce4a 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -495,8 +495,8 @@ int main(int argc, char **argv)
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
&local_err);
if (local_err) {
- error_report("Failed to parse detect_zeroes mode: %s",
- error_get_pretty(local_err));
+ error_reportf_err(local_err,
+ "Failed to parse detect_zeroes mode: ");
exit(EXIT_FAILURE);
}
if (detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
@@ -703,8 +703,8 @@ int main(int argc, char **argv)
srcpath = argv[optind];
blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
if (!blk) {
- error_report("Failed to blk_new_open '%s': %s", argv[optind],
- error_get_pretty(local_err));
+ error_reportf_err(local_err, "Failed to blk_new_open '%s': ",
+ argv[optind]);
exit(EXIT_FAILURE);
}
bs = blk_bs(blk);
@@ -719,8 +719,7 @@ int main(int argc, char **argv)
&local_err);
}
if (ret < 0) {
- error_report("Failed to load snapshot: %s",
- error_get_pretty(local_err));
+ error_reportf_err(local_err, "Failed to load snapshot: ");
exit(EXIT_FAILURE);
}
diff --git a/replay/replay.c b/replay/replay.c
index 0d33e82..e4673b3 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -291,8 +291,7 @@ void replay_start(void)
}
if (replay_blockers) {
- error_report("Record/replay: %s",
- error_get_pretty(replay_blockers->data));
+ error_reportf_err(replay_blockers->data, "Record/replay: ");
exit(1);
}
if (!use_icount) {
diff --git a/tests/test-aio.c b/tests/test-aio.c
index f0b447e..6ccea98 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -832,9 +832,7 @@ int main(int argc, char **argv)
ctx = aio_context_new(&local_error);
if (!ctx) {
- error_report("Failed to create AIO Context: %s",
- error_get_pretty(local_error));
- error_free(local_error);
+ error_reportf_err(local_error, "Failed to create AIO Context: ");
exit(1);
}
src = aio_get_g_source(ctx);
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index 153b8f5..ccdee39 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -229,9 +229,7 @@ int main(int argc, char **argv)
ctx = aio_context_new(&local_error);
if (!ctx) {
- error_report("Failed to create AIO Context: %s",
- error_get_pretty(local_error));
- error_free(local_error);
+ error_reportf_err(local_error, "Failed to create AIO Context: ");
exit(1);
}
pool = aio_get_thread_pool(ctx);
diff --git a/ui/vnc.c b/ui/vnc.c
index 09756cd..54673eb 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3861,9 +3861,7 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
vnc_display_init(id);
vnc_display_open(id, &local_err);
if (local_err != NULL) {
- error_report("Failed to start VNC server: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "Failed to start VNC server: ");
exit(1);
}
return 0;
diff --git a/vl.c b/vl.c
index 7548fa2..20a367b 100644
--- a/vl.c
+++ b/vl.c
@@ -3043,7 +3043,7 @@ int main(int argc, char **argv, char **envp)
runstate_init();
if (qcrypto_init(&err) < 0) {
- error_report("cannot initialize crypto: %s", error_get_pretty(err));
+ error_reportf_err(err, "cannot initialize crypto: ");
exit(1);
}
rtc_clock = QEMU_CLOCK_HOST;
@@ -4649,9 +4649,7 @@ int main(int argc, char **argv, char **envp)
Error *local_err = NULL;
qemu_start_incoming_migration(incoming, &local_err);
if (local_err) {
- error_report("-incoming %s: %s", incoming,
- error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "-incoming %s: ", incoming);
exit(1);
}
} else if (autostart) {
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 28/41] error: Use error_prepend() where it makes obvious sense
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (26 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 27/41] error: Use error_reportf_err() where it makes obvious sense Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 29/41] spapr: Use error_reportf_err() Markus Armbruster
` (13 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Done with this Coccinelle semantic patch
@@
expression FMT, E1, E2;
expression list ARGS;
@@
- error_setg(E1, FMT, ARGS, error_get_pretty(E2));
+ error_propagate(E1, E2);/*###*/
+ error_prepend(E1, FMT/*@@@*/, ARGS);
followed by manual cleanup, first because I can't figure out how to
make Coccinelle transform strings, and second to get rid of now
superfluous error_propagate().
We now use or propagate the original error whole instead of just its
message obtained with error_get_pretty(). This avoids suppressing its
hint (see commit 50b7b00), but I can't see how the errors touched in
this commit could come with hints. It also improves the message
printed with &error_abort when we screw up (see commit 1e9b65b).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 19 ++++++++-----------
block/qcow2.c | 5 ++---
block/qed.c | 5 ++---
hw/block/dataplane/virtio-blk.c | 8 ++------
hw/scsi/vhost-scsi.c | 6 ++----
hw/usb/bus.c | 6 +++---
6 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/block.c b/block.c
index b2bdff9..54c37f9 100644
--- a/block.c
+++ b/block.c
@@ -1349,12 +1349,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
ret = bdrv_open_inherit(&backing_hd,
*backing_filename ? backing_filename : NULL,
reference, options, 0, bs, &child_backing,
- &local_err);
+ errp);
if (ret < 0) {
bs->open_flags |= BDRV_O_NO_BACKING;
- error_setg(errp, "Could not open backing file: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prepend(errp, "Could not open backing file: ");
goto free_exit;
}
@@ -1460,12 +1458,11 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
opts = qemu_opts_create(bdrv_qcow2.create_opts, NULL, 0,
&error_abort);
qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size, &error_abort);
- ret = bdrv_create(&bdrv_qcow2, tmp_filename, opts, &local_err);
+ ret = bdrv_create(&bdrv_qcow2, tmp_filename, opts, errp);
qemu_opts_del(opts);
if (ret < 0) {
- error_setg(errp, "Could not create temporary overlay '%s': %s",
- tmp_filename, error_get_pretty(local_err));
- error_free(local_err);
+ error_prepend(errp, "Could not create temporary overlay '%s': ",
+ tmp_filename);
goto out;
}
@@ -3729,9 +3726,9 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]);
if (errp) {
- error_setg(errp, "Node '%s' is busy: %s",
- bdrv_get_device_or_node_name(bs),
- error_get_pretty(blocker->reason));
+ *errp = error_copy(blocker->reason);
+ error_prepend(errp, "Node '%s' is busy: ",
+ bdrv_get_device_or_node_name(bs));
}
return true;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 1789af4..d992e7f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1762,9 +1762,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
ret = qcow2_open(bs, options, flags, &local_err);
QDECREF(options);
if (local_err) {
- error_setg(errp, "Could not reopen qcow2 layer: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_propagate(errp, local_err);
+ error_prepend(errp, "Could not reopen qcow2 layer: ");
return;
} else if (ret < 0) {
error_setg_errno(errp, -ret, "Could not reopen qcow2 layer");
diff --git a/block/qed.c b/block/qed.c
index 9b88895..31f4cc9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1611,9 +1611,8 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
memset(s, 0, sizeof(BDRVQEDState));
ret = bdrv_qed_open(bs, NULL, bs->open_flags, &local_err);
if (local_err) {
- error_setg(errp, "Could not reopen qed layer: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_propagate(errp, local_err);
+ error_prepend(errp, "Could not reopen qed layer: ");
return;
} else if (ret < 0) {
error_setg_errno(errp, -ret, "Could not reopen qed layer");
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index a2529b2..b8ce6cd 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -142,7 +142,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
Error **errp)
{
VirtIOBlockDataPlane *s;
- Error *local_err = NULL;
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
@@ -163,11 +162,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
/* If dataplane is (re-)enabled while the guest is running there could be
* block jobs that can conflict.
*/
- if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE,
- &local_err)) {
- error_setg(errp, "cannot start dataplane thread: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
+ error_prepend(errp, "cannot start dataplane thread: ");
return;
}
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 00cdac6..7bc8288 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -217,11 +217,9 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
}
if (vs->conf.vhostfd) {
- vhostfd = monitor_fd_param(cur_mon, vs->conf.vhostfd, &err);
+ vhostfd = monitor_fd_param(cur_mon, vs->conf.vhostfd, errp);
if (vhostfd == -1) {
- error_setg(errp, "vhost-scsi: unable to parse vhostfd: %s",
- error_get_pretty(err));
- error_free(err);
+ error_prepend(errp, "vhost-scsi: unable to parse vhostfd: ");
return;
}
} else {
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 26ab67f..1bbe930 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -329,9 +329,9 @@ static USBDevice *usb_try_create_simple(USBBus *bus, const char *name,
}
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
- error_setg(errp, "Failed to initialize USB device '%s': %s",
- name, error_get_pretty(err));
- error_free(err);
+ error_propagate(errp, err);
+ error_prepend(errp, "Failed to initialize USB device '%s': ",
+ name);
object_unparent(OBJECT(dev));
return NULL;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 29/41] spapr: Use error_reportf_err()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (27 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 28/41] error: Use error_prepend() " Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 30/41] migration: Use error_reportf_err() instead of monitor_printf() Markus Armbruster
` (12 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Not caught by Coccinelle, because we report the error only
conditionally here.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-14-git-send-email-armbru@redhat.com>
---
hw/ppc/spapr.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0ca0176..091cdb1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -122,10 +122,11 @@ static XICSState *xics_system_init(MachineState *machine,
icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs, &err);
}
if (machine_kernel_irqchip_required(machine) && !icp) {
- error_report("kernel_irqchip requested but unavailable: %s",
- error_get_pretty(err));
+ error_reportf_err(err,
+ "kernel_irqchip requested but unavailable: ");
+ } else {
+ error_free(err);
}
- error_free(err);
}
if (!icp) {
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 30/41] migration: Use error_reportf_err() instead of monitor_printf()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (28 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 29/41] spapr: Use error_reportf_err() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 31/41] qemu-io qemu-nbd: Use error_report() etc. instead of fprintf() Markus Armbruster
` (11 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Both error_reportf_err() and monitor_printf() print to the same
destination when monitor_printf() is used correctly, i.e. within an
HMP monitor. Elsewhere, monitor_printf() does nothing, while
error_reportf_err() reports to stderr.
Both changed functions are HMP command handlers. These should only
run within an HMP monitor.
Unlike monitor_printf(), error_reportf_err() uses the error whole
instead of just its message obtained with error_get_pretty(). This
avoids suppressing its hint (see commit 50b7b00), but I don't think
the errors touched in this commit can come with hints.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-15-git-send-email-armbru@redhat.com>
---
migration/savevm.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index e277b72..bcaeb70 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1927,10 +1927,9 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
/* Delete old snapshots of the same name */
if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s': %s\n",
- bdrv_get_device_name(bs1), error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err,
+ "Error while deleting snapshot on device '%s': ",
+ bdrv_get_device_name(bs1));
return;
}
@@ -2108,10 +2107,9 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
const char *name = qdict_get_str(qdict, "name");
if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s': %s\n",
- bdrv_get_device_name(bs), error_get_pretty(err));
- error_free(err);
+ error_reportf_err(err,
+ "Error while deleting snapshot on device '%s': ",
+ bdrv_get_device_name(bs));
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 31/41] qemu-io qemu-nbd: Use error_report() etc. instead of fprintf()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (29 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 30/41] migration: Use error_reportf_err() instead of monitor_printf() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 32/41] error: Strip trailing '\n' from error string arguments (again) Markus Armbruster
` (10 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Just three instances left.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-16-git-send-email-armbru@redhat.com>
---
qemu-io.c | 8 +++-----
qemu-nbd.c | 2 +-
tests/qemu-iotests/059.out | 8 ++++----
tests/qemu-iotests/060.out | 2 +-
tests/qemu-iotests/069.out | 2 +-
tests/qemu-iotests/070.out | 2 +-
tests/qemu-iotests/075.out | 14 +++++++-------
tests/qemu-iotests/076.out | 6 +++---
tests/qemu-iotests/078.out | 12 ++++++------
tests/qemu-iotests/080.out | 36 ++++++++++++++++++------------------
tests/qemu-iotests/083.out | 34 +++++++++++++++++-----------------
tests/qemu-iotests/088.out | 12 ++++++------
tests/qemu-iotests/092.out | 24 ++++++++++++------------
tests/qemu-iotests/103.out | 8 ++++----
tests/qemu-iotests/114.out | 2 +-
tests/qemu-iotests/116.out | 14 +++++++-------
tests/qemu-iotests/131.out | 2 +-
17 files changed, 93 insertions(+), 95 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 269f17c..d47228a 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -57,17 +57,15 @@ static int openfile(char *name, int flags, QDict *opts)
BlockDriverState *bs;
if (qemuio_blk) {
- fprintf(stderr, "file open already, try 'help close'\n");
+ error_report("file open already, try 'help close'");
QDECREF(opts);
return 1;
}
qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, &local_err);
if (!qemuio_blk) {
- fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
- name ? " device " : "", name ?: "",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_reportf_err(local_err, "can't open%s%s: ",
+ name ? " device " : "", name ?: "");
return 1;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index f9fce4a..99df01f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -257,7 +257,7 @@ static void *nbd_client_thread(void *arg)
fd = open(device, O_RDWR);
if (fd < 0) {
/* Linux-only, we can use %m in printf. */
- fprintf(stderr, "Failed to open %s: %m\n", device);
+ error_report("Failed to open %s: %m", device);
goto out_socket;
}
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 00057fe..d28df5b 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2,17 +2,17 @@ QA output created by 059
=== Testing invalid granularity ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.vmdk: Invalid granularity, image may be corrupt
+can't open device TEST_DIR/t.vmdk: Invalid granularity, image may be corrupt
no file open, try 'help open'
=== Testing too big L2 table size ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.vmdk: L2 table size too big
+can't open device TEST_DIR/t.vmdk: L2 table size too big
no file open, try 'help open'
=== Testing too big L1 table size ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big
+can't open device TEST_DIR/t.vmdk: L1 size too big
no file open, try 'help open'
=== Testing monolithicFlat creation and opening ===
@@ -2055,7 +2055,7 @@ wrote 512/512 bytes at offset 10240
=== Testing monolithicFlat with internally generated JSON file name ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 subformat=monolithicFlat
-qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'
+can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'
=== Testing version 3 ===
image: TEST_DIR/iotest-version3.IMGFMT
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 7511189..5d40206 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -20,7 +20,7 @@ Format specific information:
lazy refcounts: false
refcount bits: 16
corrupt: true
-qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write
+can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write
read 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/qemu-iotests/069.out b/tests/qemu-iotests/069.out
index c78e8c2..f975856 100644
--- a/tests/qemu-iotests/069.out
+++ b/tests/qemu-iotests/069.out
@@ -4,5 +4,5 @@ QA output created by 069
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=131072
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file=TEST_DIR/t.IMGFMT.base
-qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open backing file: Could not open 'TEST_DIR/t.IMGFMT.base': No such file or directory
+can't open device TEST_DIR/t.IMGFMT: Could not open backing file: Could not open 'TEST_DIR/t.IMGFMT.base': No such file or directory
*** done
diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
index ca74383..ffd4251 100644
--- a/tests/qemu-iotests/070.out
+++ b/tests/qemu-iotests/070.out
@@ -1,7 +1,7 @@
QA output created by 070
=== Verify open image read-only fails, due to dirty log ===
-qemu-io: can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed. To replay the log, execute:
+can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed. To replay the log, execute:
qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx': Operation not permitted
no file open, try 'help open'
=== Verify open image replays log ===
diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out
index 5f1d6c1..87beae4 100644
--- a/tests/qemu-iotests/075.out
+++ b/tests/qemu-iotests/075.out
@@ -9,30 +9,30 @@ read 512/512 bytes at offset 1048064
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== block_size must be a multiple of 512 ==
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size 513 must be a multiple of 512
+can't open device TEST_DIR/simple-pattern.cloop: block_size 513 must be a multiple of 512
no file open, try 'help open'
== block_size cannot be zero ==
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size cannot be zero
+can't open device TEST_DIR/simple-pattern.cloop: block_size cannot be zero
no file open, try 'help open'
== huge block_size ===
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size 4294966784 must be 64 MB or less
+can't open device TEST_DIR/simple-pattern.cloop: block_size 4294966784 must be 64 MB or less
no file open, try 'help open'
== offsets_size overflow ===
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: n_blocks 4294967295 must be 536870911 or less
+can't open device TEST_DIR/simple-pattern.cloop: n_blocks 4294967295 must be 536870911 or less
no file open, try 'help open'
== refuse images that require too many offsets ===
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: image requires too many offsets, try increasing block size
+can't open device TEST_DIR/simple-pattern.cloop: image requires too many offsets, try increasing block size
no file open, try 'help open'
== refuse images with non-monotonically increasing offsets ==
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: offsets not monotonically increasing at index 1, image file is corrupt
+can't open device TEST_DIR/simple-pattern.cloop: offsets not monotonically increasing at index 1, image file is corrupt
no file open, try 'help open'
== refuse images with invalid compressed block size ==
-qemu-io: can't open device TEST_DIR/simple-pattern.cloop: invalid compressed block size at index 1, image file is corrupt
+can't open device TEST_DIR/simple-pattern.cloop: invalid compressed block size at index 1, image file is corrupt
no file open, try 'help open'
*** done
diff --git a/tests/qemu-iotests/076.out b/tests/qemu-iotests/076.out
index b0000ae..72645b2 100644
--- a/tests/qemu-iotests/076.out
+++ b/tests/qemu-iotests/076.out
@@ -5,15 +5,15 @@ read 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== Negative catalog size ==
-qemu-io: can't open device TEST_DIR/parallels-v1: Catalog too large
+can't open device TEST_DIR/parallels-v1: Catalog too large
no file open, try 'help open'
== Overflow in catalog allocation ==
-qemu-io: can't open device TEST_DIR/parallels-v1: Catalog too large
+can't open device TEST_DIR/parallels-v1: Catalog too large
no file open, try 'help open'
== Zero sectors per track ==
-qemu-io: can't open device TEST_DIR/parallels-v1: Invalid image: Zero sectors per track
+can't open device TEST_DIR/parallels-v1: Invalid image: Zero sectors per track
no file open, try 'help open'
== Read from a valid v2 image ==
diff --git a/tests/qemu-iotests/078.out b/tests/qemu-iotests/078.out
index ca18d2e..42b8a83 100644
--- a/tests/qemu-iotests/078.out
+++ b/tests/qemu-iotests/078.out
@@ -5,24 +5,24 @@ read 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== Negative catalog size ==
-qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too large
+can't open device TEST_DIR/empty.bochs: Catalog size is too large
no file open, try 'help open'
== Overflow for catalog size * sizeof(uint32_t) ==
-qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too large
+can't open device TEST_DIR/empty.bochs: Catalog size is too large
no file open, try 'help open'
== Too small catalog bitmap for image size ==
-qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size
+can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size
+can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size
no file open, try 'help open'
== Negative extent size ==
-qemu-io: can't open device TEST_DIR/empty.bochs: Extent size 2147483648 is too large
+can't open device TEST_DIR/empty.bochs: Extent size 2147483648 is too large
no file open, try 'help open'
== Zero extent size ==
-qemu-io: can't open device TEST_DIR/empty.bochs: Extent size must be at least 512
+can't open device TEST_DIR/empty.bochs: Extent size must be at least 512
no file open, try 'help open'
*** done
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 6061d84..0daac48 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -2,46 +2,46 @@ QA output created by 080
== Huge header size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
+can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
+can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
no file open, try 'help open'
== Huge unknown header extension ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid backing file offset
+can't open device TEST_DIR/t.qcow2: Invalid backing file offset
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Header extension too large
+can't open device TEST_DIR/t.qcow2: Header extension too large
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Header extension too large
+can't open device TEST_DIR/t.qcow2: Header extension too large
no file open, try 'help open'
== Huge refcount table size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Reference count table too large
+can't open device TEST_DIR/t.qcow2: Reference count table too large
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Reference count table too large
+can't open device TEST_DIR/t.qcow2: Reference count table too large
no file open, try 'help open'
== Misaligned refcount table ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
+can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
no file open, try 'help open'
== Huge refcount offset ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
+can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
no file open, try 'help open'
== Invalid snapshot table ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Too many snapshots
+can't open device TEST_DIR/t.qcow2: Too many snapshots
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Too many snapshots
+can't open device TEST_DIR/t.qcow2: Too many snapshots
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset
+can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset
+can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset
no file open, try 'help open'
== Hitting snapshot table size limit ==
@@ -52,13 +52,13 @@ read 512/512 bytes at offset 0
== Invalid L1 table ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Active L1 table too large
+can't open device TEST_DIR/t.qcow2: Active L1 table too large
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Active L1 table too large
+can't open device TEST_DIR/t.qcow2: Active L1 table too large
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
+can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
+can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
no file open, try 'help open'
== Invalid L1 table (with internal snapshot in the image) ==
@@ -67,7 +67,7 @@ qemu-img: Could not open 'TEST_DIR/t.IMGFMT': L1 table is too small
== Invalid backing file size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow2: Backing file name too long
+can't open device TEST_DIR/t.qcow2: Backing file name too long
no file open, try 'help open'
== Invalid L2 entry (huge physical offset) ==
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
index 8c1441b..78cc49a 100644
--- a/tests/qemu-iotests/083.out
+++ b/tests/qemu-iotests/083.out
@@ -1,52 +1,52 @@
QA output created by 083
=== Check disconnect before neg1 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect after neg1 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect 8 neg1 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect 16 neg1 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect before export ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect after export ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect 4 export ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect 12 export ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect 16 export ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect before neg2 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect after neg2 ===
@@ -56,12 +56,12 @@ read failed: Input/output error
=== Check disconnect 8 neg2 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect 10 neg2 ===
-qemu-io: can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd:127.0.0.1:PORT:exportname=foo
no file open, try 'help open'
=== Check disconnect before request ===
@@ -107,27 +107,27 @@ read 512/512 bytes at offset 0
=== Check disconnect before neg-classic ===
-qemu-io: can't open device nbd:127.0.0.1:PORT
+can't open device nbd:127.0.0.1:PORT
no file open, try 'help open'
=== Check disconnect 8 neg-classic ===
-qemu-io: can't open device nbd:127.0.0.1:PORT
+can't open device nbd:127.0.0.1:PORT
no file open, try 'help open'
=== Check disconnect 16 neg-classic ===
-qemu-io: can't open device nbd:127.0.0.1:PORT
+can't open device nbd:127.0.0.1:PORT
no file open, try 'help open'
=== Check disconnect 24 neg-classic ===
-qemu-io: can't open device nbd:127.0.0.1:PORT
+can't open device nbd:127.0.0.1:PORT
no file open, try 'help open'
=== Check disconnect 28 neg-classic ===
-qemu-io: can't open device nbd:127.0.0.1:PORT
+can't open device nbd:127.0.0.1:PORT
no file open, try 'help open'
=== Check disconnect after neg-classic ===
diff --git a/tests/qemu-iotests/088.out b/tests/qemu-iotests/088.out
index 6e6bfca..a2a83b8 100644
--- a/tests/qemu-iotests/088.out
+++ b/tests/qemu-iotests/088.out
@@ -2,16 +2,16 @@ QA output created by 088
== Invalid block size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 0
+can't open device TEST_DIR/t.vpc: Invalid block size 0
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 0
+can't open device TEST_DIR/t.vpc: Invalid block size 0
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 128
+can't open device TEST_DIR/t.vpc: Invalid block size 128
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 128
+can't open device TEST_DIR/t.vpc: Invalid block size 128
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 305419896
+can't open device TEST_DIR/t.vpc: Invalid block size 305419896
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 305419896
+can't open device TEST_DIR/t.vpc: Invalid block size 305419896
no file open, try 'help open'
*** done
diff --git a/tests/qemu-iotests/092.out b/tests/qemu-iotests/092.out
index c5c60f9..e18f54c 100644
--- a/tests/qemu-iotests/092.out
+++ b/tests/qemu-iotests/092.out
@@ -2,37 +2,37 @@ QA output created by 092
== Invalid cluster size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k
no file open, try 'help open'
== Invalid L2 table size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
+can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k
no file open, try 'help open'
== Invalid size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow: Image too large
+can't open device TEST_DIR/t.qcow: Image too large
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: Image too large
+can't open device TEST_DIR/t.qcow: Image too large
no file open, try 'help open'
== Invalid backing file length ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open device TEST_DIR/t.qcow: Backing file name too long
+can't open device TEST_DIR/t.qcow: Backing file name too long
no file open, try 'help open'
-qemu-io: can't open device TEST_DIR/t.qcow: Backing file name too long
+can't open device TEST_DIR/t.qcow: Backing file name too long
no file open, try 'help open'
*** done
diff --git a/tests/qemu-iotests/103.out b/tests/qemu-iotests/103.out
index d05f49f..b7aaadf 100644
--- a/tests/qemu-iotests/103.out
+++ b/tests/qemu-iotests/103.out
@@ -5,10 +5,10 @@ wrote 65536/65536 bytes at offset 0
=== Testing invalid option combinations ===
-qemu-io: can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
-qemu-io: can't open device TEST_DIR/t.IMGFMT: l2-cache-size may not exceed cache-size
-qemu-io: can't open device TEST_DIR/t.IMGFMT: refcount-cache-size may not exceed cache-size
-qemu-io: can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
+can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
+can't open device TEST_DIR/t.IMGFMT: l2-cache-size may not exceed cache-size
+can't open device TEST_DIR/t.IMGFMT: refcount-cache-size may not exceed cache-size
+can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
=== Testing valid option combinations ===
diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out
index 6a2c750..b6d10e4 100644
--- a/tests/qemu-iotests/114.out
+++ b/tests/qemu-iotests/114.out
@@ -7,7 +7,7 @@ virtual size: 64M (67108864 bytes)
cluster_size: 65536
backing file: TEST_DIR/t.IMGFMT.base
backing file format: foo
-qemu-io: can't open device TEST_DIR/t.qcow2: Could not open backing file: Unknown driver 'foo'
+can't open device TEST_DIR/t.qcow2: Could not open backing file: Unknown driver 'foo'
read 4096/4096 bytes at offset 0
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
diff --git a/tests/qemu-iotests/116.out b/tests/qemu-iotests/116.out
index b679cee..1f11d44 100644
--- a/tests/qemu-iotests/116.out
+++ b/tests/qemu-iotests/116.out
@@ -2,36 +2,36 @@ QA output created by 116
== truncated header cluster ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
no file open, try 'help open'
== invalid header magic ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Image not in QED format
+can't open device TEST_DIR/t.qed: Image not in QED format
no file open, try 'help open'
== invalid cluster size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
no file open, try 'help open'
== invalid table size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
no file open, try 'help open'
== invalid header size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
no file open, try 'help open'
== invalid L1 table offset ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
no file open, try 'help open'
== invalid image size ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
no file open, try 'help open'
*** done
diff --git a/tests/qemu-iotests/131.out b/tests/qemu-iotests/131.out
index 021a04c..ae2412e 100644
--- a/tests/qemu-iotests/131.out
+++ b/tests/qemu-iotests/131.out
@@ -22,7 +22,7 @@ read 32768/32768 bytes at offset 163840
read 32768/32768 bytes at offset 0
32 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== Corrupt image ==
-qemu-io: can't open device TEST_DIR/t.parallels: parallels: Image was not closed correctly; cannot be opened read/write
+can't open device TEST_DIR/t.parallels: parallels: Image was not closed correctly; cannot be opened read/write
no file open, try 'help open'
ERROR image was not closed correctly
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 32/41] error: Strip trailing '\n' from error string arguments (again)
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (30 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 31/41] qemu-io qemu-nbd: Use error_report() etc. instead of fprintf() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 33/41] vmdk: Clean up control flow in vmdk_parse_extents() a bit Markus Armbruster
` (9 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, zhanghailiang, Stefan Berger, Markus Armbruster,
Pavel Fedin, Dr. David Alan Gilbert, Dominik Dingel,
David Hildenbrand, Peter Crosthwaite, Jason J. Herne,
Bharata B Rao, Changchun Ouyang
Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
keep coming back. Tracked down with the Coccinelle semantic patch
from commit 312fd5f.
Cc: Fam Zheng <famz@redhat.com>
Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Changchun Ouyang <changchun.ouyang@intel.com>
Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
Cc: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Acked-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-17-git-send-email-armbru@redhat.com>
---
block/vmdk.c | 4 ++--
hw/arm/xlnx-zynqmp.c | 2 +-
hw/ppc/spapr.c | 3 ++-
hw/s390x/ipl.c | 8 ++++----
hw/s390x/s390-skeys-kvm.c | 2 +-
hw/s390x/s390-skeys.c | 16 ++++++++--------
hw/tpm/tpm_tis.c | 2 +-
migration/ram.c | 2 +-
migration/savevm.c | 4 ++--
net/vhost-user.c | 6 +++---
qemu-nbd.c | 4 ++--
qga/commands-posix.c | 2 +-
target-arm/cpu.c | 2 +-
target-arm/machine.c | 4 ++--
14 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 6f819e4..b4a224e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1494,8 +1494,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
if (sector_num > bs->total_sectors) {
error_report("Wrong offset: sector_num=0x%" PRIx64
- " total_sectors=0x%" PRIx64 "\n",
- sector_num, bs->total_sectors);
+ " total_sectors=0x%" PRIx64,
+ sector_num, bs->total_sectors);
return -EIO;
}
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 87553bb..20a3b2b 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -227,7 +227,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
}
if (!s->boot_cpu_ptr) {
- error_setg(errp, "ZynqMP Boot cpu %s not found\n", boot_cpu);
+ error_setg(errp, "ZynqMP Boot cpu %s not found", boot_cpu);
return;
}
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 091cdb1..50e5a26 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1836,7 +1836,8 @@ static void ppc_spapr_init(MachineState *machine)
ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
- error_report("Specified number of memory slots %"PRIu64" exceeds max supported %d\n",
+ error_report("Specified number of memory slots %" PRIu64
+ " exceeds max supported %d",
machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
exit(EXIT_FAILURE);
}
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index b91fcc6..e100428 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -94,7 +94,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
if (bios_filename == NULL) {
- error_setg(&l_err, "could not find stage1 bootloader\n");
+ error_setg(&l_err, "could not find stage1 bootloader");
goto error;
}
@@ -113,7 +113,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
g_free(bios_filename);
if (bios_size == -1) {
- error_setg(&l_err, "could not load bootloader '%s'\n", bios_name);
+ error_setg(&l_err, "could not load bootloader '%s'", bios_name);
goto error;
}
@@ -128,7 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
}
if (kernel_size < 0) {
- error_setg(&l_err, "could not load kernel '%s'\n", ipl->kernel);
+ error_setg(&l_err, "could not load kernel '%s'", ipl->kernel);
goto error;
}
/*
@@ -156,7 +156,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
ram_size - initrd_offset);
if (initrd_size == -1) {
- error_setg(&l_err, "could not load initrd '%s'\n", ipl->initrd);
+ error_setg(&l_err, "could not load initrd '%s'", ipl->initrd);
goto error;
}
diff --git a/hw/s390x/s390-skeys-kvm.c b/hw/s390x/s390-skeys-kvm.c
index 682949a..eaa37ba 100644
--- a/hw/s390x/s390-skeys-kvm.c
+++ b/hw/s390x/s390-skeys-kvm.c
@@ -21,7 +21,7 @@ static int kvm_s390_skeys_enabled(S390SKeysState *ss)
r = skeyclass->get_skeys(ss, 0, 1, &single_key);
if (r != 0 && r != KVM_S390_GET_SKEYS_NONE) {
- error_report("S390_GET_KEYS error %d\n", r);
+ error_report("S390_GET_KEYS error %d", r);
}
return (r == 0);
}
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 4af1558..f2b732e 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -191,8 +191,8 @@ static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
/* Check for uint64 overflow and access beyond end of key data */
if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
error_report("Error: Setting storage keys for page beyond the end "
- "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
- count);
+ "of memory: gfn=%" PRIx64 " count=%" PRId64,
+ start_gfn, count);
return -EINVAL;
}
@@ -211,8 +211,8 @@ static int qemu_s390_skeys_get(S390SKeysState *ss, uint64_t start_gfn,
/* Check for uint64 overflow and access beyond end of key data */
if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
error_report("Error: Getting storage keys for page beyond the end "
- "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
- count);
+ "of memory: gfn=%" PRIx64 " count=%" PRId64,
+ start_gfn, count);
return -EINVAL;
}
@@ -256,7 +256,7 @@ static void s390_storage_keys_save(QEMUFile *f, void *opaque)
buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
if (!buf) {
- error_report("storage key save could not allocate memory\n");
+ error_report("storage key save could not allocate memory");
goto end_stream;
}
@@ -276,7 +276,7 @@ static void s390_storage_keys_save(QEMUFile *f, void *opaque)
* use S390_SKEYS_SAVE_FLAG_ERROR to indicate failure to the
* reading side.
*/
- error_report("S390_GET_KEYS error %d\n", error);
+ error_report("S390_GET_KEYS error %d", error);
memset(buf, 0, S390_SKEYS_BUFFER_SIZE);
eos = S390_SKEYS_SAVE_FLAG_ERROR;
}
@@ -314,7 +314,7 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
uint8_t *buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
if (!buf) {
- error_report("storage key load could not allocate memory\n");
+ error_report("storage key load could not allocate memory");
ret = -ENOMEM;
break;
}
@@ -326,7 +326,7 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
ret = skeyclass->set_skeys(ss, cur_gfn, cur_count, buf);
if (ret < 0) {
- error_report("S390_SET_KEYS error %d\n", ret);
+ error_report("S390_SET_KEYS error %d", ret);
break;
}
handled_count += cur_count;
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index ff073d5..95fc66e 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -1051,7 +1051,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
if (tis->irq_num > 15) {
error_setg(errp, "tpm_tis: IRQ %d for TPM TIS is outside valid range "
- "of 0 to 15.\n", tis->irq_num);
+ "of 0 to 15", tis->irq_num);
return;
}
diff --git a/migration/ram.c b/migration/ram.c
index 0490f00..5bfcca3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2323,7 +2323,7 @@ static int ram_load_postcopy(QEMUFile *f)
} else {
/* not the 1st TP within the HP */
if (host != (last_host + TARGET_PAGE_SIZE)) {
- error_report("Non-sequential target page %p/%p\n",
+ error_report("Non-sequential target page %p/%p",
host, last_host);
ret = -EINVAL;
break;
diff --git a/migration/savevm.c b/migration/savevm.c
index bcaeb70..540eb28 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1563,8 +1563,8 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
ret = qemu_get_buffer(mis->from_src_file, buffer, (int)length);
if (ret != length) {
g_free(buffer);
- error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d\n",
- ret, length);
+ error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d",
+ ret, length);
return (ret < 0) ? ret : -EAGAIN;
}
trace_loadvm_handle_cmd_packaged_received(ret);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index b368a90..e4dd089 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -82,15 +82,15 @@ static int vhost_user_start(int queues, NetClientState *ncs[])
options.opaque = s->chr;
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
- error_report("failed to init vhost_net for queue %d\n", i);
+ error_report("failed to init vhost_net for queue %d", i);
goto err;
}
if (i == 0) {
max_queues = vhost_net_get_max_queues(s->vhost_net);
if (queues > max_queues) {
- error_report("you are asking more queues than "
- "supported: %d\n", max_queues);
+ error_report("you are asking more queues than supported: %d",
+ max_queues);
goto err;
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 99df01f..023eacd 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -554,7 +554,7 @@ int main(int argc, char **argv)
case 'k':
sockpath = optarg;
if (sockpath[0] != '/') {
- error_report("socket path must be absolute\n");
+ error_report("socket path must be absolute");
exit(EXIT_FAILURE);
}
break;
@@ -571,7 +571,7 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
if (shared < 1) {
- error_report("Shared device number must be greater than 0\n");
+ error_report("Shared device number must be greater than 0");
exit(EXIT_FAILURE);
}
break;
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 8fe708f..5f91275 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2254,7 +2254,7 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
*/
if (errno != ENOENT) {
error_setg_errno(errp, errno, "Can't open directory"
- "\"/sys/devices/system/memory/\"\n");
+ "\"/sys/devices/system/memory/\"");
}
return NULL;
}
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 35a1f12..775e0a4 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -649,7 +649,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
uint32_t nr = cpu->pmsav7_dregion;
if (nr > 0xff) {
- error_setg(errp, "PMSAv7 MPU #regions invalid %" PRIu32 "\n", nr);
+ error_setg(errp, "PMSAv7 MPU #regions invalid %" PRIu32, nr);
return;
}
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 36a0d15..b1e1418 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -337,11 +337,11 @@ const char *gicv3_class_name(void)
return "kvm-arm-gicv3";
#else
error_report("KVM GICv3 acceleration is not supported on this "
- "platform\n");
+ "platform");
#endif
} else {
/* TODO: Software emulation is not implemented yet */
- error_report("KVM is currently required for GICv3 emulation\n");
+ error_report("KVM is currently required for GICv3 emulation");
}
exit(1);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 33/41] vmdk: Clean up control flow in vmdk_parse_extents() a bit
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (31 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 32/41] error: Strip trailing '\n' from error string arguments (again) Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 34/41] vmdk: Clean up "Invalid extent lines" error message Markus Armbruster
` (8 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
Factor out loop stepping to turn a while-loop with goto into a
for-loop with continue.
Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-Id: <1450452927-8346-18-git-send-email-armbru@redhat.com>
---
block/vmdk.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index b4a224e..08fa3f3 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -760,6 +760,17 @@ static int vmdk_open_sparse(BlockDriverState *bs, BdrvChild *file, int flags,
}
}
+static const char *next_line(const char *s)
+{
+ while (*s) {
+ if (*s == '\n') {
+ return s + 1;
+ }
+ s++;
+ }
+ return s;
+}
+
static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
const char *desc_file_path, QDict *options,
Error **errp)
@@ -769,7 +780,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
char access[11];
char type[11];
char fname[512];
- const char *p = desc;
+ const char *p;
int64_t sectors = 0;
int64_t flat_offset;
char *extent_path;
@@ -779,7 +790,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
char extent_opt_prefix[32];
Error *local_err = NULL;
- while (*p) {
+ for (p = desc; *p; p = next_line(p)) {
/* parse extent line in one of below formats:
*
* RW [size in sectors] FLAT "file-name.vmdk" OFFSET
@@ -791,7 +802,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
matches = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
access, §ors, type, fname, &flat_offset);
if (matches < 4 || strcmp(access, "RW")) {
- goto next_line;
+ continue;
} else if (!strcmp(type, "FLAT")) {
if (matches != 5 || flat_offset < 0) {
error_setg(errp, "Invalid extent lines: \n%s", p);
@@ -813,7 +824,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
(strcmp(type, "FLAT") && strcmp(type, "SPARSE") &&
strcmp(type, "VMFS") && strcmp(type, "VMFSSPARSE")) ||
(strcmp(access, "RW"))) {
- goto next_line;
+ continue;
}
if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
@@ -870,15 +881,6 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
return -ENOTSUP;
}
extent->type = g_strdup(type);
-next_line:
- /* move to next line */
- while (*p) {
- if (*p == '\n') {
- p++;
- break;
- }
- p++;
- }
}
return 0;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 34/41] vmdk: Clean up "Invalid extent lines" error message
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (32 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 33/41] vmdk: Clean up control flow in vmdk_parse_extents() a bit Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 35/41] pci-assign: Clean up "Failed to assign" error messages Markus Armbruster
` (7 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
vmdk_parse_extents() reports parse errors like this:
error_setg(errp, "Invalid extent lines:\n%s", p);
where p points to the beginning of the malformed line in the image
descriptor. This results in a multi-line error message
Invalid extent lines:
<first line that doesn't parse>
<remaining text that may or may not parse, if any>
Error messages should not have newlines embedded. Since the remaining
text is not helpful, we can simply report:
Invalid extent line: <first line that doesn't parse>
Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-19-git-send-email-armbru@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
block/vmdk.c | 20 +++++++++++++-------
tests/qemu-iotests/059.out | 4 +---
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 08fa3f3..2b5cb00 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -780,7 +780,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
char access[11];
char type[11];
char fname[512];
- const char *p;
+ const char *p, *np;
int64_t sectors = 0;
int64_t flat_offset;
char *extent_path;
@@ -805,19 +805,16 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
continue;
} else if (!strcmp(type, "FLAT")) {
if (matches != 5 || flat_offset < 0) {
- error_setg(errp, "Invalid extent lines: \n%s", p);
- return -EINVAL;
+ goto invalid;
}
} else if (!strcmp(type, "VMFS")) {
if (matches == 4) {
flat_offset = 0;
} else {
- error_setg(errp, "Invalid extent lines:\n%s", p);
- return -EINVAL;
+ goto invalid;
}
} else if (matches != 4) {
- error_setg(errp, "Invalid extent lines:\n%s", p);
- return -EINVAL;
+ goto invalid;
}
if (sectors <= 0 ||
@@ -883,6 +880,15 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
extent->type = g_strdup(type);
}
return 0;
+
+invalid:
+ np = next_line(p);
+ assert(np != p);
+ if (np[-1] == '\n') {
+ np--;
+ }
+ error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
+ return -EINVAL;
}
static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index d28df5b..9d506cb 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2038,9 +2038,7 @@ Format specific information:
format: FLAT
=== Testing malformed VMFS extent description line ===
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent lines:
-RW 12582912 VMFS "dummy.IMGFMT" 1
-
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW 12582912 VMFS "dummy.IMGFMT" 1
=== Testing truncated sparse ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 subformat=monolithicSparse
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 35/41] pci-assign: Clean up "Failed to assign" error messages
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (33 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 34/41] vmdk: Clean up "Invalid extent lines" error message Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 36/41] vhdx: Fix "log that needs to be replayed" error message Markus Armbruster
` (6 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Laszlo Ersek
The arguments of error_setg() & friends should yield a short error
string without newlines.
Two places try to append additional help to the error message by
embedding newlines in the error string. That's nice, but let's do it
the right way, with error_append_hint().
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-20-git-send-email-armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
hw/i386/kvm/pci-assign.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 0fd6923..eec1340 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -770,7 +770,7 @@ static char *assign_failed_examine(const AssignedDevice *dev)
"*** $ echo \"%04x:%02x:%02x.%x\" > /sys/bus/pci/drivers/"
"pci-stub/bind\n"
"*** $ echo \"%04x %04x\" > /sys/bus/pci/drivers/pci-stub/remove_id\n"
- "***",
+ "***\n",
ns, dev->host.domain, dev->host.bus, dev->host.slot,
dev->host.function, vendor_id, device_id,
dev->host.domain, dev->host.bus, dev->host.slot, dev->host.function,
@@ -778,7 +778,7 @@ static char *assign_failed_examine(const AssignedDevice *dev)
dev->host.function, vendor_id, device_id);
fail:
- return g_strdup("Couldn't find out why.");
+ return g_strdup("Couldn't find out why.\n");
}
static void assign_device(AssignedDevice *dev, Error **errp)
@@ -812,8 +812,9 @@ static void assign_device(AssignedDevice *dev, Error **errp)
char *cause;
cause = assign_failed_examine(dev);
- error_setg_errno(errp, -r, "Failed to assign device \"%s\"\n%s",
- dev->dev.qdev.id, cause);
+ error_setg_errno(errp, -r, "Failed to assign device \"%s\"",
+ dev->dev.qdev.id);
+ error_append_hint(errp, "%s", cause);
g_free(cause);
break;
}
@@ -912,11 +913,10 @@ retry:
dev->features |= ASSIGNED_DEVICE_PREFER_MSI_MASK;
goto retry;
}
- error_setg_errno(errp, -r,
- "Failed to assign irq for \"%s\"\n"
- "Perhaps you are assigning a device "
- "that shares an IRQ with another device?",
+ error_setg_errno(errp, -r, "Failed to assign irq for \"%s\"",
dev->dev.qdev.id);
+ error_append_hint(errp, "Perhaps you are assigning a device "
+ "that shares an IRQ with another device?\n");
return r;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 36/41] vhdx: Fix "log that needs to be replayed" error message
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (34 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 35/41] pci-assign: Clean up "Failed to assign" error messages Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 37/41] error: Clean up errors with embedded newlines (again) Markus Armbruster
` (5 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
The arguments of error_setg_errno() should yield a short error string
without newlines.
Here, we try to append additional help to the error message by
embedding newlines in the error string. That's nice, but it's doesn't
play nicely with the errno part. tests/qemu-iotests/070.out shows the
resulting mess:
can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed. To replay the log, execute:
qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx': Operation not permitted
Switch to error_setg() and error_append_hint(). Result:
can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed
To replay the log, run:
qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx'
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-21-git-send-email-armbru@redhat.com>
---
block/vhdx-log.c | 13 +++++++------
tests/qemu-iotests/070.out | 5 +++--
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index 47ae4b1..ab86416 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -784,12 +784,13 @@ int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed,
if (logs.valid) {
if (bs->read_only) {
ret = -EPERM;
- error_setg_errno(errp, EPERM,
- "VHDX image file '%s' opened read-only, but "
- "contains a log that needs to be replayed. To "
- "replay the log, execute:\n qemu-img check -r "
- "all '%s'",
- bs->filename, bs->filename);
+ error_setg(errp,
+ "VHDX image file '%s' opened read-only, but "
+ "contains a log that needs to be replayed",
+ bs->filename);
+ error_append_hint(errp, "To replay the log, run:\n"
+ "qemu-img check -r all '%s'\n",
+ bs->filename);
goto exit;
}
/* now flush the log */
diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
index ffd4251..131a5b1 100644
--- a/tests/qemu-iotests/070.out
+++ b/tests/qemu-iotests/070.out
@@ -1,8 +1,9 @@
QA output created by 070
=== Verify open image read-only fails, due to dirty log ===
-can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed. To replay the log, execute:
- qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx': Operation not permitted
+can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed
+To replay the log, run:
+qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx'
no file open, try 'help open'
=== Verify open image replays log ===
read 18874368/18874368 bytes at offset 0
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 37/41] error: Clean up errors with embedded newlines (again)
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (35 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 36/41] vhdx: Fix "log that needs to be replayed" error message Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 38/41] hw/s390x: Rename local variables Error *l_err to just err Markus Armbruster
` (4 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Pavel Fedin, Markus Armbruster, Laszlo Ersek
The arguments of error_report() should yield a short error string
without newlines.
A few places try to print additional help after the error message by
embedding newlines in the error string. That's nice, but let's do it
the right way. Commit 474c213 cleaned up some, but they keep coming
back. Offenders tracked down with the Coccinelle semantic patch from
commit 312fd5f.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/i386/pc.c | 4 ++--
kvm-all.c | 6 +++---
qemu-nbd.c | 5 ++---
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0e5c86a..9e37186 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -377,8 +377,8 @@ ISADevice *pc_find_fdc0(void)
if (state.multiple) {
error_report("warning: multiple floppy disk controllers with "
- "iobase=0x3f0 have been found;\n"
- "the one being picked for CMOS setup might not reflect "
+ "iobase=0x3f0 have been found");
+ error_printf("the one being picked for CMOS setup might not reflect "
"your intent");
}
diff --git a/kvm-all.c b/kvm-all.c
index bd9e764..9148889 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2063,9 +2063,9 @@ void kvm_device_access(int fd, int group, uint64_t attr,
write ? KVM_SET_DEVICE_ATTR : KVM_GET_DEVICE_ATTR,
&kvmattr);
if (err < 0) {
- error_report("KVM_%s_DEVICE_ATTR failed: %s\n"
- "Group %d attr 0x%016" PRIx64, write ? "SET" : "GET",
- strerror(-err), group, attr);
+ error_report("KVM_%s_DEVICE_ATTR failed: %s",
+ write ? "SET" : "GET", strerror(-err));
+ error_printf("Group %d attr 0x%016" PRIx64, group, attr);
abort();
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 023eacd..a4cf847 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -599,9 +599,8 @@ int main(int argc, char **argv)
}
if ((argc - optind) != 1) {
- error_report("Invalid number of argument.\n"
- "Try `%s --help' for more information.",
- argv[0]);
+ error_report("Invalid number of arguments");
+ error_printf("Try `%s --help' for more information.\n", argv[0]);
exit(EXIT_FAILURE);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 38/41] hw/s390x: Rename local variables Error *l_err to just err
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (36 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 37/41] error: Clean up errors with embedded newlines (again) Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 39/41] s390/sclp: Simplify control flow in sclp_realize() Markus Armbruster
` (3 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand, Markus Armbruster
Let's follow established naming practice here as well.
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-23-git-send-email-armbru@redhat.com>
---
hw/s390x/ipl.c | 12 ++++++------
hw/s390x/sclp.c | 14 +++++++-------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index e100428..9c01be5 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -76,7 +76,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
S390IPLState *ipl = S390_IPL(dev);
uint64_t pentry = KERN_IMAGE_START;
int kernel_size;
- Error *l_err = NULL;
+ Error *err = NULL;
int bios_size;
char *bios_filename;
@@ -94,7 +94,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
if (bios_filename == NULL) {
- error_setg(&l_err, "could not find stage1 bootloader");
+ error_setg(&err, "could not find stage1 bootloader");
goto error;
}
@@ -113,7 +113,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
g_free(bios_filename);
if (bios_size == -1) {
- error_setg(&l_err, "could not load bootloader '%s'", bios_name);
+ error_setg(&err, "could not load bootloader '%s'", bios_name);
goto error;
}
@@ -128,7 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
}
if (kernel_size < 0) {
- error_setg(&l_err, "could not load kernel '%s'", ipl->kernel);
+ error_setg(&err, "could not load kernel '%s'", ipl->kernel);
goto error;
}
/*
@@ -156,7 +156,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
ram_size - initrd_offset);
if (initrd_size == -1) {
- error_setg(&l_err, "could not load initrd '%s'", ipl->initrd);
+ error_setg(&err, "could not load initrd '%s'", ipl->initrd);
goto error;
}
@@ -170,7 +170,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
}
qemu_register_reset(qdev_reset_all_fn, dev);
error:
- error_propagate(errp, l_err);
+ error_propagate(errp, err);
}
static Property s390_ipl_properties[] = {
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index a061b49..9a117c9 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -456,29 +456,29 @@ static void sclp_realize(DeviceState *dev, Error **errp)
{
MachineState *machine = MACHINE(qdev_get_machine());
SCLPDevice *sclp = SCLP(dev);
- Error *l_err = NULL;
+ Error *err = NULL;
uint64_t hw_limit;
int ret;
object_property_set_bool(OBJECT(sclp->event_facility), true, "realized",
- &l_err);
- if (l_err) {
+ &err);
+ if (err) {
goto error;
}
ret = s390_set_memory_limit(machine->maxram_size, &hw_limit);
if (ret == -E2BIG) {
- error_setg(&l_err, "qemu: host supports a maximum of %" PRIu64 " GB",
+ error_setg(&err, "qemu: host supports a maximum of %" PRIu64 " GB",
hw_limit >> 30);
goto error;
} else if (ret) {
- error_setg(&l_err, "qemu: setting the guest size failed");
+ error_setg(&err, "qemu: setting the guest size failed");
goto error;
}
return;
error:
- assert(l_err);
- error_propagate(errp, l_err);
+ assert(err);
+ error_propagate(errp, err);
}
static void sclp_memory_init(SCLPDevice *sclp)
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 39/41] s390/sclp: Simplify control flow in sclp_realize()
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (37 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 38/41] hw/s390x: Rename local variables Error *l_err to just err Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 40/41] error: Consistently name Error * objects err, and not errp Markus Armbruster
` (2 subsequent siblings)
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Suggested-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-24-git-send-email-armbru@redhat.com>
---
hw/s390x/sclp.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 9a117c9..74f2b40 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -463,21 +463,18 @@ static void sclp_realize(DeviceState *dev, Error **errp)
object_property_set_bool(OBJECT(sclp->event_facility), true, "realized",
&err);
if (err) {
- goto error;
+ goto out;
}
ret = s390_set_memory_limit(machine->maxram_size, &hw_limit);
if (ret == -E2BIG) {
error_setg(&err, "qemu: host supports a maximum of %" PRIu64 " GB",
hw_limit >> 30);
- goto error;
} else if (ret) {
error_setg(&err, "qemu: setting the guest size failed");
- goto error;
}
- return;
-error:
- assert(err);
+
+out:
error_propagate(errp, err);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 40/41] error: Consistently name Error * objects err, and not errp
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (38 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 39/41] s390/sclp: Simplify control flow in sclp_realize() Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-13 15:43 ` [Qemu-devel] [PULL 41/41] checkpatch: Detect newlines in error_report and other error functions Markus Armbruster
2016-01-14 14:09 ` [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Peter Maydell
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1450452927-8346-25-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
contrib/ivshmem-server/main.c | 8 ++++----
hmp.c | 32 ++++++++++++++++----------------
hw/core/nmi.c | 10 +++++-----
include/qemu/sockets.h | 2 +-
tests/test-string-output-visitor.c | 6 +++---
5 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index 00508b5..9b0d6e2 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -65,7 +65,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int argc, char *argv[])
{
int c;
unsigned long long v;
- Error *errp = NULL;
+ Error *err = NULL;
while ((c = getopt(argc, argv,
"h" /* help */
@@ -104,9 +104,9 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int argc, char *argv[])
break;
case 'l': /* shm_size */
- parse_option_size("shm_size", optarg, &args->shm_size, &errp);
- if (errp) {
- error_report_err(errp);
+ parse_option_size("shm_size", optarg, &args->shm_size, &err);
+ if (err) {
+ error_report_err(err);
ivshmem_server_usage(argv[0], 1);
}
break;
diff --git a/hmp.c b/hmp.c
index 9723397..54f2620 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2078,11 +2078,11 @@ void hmp_rocker(Monitor *mon, const QDict *qdict)
{
const char *name = qdict_get_str(qdict, "name");
RockerSwitch *rocker;
- Error *errp = NULL;
+ Error *err = NULL;
- rocker = qmp_query_rocker(name, &errp);
- if (errp != NULL) {
- hmp_handle_error(mon, &errp);
+ rocker = qmp_query_rocker(name, &err);
+ if (err != NULL) {
+ hmp_handle_error(mon, &err);
return;
}
@@ -2097,11 +2097,11 @@ void hmp_rocker_ports(Monitor *mon, const QDict *qdict)
{
RockerPortList *list, *port;
const char *name = qdict_get_str(qdict, "name");
- Error *errp = NULL;
+ Error *err = NULL;
- list = qmp_query_rocker_ports(name, &errp);
- if (errp != NULL) {
- hmp_handle_error(mon, &errp);
+ list = qmp_query_rocker_ports(name, &err);
+ if (err != NULL) {
+ hmp_handle_error(mon, &err);
return;
}
@@ -2126,11 +2126,11 @@ void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict)
RockerOfDpaFlowList *list, *info;
const char *name = qdict_get_str(qdict, "name");
uint32_t tbl_id = qdict_get_try_int(qdict, "tbl_id", -1);
- Error *errp = NULL;
+ Error *err = NULL;
- list = qmp_query_rocker_of_dpa_flows(name, tbl_id != -1, tbl_id, &errp);
- if (errp != NULL) {
- hmp_handle_error(mon, &errp);
+ list = qmp_query_rocker_of_dpa_flows(name, tbl_id != -1, tbl_id, &err);
+ if (err != NULL) {
+ hmp_handle_error(mon, &err);
return;
}
@@ -2276,12 +2276,12 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
RockerOfDpaGroupList *list, *g;
const char *name = qdict_get_str(qdict, "name");
uint8_t type = qdict_get_try_int(qdict, "type", 9);
- Error *errp = NULL;
+ Error *err = NULL;
bool set = false;
- list = qmp_query_rocker_of_dpa_groups(name, type != 9, type, &errp);
- if (errp != NULL) {
- hmp_handle_error(mon, &errp);
+ list = qmp_query_rocker_of_dpa_groups(name, type != 9, type, &err);
+ if (err != NULL) {
+ hmp_handle_error(mon, &err);
return;
}
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index de1d1f8..4057cdd 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -25,7 +25,7 @@
struct do_nmi_s {
int cpu_index;
- Error *errp;
+ Error *err;
bool handled;
};
@@ -40,8 +40,8 @@ static int do_nmi(Object *o, void *opaque)
NMIClass *nc = NMI_GET_CLASS(n);
ns->handled = true;
- nc->nmi_monitor_handler(n, ns->cpu_index, &ns->errp);
- if (ns->errp) {
+ nc->nmi_monitor_handler(n, ns->cpu_index, &ns->err);
+ if (ns->err) {
return -1;
}
}
@@ -59,13 +59,13 @@ void nmi_monitor_handle(int cpu_index, Error **errp)
{
struct do_nmi_s ns = {
.cpu_index = cpu_index,
- .errp = NULL,
+ .err = NULL,
.handled = false
};
nmi_children(object_get_root(), &ns);
if (ns.handled) {
- error_propagate(errp, ns.errp);
+ error_propagate(errp, ns.err);
} else {
error_setg(errp, QERR_UNSUPPORTED);
}
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 74c692d..2e7f985 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -53,7 +53,7 @@ int recv_all(int fd, void *buf, int len1, bool single_read);
/* callback function for nonblocking connect
* valid fd on success, negative error code on failure
*/
-typedef void NonBlockingConnectHandler(int fd, Error *errp, void *opaque);
+typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque);
InetSocketAddress *inet_parse(const char *str, Error **errp);
int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 9585252..7aecdfc 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -81,7 +81,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
3, 4, 5, 6, 11, 12, 13, 21, 22, INT64_MAX - 1, INT64_MAX};
intList *list = NULL, **tmp = &list;
int i;
- Error *errp = NULL;
+ Error *err = NULL;
char *str;
for (i = 0; i < sizeof(value) / sizeof(value[0]); i++) {
@@ -90,8 +90,8 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
tmp = &(*tmp)->next;
}
- visit_type_intList(data->ov, &list, NULL, &errp);
- g_assert(errp == NULL);
+ visit_type_intList(data->ov, &list, NULL, &err);
+ g_assert(err == NULL);
str = string_output_get_string(data->sov);
g_assert(str != NULL);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* [Qemu-devel] [PULL 41/41] checkpatch: Detect newlines in error_report and other error functions
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (39 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 40/41] error: Consistently name Error * objects err, and not errp Markus Armbruster
@ 2016-01-13 15:43 ` Markus Armbruster
2016-01-14 14:09 ` [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Peter Maydell
41 siblings, 0 replies; 43+ messages in thread
From: Markus Armbruster @ 2016-01-13 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason J. Herne
From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
We don't want newlines embedded in error messages. This seems to be a common
problem with new code so let's try to catch it with checkpatch.
This will not catch cases where newlines are inserted into the middle of an
existing multi-line statement. But those cases should be rare.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Message-Id: <1449858642-24267-1-git-send-email-jjherne@linux.vnet.ibm.com>
[Rephrased "Error function text" to "Error messages", dropped
error_vprintf, error_printf, error_printf from $qemu_error_funcs,
because they may legitimately print newlines]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index efca817..257126f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2498,6 +2498,42 @@ sub process {
WARN("use QEMU instead of Qemu or QEmu\n" . $herecurr);
}
+# Qemu error function tests
+
+ # Find newlines in error messages
+ my $qemu_error_funcs = qr{error_setg|
+ error_setg_errno|
+ error_setg_win32|
+ error_set|
+ error_vreport|
+ error_report}x;
+
+ if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(\s*\".*\\n/) {
+ WARN("Error messages should not contain newlines\n" . $herecurr);
+ }
+
+ # Continue checking for error messages that contains newlines. This
+ # check handles cases where string literals are spread over multiple lines.
+ # Example:
+ # error_report("Error msg line #1"
+ # "Error msg line #2\n");
+ my $quoted_newline_regex = qr{\+\s*\".*\\n.*\"};
+ my $continued_str_literal = qr{\+\s*\".*\"};
+
+ if ($rawline =~ /$quoted_newline_regex/) {
+ # Backtrack to first line that does not contain only a quoted literal
+ # and assume that it is the start of the statement.
+ my $i = $linenr - 2;
+
+ while (($i >= 0) & $rawlines[$i] =~ /$continued_str_literal/) {
+ $i--;
+ }
+
+ if ($rawlines[$i] =~ /\b(?:$qemu_error_funcs)\s*\(/) {
+ WARN("Error messages should not contain newlines\n" . $herecurr);
+ }
+ }
+
# check for non-portable ffs() calls that have portable alternatives in QEMU
if ($line =~ /\bffs\(/) {
ERROR("use ctz32() instead of ffs()\n" . $herecurr);
--
2.4.3
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13
2016-01-13 15:42 [Qemu-devel] [PULL 00/41] Error reporting patches for 2016-01-13 Markus Armbruster
` (40 preceding siblings ...)
2016-01-13 15:43 ` [Qemu-devel] [PULL 41/41] checkpatch: Detect newlines in error_report and other error functions Markus Armbruster
@ 2016-01-14 14:09 ` Peter Maydell
41 siblings, 0 replies; 43+ messages in thread
From: Peter Maydell @ 2016-01-14 14:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 13 January 2016 at 15:42, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit 649a1bbaf95adb228f1030ab0618a932bc26aa8b:
>
> Merge remote-tracking branch 'remotes/kvaneesh/tags/for-upstream-signed' into staging (2016-01-12 17:37:22 +0000)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-error-2016-01-13
>
> for you to fetch changes up to 5d596c245d675000ddee69e87616d537ef273be5:
>
> checkpatch: Detect newlines in error_report and other error functions (2016-01-13 15:16:19 +0100)
>
> ----------------------------------------------------------------
> Error reporting patches for 2016-01-13
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 43+ messages in thread