From 8d780f43921feb7fd8d0b58f779a22d1265f2378 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Nov 2015 17:05:49 +0100 Subject: [PATCH 01/41] error: Document how to accumulate multiple errors Suggested-by: Eric Blake Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake 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 6285cf50d1..1480f59ec5 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 From 007b06578ab6063d49b6834d95274c37387a1efb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 11 Sep 2015 15:04:45 +0200 Subject: [PATCH 02/41] Use error_fatal to simplify obvious fatal errors 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" Cc: Eduardo Habkost Cc: Paolo Bonzini Signed-off-by: Markus Armbruster Reviewed-by: Eduardo Habkost --- 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 d934980efa..79b7c5ab3d 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 85ae69efd9..a0a5a061ea 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 421bde9a1c..96dedce906 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 e14828db0d..2d6952c393 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 912c2908f3..70eefe9987 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 058abbde3f..ea9a9840d0 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 1c1a44547f..65e92e1824 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 513d73c27f..566e9ef194 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 921e799dbb..d515e99f98 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 c36b8cf45a..166e8e2112 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 b81a1d349d..a3e575ac79 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 17109461ce..425ef8dc21 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 5aaea77b0a..6c2add9421 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); From 6231a6da9f40c3a33589402c9c57557e3a4f05ad Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2015 17:29:15 +0100 Subject: [PATCH 03/41] hw: Inline the qdev_prop_set_drive_nofail() wrapper Signed-off-by: Markus Armbruster Message-Id: <1449764955-10741-3-git-send-email-armbru@redhat.com> Reviewed-by: Peter Maydell --- 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 2a8835ec01..57170aea8b 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 4292eced32..858f5f7ce7 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 f0e34139fe..478e1a6b3f 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 d515e99f98..1589abaf60 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 788b36133c..1f831098c7 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 3b1fcec537..38030655f4 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 414e0f9b7a..0ca01767e5 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 77538a8ca2..254afd8859 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. */ From c525436e69bb7e74ca96982a40b8ead037186049 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:09 +0100 Subject: [PATCH 04/41] hw: Don't use hw_error() for machine initialization errors 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 Cc: qemu-arm@nongnu.org Cc: qemu-ppc@nongnu.org Cc: Guan Xuetao Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell Reviewed-by: Thomas Huth Message-Id: <1450370121-5768-2-git-send-email-armbru@redhat.com> Reviewed-by: Richard Henderson --- 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 27bdaa1ad6..38b85bafe5 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 a0a5a061ea..cb9926e50f 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 215f9622c9..2736b37e71 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 c1dea17f33..8d9ccaa88e 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 1b9a573cae..ca3d6a8c40 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 21eaf0e66b..c3f9fe3907 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 5ad28f75cf..0e102fcfbe 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 91117b2b94..1968202c82 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; From 84a3a53cf61ef691478bd91afa455c801696053c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:10 +0100 Subject: [PATCH 05/41] omap: Don't use hw_error() in device init() methods 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 Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell 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 3c538985ee..63d8b42674 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 b6f544a221..8b0b14684d 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 e9b38a3c63..07b6272b09 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; From b097e481217ac30eeac675ec16711ef360ccce72 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:11 +0100 Subject: [PATCH 06/41] arm_mptimer: Don't use hw_error() in realize() method Device realize() methods aren't supposed to call hw_error(), they should set an error and fail cleanly. Do that. Cc: Peter Maydell Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell 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 3e59c2a288..5dfab669f4 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" From 5a8de107e3496855147e823de32ff70cada725d1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:12 +0100 Subject: [PATCH 07/41] etraxfs_eth: Don't use hw_error() in init() method Device init() methods aren't supposed to call hw_error(), they should report the error and fail cleanly. Do that. Cc: "Edgar E. Iglesias" Signed-off-by: Markus Armbruster Reviewed-by: Edgar E. Iglesias 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 d6002750f0..b562ac990a 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; From 9280eb34deb032d7c86275a92651ae63cc5418d5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:13 +0100 Subject: [PATCH 08/41] raven: Mark use of hw_error() in realize() FIXME MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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" Cc: qemu-ppc@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth 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 da88cb3352..f434596e8f 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; } From 543202c0ddbcc4ee97d82fe45356e1ab00093f90 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:14 +0100 Subject: [PATCH 09/41] error: Don't append a newline when printing the error hint 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 Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake 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 a35098f711..30936dfba4 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 80c89a2079..9b27c4508e 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 a50eceae4a..a2d593ad2b 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); From 7b55044f9d96ec518e7ab58bd8a3637b52a35f79 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:15 +0100 Subject: [PATCH 10/41] hw/arm/virt: Fix property "gic-version" error handling 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 Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell --- 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 fd52b76882..92dcd02119 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"); } } From c72fbf98cb243df53b92081dd9daae21c7ea0d46 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:16 +0100 Subject: [PATCH 11/41] sysbus: Don't use hw_error() in machine_init_done_notifiers 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 Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth 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 70e0518901..aa55d01817 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; } /* From 3a80ceadcbfa2bfd65ecfa81e389e93b738aaef0 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:17 +0100 Subject: [PATCH 12/41] isa: Trivially convert remaining PCI-ISA bridges to realize() These are "ICH9-LPC" and "ebus". Cc: "Michael S. Tsirkin" Cc: Mark Cave-Ayland Signed-off-by: Markus Armbruster Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin 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 1ffc80362b..8e5844906c 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 7a433d33a7..8058aac5d7 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; From d10e54329bbfe6bfacf75eb33caead39eef9f3c8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:18 +0100 Subject: [PATCH 13/41] isa: Clean up error handling around isa_bus_new() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Cc: "Michael S. Tsirkin" Cc: "Hervé Poussineau" Cc: Aurelien Jarno Cc: Mark Cave-Ayland Signed-off-by: Markus Armbruster Reviewed-by: Marcel Apfelbaum Reviewed-by: Hervé Poussineau Reviewed-by: Michael S. Tsirkin 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 421162e1d4..35dc8a5b47 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 438cdae99e..df2b824385 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 d4c830684b..3793c6fe7a 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 43e0cd8ddd..af6ffd6461 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 8e5844906c..ed9907d29a 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 2c59e91fff..644cfd9536 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 252e1d7145..6c2190b46c 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 1ab885bb3f..1cfbaa605a 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 af10da1c57..2d4e038673 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 924f0fa82b..b0d7e31607 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 8058aac5d7..1925a1cef9 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 d758b39a2e..de3cd3d38a 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); From 675463d9b6b2c2b65a713a6d906aeebe9e6750ae Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 13:19:53 +0100 Subject: [PATCH 14/41] isa: Clean up inappropriate hw_error() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Cc: "Michael S. Tsirkin" Cc: "Hervé Poussineau" Cc: Aurelien Jarno Cc: Mark Cave-Ayland Signed-off-by: Markus Armbruster Reviewed-by: Hervé Poussineau Message-Id: <1450354795-31608-12-git-send-email-armbru@redhat.com> Reviewed-by: Richard Henderson --- 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 af6ffd6461..630054c03e 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); } From 7e274652e4e2225298a2735c3dcc74f2f8ba7b07 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:20 +0100 Subject: [PATCH 15/41] audio: Clean up inappropriate and unreachable use of hw_error() 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 Signed-off-by: Markus Armbruster Reviewed-by: Gerd Hoffmann --- audio/audio.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 5be4b15fcf..a0fc8b31ae 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) { From acef5c02e54e6fb60f8f41a85772be47e5196e81 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:21 +0100 Subject: [PATCH 16/41] xen-hvm: Mark inappropriate error handling FIXME Cc: Stefano Stabellini Cc: xen-devel@lists.xensource.com Signed-off-by: Markus Armbruster 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 3d78a0c529..2a9339062a 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; From 85b01e096088832e68fb862e6277a3f858d69cbe Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:04 +0100 Subject: [PATCH 17/41] qemu-nbd: Replace BSDism by error_report() 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 Reviewed-by: Eric Blake 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 65dc30c189..d5c32deab1 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -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; From 4fffeb5e197e4e5ca01c8ec386ecd712f3319dcf Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:05 +0100 Subject: [PATCH 18/41] error: Use error_report_err() where appropriate (again) 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 Reviewed-by: Eric Blake 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 d80e4ed18d..dd8301bdbd 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 59a4c11277..039f0ebdb8 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 eff6f4681d..f4b463aa1e 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 a3b9e82ff4..3ab83a1acd 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 85b978fa76..73e60876e8 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 8be62c349b..4fb86a68c4 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 d5c32deab1..65c0ebda04 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 6c2add9421..7548fa2490 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)); From 193227f9e565803b1167fa01301bdf9f6d294d6a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:06 +0100 Subject: [PATCH 19/41] error: Use error_report_err() instead of monitor_printf() 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 Reviewed-by: Eric Blake 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 c2b2c161c8..9723397b8b 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 539ef6d3a4..4af1558205 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 0ad1b93a8b..e277b72bbf 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 e7e7ae2c5e..c53a453c7c 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) From 782886719822c956e800dfb9c0665e2b301cb1fb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:07 +0100 Subject: [PATCH 20/41] error: Use error_report_err() instead of ad hoc prints 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 Reviewed-by: Eric Blake 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 54ff001c23..00508b5edd 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 30936dfba4..3ce47109e3 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 65c0ebda04..706552e64c 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; } From f4d0064afcff4c38b379800674938cde8f069dcd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:08 +0100 Subject: [PATCH 21/41] error: Improve documentation While there, tighten error_append_hint()'s assertion. Signed-off-by: Markus Armbruster Message-Id: <1450452927-8346-6-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- 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 1480f59ec5..b18a608c6d 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 9b27c4508e..ebfb74b02e 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 c1574bb348..ecf5708478 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. */ From cd5c2dac2e6e60b4f7048d932530cec9d3fdc5da Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:09 +0100 Subject: [PATCH 22/41] block: Clean up "Could not create temporary overlay" error message 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 Reviewed-by: Eric Blake 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 01655ded13..b2bdff90e4 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; } From a4699e55f596e552c9b45028a0e831c3438f768d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:10 +0100 Subject: [PATCH 23/41] qemu-nbd: Clean up "Failed to load snapshot" error message 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 Reviewed-by: Eric Blake 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 706552e64c..b8be3bc582 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); } } From 73eaa04777001e6d68181910ed36729528f77d74 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:11 +0100 Subject: [PATCH 24/41] test-throttle: Simplify qemu_init_main_loop() error handling 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 Reviewed-by: Eric Blake 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 85c9b6ceed..a95039fdbf 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)); From 8277d2aa58fe4f8f3ee394ea647ea652faf822a4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:12 +0100 Subject: [PATCH 25/41] error: New error_prepend(), error_reportf_err() 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 Reviewed-by: Eric Blake 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 b18a608c6d..45d6c72dee 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 +#include #include "qemu/compiler.h" #include "qapi-types.h" -#include /* * 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 @@ -250,6 +271,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 diff --git a/util/error.c b/util/error.c index ebfb74b02e..57303fd05c 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) { From 8aa802a6b768e811e492c7404a58c52ecfb382ed Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:13 +0100 Subject: [PATCH 26/41] error: Don't decorate original error message when adding to it 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 Reviewed-by: Eric Blake 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 33e245e12f..fffb58efb3 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 3d48b4f00e..f4f5540b87 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 e188d8c13d..f0b447ea42 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 6a0b9813f5..153b8f5870 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); From c29b77f955ff2f7b57c1e71e9dc26243eefd0b28 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:14 +0100 Subject: [PATCH 27/41] error: Use error_reportf_err() where it makes obvious sense 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 Message-Id: <1450452927-8346-12-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- 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 38f5fb9c22..d1383b3c43 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 dd8301bdbd..6986be8151 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 2df0c6d366..1392fffaaa 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 bf068cd3cc..a71e43cf5f 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 710045adca..dfaed257f5 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 fffb58efb3..3572810b42 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 4e3173d81a..2c7101d91d 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 166e8e2112..0e5c86ae1b 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 b3418dbae4..bd7da47fe2 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 ee6b43abc6..26ab67fc2a 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 f4f5540b87..a5949e6b05 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 b8be3bc582..f9fce4a57a 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 0d33e82c95..e4673b3d92 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 f0b447ea42..6ccea98977 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 153b8f5870..ccdee39993 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 09756cd7cc..54673eb8c7 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 7548fa2490..20a367b123 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) { From e43bfd9c876676e884b561eca009a571992a4b76 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:15 +0100 Subject: [PATCH 28/41] error: Use error_prepend() where it makes obvious sense 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 Reviewed-by: Eric Blake --- 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 b2bdff90e4..54c37f9629 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 1789af43d2..d992e7fac7 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 9b88895038..31f4cc9e60 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 a2529b2242..b8ce6cd5f3 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 00cdac62f9..7bc82885ac 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 26ab67fc2a..1bbe9302f9 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; } From b83baa6025d0bf921f2fc4514df4f60493ea41de Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:16 +0100 Subject: [PATCH 29/41] spapr: Use error_reportf_err() Not caught by Coccinelle, because we report the error only conditionally here. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake 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 0ca01767e5..091cdb1e1f 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) { From d410fe145446968055f3807b0d41ae8150eb0926 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:17 +0100 Subject: [PATCH 30/41] migration: Use error_reportf_err() instead of monitor_printf() 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 Reviewed-by: Eric Blake 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 e277b72bbf..bcaeb70fe0 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)); } } From b9884681491fbe8b3fa32d58f35bcc5f725c5258 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:18 +0100 Subject: [PATCH 31/41] qemu-io qemu-nbd: Use error_report() etc. instead of fprintf() Just three instances left. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake 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 269f17cc2c..d47228a963 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 f9fce4a57a..99df01fd20 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 00057fef9b..d28df5bb62 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 751118951f..5d40206ef8 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 c78e8c2b72..f97585677b 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 ca743831c9..ffd425157f 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 5f1d6c120a..87beae4e3c 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 b0000aeedd..72645b2522 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 ca18d2ea38..42b8a83015 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 6061d84432..0daac48b12 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 8c1441bf4f..78cc49ad91 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 6e6bfcaf90..a2a83b8a1c 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 c5c60f9178..e18f54c200 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 d05f49fdba..b7aaadf89a 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 6a2c750567..b6d10e4804 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 b679ceea63..1f11d4446d 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 021a04c812..ae2412ebf7 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 From 9af9e0fed76e91ed42cb6d31ab6c665f7d691c1c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:19 +0100 Subject: [PATCH 32/41] error: Strip trailing '\n' from error string arguments (again) 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 Cc: Peter Crosthwaite Cc: Bharata B Rao Cc: Dominik Dingel Cc: David Hildenbrand Cc: Jason J. Herne Cc: Stefan Berger Cc: Dr. David Alan Gilbert Cc: Changchun Ouyang Cc: zhanghailiang Cc: Pavel Fedin Signed-off-by: Markus Armbruster Reviewed-by: Dr. David Alan Gilbert Acked-by: Cornelia Huck Acked-by: Bharata B Rao Acked-by: Fam Zheng Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake 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 6f819e413f..b4a224ed76 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 87553bbc60..20a3b2b093 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 091cdb1e1f..50e5a268da 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 b91fcc6e79..e100428f20 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 682949afb2..eaa37ba408 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 4af1558205..f2b732e300 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 ff073d501a..95fc66e8fc 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 0490f005dd..5bfcca3884 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 bcaeb70fe0..540eb288af 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 b368a90219..e4dd089c5c 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 99df01fd20..023eacd1c8 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 8fe708f001..5f91275484 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 35a1f12661..775e0a46bd 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 36a0d159cc..b1e1418a6e 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); From e4937694b66d1468aec3cd95e90888f291c3f599 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:20 +0100 Subject: [PATCH 33/41] vmdk: Clean up control flow in vmdk_parse_extents() a bit Factor out loop stepping to turn a while-loop with goto into a for-loop with continue. Cc: Fam Zheng Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Fam Zheng 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 b4a224ed76..08fa3f3b12 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; } From d28d737fb9b1f6c2a58821f8a4640adecf99249b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:21 +0100 Subject: [PATCH 34/41] vmdk: Clean up "Invalid extent lines" error message 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: Error messages should not have newlines embedded. Since the remaining text is not helpful, we can simply report: Invalid extent line: Cc: Fam Zheng Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1450452927-8346-19-git-send-email-armbru@redhat.com> Reviewed-by: Fam Zheng --- 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 08fa3f3b12..2b5cb00ef1 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 d28df5bb62..9d506cb80c 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 From c3d2d68ad68d044417bbecaa0ebb0aa0dfb44b5f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:22 +0100 Subject: [PATCH 35/41] pci-assign: Clean up "Failed to assign" error messages 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 Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1450452927-8346-20-git-send-email-armbru@redhat.com> Reviewed-by: Laszlo Ersek --- 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 0fd69230ae..eec1340f47 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; } From bf89e87427fb99b994eb0dfb710bb4b45785f733 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:23 +0100 Subject: [PATCH 36/41] vhdx: Fix "log that needs to be replayed" error message 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 Reviewed-by: Eric Blake 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 47ae4b1351..ab86416def 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 ffd425157f..131a5b17dc 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 From 433672b0d5cfdd4acbf269e6aef079e162af5bad Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:24 +0100 Subject: [PATCH 37/41] error: Clean up errors with embedded newlines (again) 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 Cc: Pavel Fedin Signed-off-by: Markus Armbruster Reviewed-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Markus Armbruster --- 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 0e5c86ae1b..9e37186776 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 bd9e7641b2..9148889921 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 023eacd1c8..a4cf847976 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); } From e6da780d5f8e258f4a97631612d2b841faed4885 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:25 +0100 Subject: [PATCH 38/41] hw/s390x: Rename local variables Error *l_err to just err Let's follow established naming practice here as well. Cc: David Hildenbrand Signed-off-by: Markus Armbruster Acked-by: Cornelia Huck Reviewed-by: David Hildenbrand Reviewed-by: Eric Blake 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 e100428f20..9c01be5ca9 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 a061b49f19..9a117c90a7 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) From 24da21f265b091dee756398a24d63dd8a2f3d5ab Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:26 +0100 Subject: [PATCH 39/41] s390/sclp: Simplify control flow in sclp_realize() Suggested-by: David Hildenbrand Signed-off-by: Markus Armbruster Reviewed-by: David Hildenbrand Acked-by: Cornelia Huck Reviewed-by: Eric Blake 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 9a117c90a7..74f2b40154 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); } From 533fdaedeb7f7ae90866312f57249ebbb7e3c97f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:27 +0100 Subject: [PATCH 40/41] error: Consistently name Error * objects err, and not errp Signed-off-by: Markus Armbruster Message-Id: <1450452927-8346-25-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- 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 00508b5edd..9b0d6e2319 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 9723397b8b..54f2620690 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 de1d1f8cb1..4057cdd6a2 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 74c692d432..2e7f98518e 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 95852523b4..7aecdfcefb 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); From 5d596c245d675000ddee69e87616d537ef273be5 Mon Sep 17 00:00:00 2001 From: "Jason J. Herne" Date: Fri, 11 Dec 2015 13:30:42 -0500 Subject: [PATCH 41/41] checkpatch: Detect newlines in error_report and other error functions 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 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 --- scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index efca817b9b..257126f91b 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);