All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init()
@ 2015-02-04 17:33 Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 1/8] qdev: Improve qdev_init_nofail()'s error reporting Markus Armbruster
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: afaerber

Board setup should normally use qdev_init_nofail(), not qdev_init(),
because inability to create an onboard device is normally fatal.
Board setup should certainly not ignore qdev_init() failure.  Even
when we know that realization can't actually fail now,
qdev_init_nofail() is clearer and more robust.  Correct a few obvious
instances.

Markus Armbruster (8):
  qdev: Improve qdev_init_nofail()'s error reporting
  ide/isa: Replace unchecked qdev_init() by qdev_init_nofail()
  leon3: Replace unchecked qdev_init() by qdev_init_nofail()
  etsec: Replace qdev_init() by qdev_init_nofail()
  serial: Factor out common serial_hds_isa_init()
  serial: serial_hds_isa_init() shouldn't fail
  parallel: Factor out common parallel_hds_isa_init()
  parallel: parallel_hds_isa_init() shouldn't fail

 hw/alpha/dp264.c         |  6 +-----
 hw/char/parallel.c       | 25 +++++++++++++++++++++++++
 hw/char/serial-isa.c     | 23 +++++++++++++++--------
 hw/core/qdev.c           | 11 ++++++++---
 hw/i386/pc.c             | 13 ++-----------
 hw/ide/isa.c             |  4 +---
 hw/mips/mips_fulong2e.c  | 12 ++----------
 hw/mips/mips_malta.c     |  7 +++----
 hw/mips/mips_r4k.c       |  6 +-----
 hw/net/fsl_etsec/etsec.c |  5 +----
 hw/sparc64/sun4u.c       | 12 ++----------
 include/hw/char/serial.h |  2 +-
 include/hw/i386/pc.h     | 17 +----------------
 include/hw/sparc/grlib.h | 12 +++---------
 14 files changed, 66 insertions(+), 89 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 1/8] qdev: Improve qdev_init_nofail()'s error reporting
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 2/8] ide/isa: Replace unchecked qdev_init() by qdev_init_nofail() Markus Armbruster
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: afaerber

We get two error messages: a specific one from qdev_init(), and a
generic one from qdev_init_nofail().  The specific one gets suppressed
in QMP context.  qdev_init_nofail() failing there is a bug, though.

Cut out the qdev_init() middle-man: realize the device, and on error
exit with a single error message.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/qdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2eacac0..416c7a4 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -373,10 +373,15 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
    way is somewhat unclean, and best avoided.  */
 void qdev_init_nofail(DeviceState *dev)
 {
-    const char *typename = object_get_typename(OBJECT(dev));
+    Error *err = NULL;
 
-    if (qdev_init(dev) < 0) {
-        error_report("Initialization of device %s failed", typename);
+    assert(!dev->realized);
+
+    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));
         exit(1);
     }
 }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 2/8] ide/isa: Replace unchecked qdev_init() by qdev_init_nofail()
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 1/8] qdev: Improve qdev_init_nofail()'s error reporting Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 3/8] leon3: " Markus Armbruster
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, afaerber, Stefan Hajnoczi

isa_ide_init()'s callers don't check for failure.  isa_ide_init()
looks like it could fail, but since isa_ide_realizefn() can't fail, it
actually can't.  Replace its qdev_init() by qdev_init_nofail() to make
it obvious.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/isa.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index b084162..c0c4e1b 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -88,9 +88,7 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
     qdev_prop_set_uint32(dev, "iobase",  iobase);
     qdev_prop_set_uint32(dev, "iobase2", iobase2);
     qdev_prop_set_uint32(dev, "irq",     isairq);
-    if (qdev_init(dev) < 0) {
-        return NULL;
-    }
+    qdev_init_nofail(dev);
 
     s = ISA_IDE(dev);
     if (hd0) {
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 3/8] leon3: Replace unchecked qdev_init() by qdev_init_nofail()
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 1/8] qdev: Improve qdev_init_nofail()'s error reporting Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 2/8] ide/isa: Replace unchecked qdev_init() by qdev_init_nofail() Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-12  9:30   ` Fabien Chouteau
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 4/8] etsec: Replace " Markus Armbruster
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: afaerber, Fabien Chouteau

grlib_irqmp_create(), grlib_gptimer_create() and
grlib_apbuart_create() are helpers to create and realize GRLIB
devices.  Their only caller leon3_generic_hw_init() doesn't check for
failure.  Only the first can actually fail, and only when the caller
fails to set up a pointer property, which is a programming error.

Replace qdev_init() by qdev_init_nofail().

Cc: Fabien Chouteau <chouteau@adacore.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/sparc/grlib.h | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/include/hw/sparc/grlib.h b/include/hw/sparc/grlib.h
index 470ce72..9a0db7b 100644
--- a/include/hw/sparc/grlib.h
+++ b/include/hw/sparc/grlib.h
@@ -55,9 +55,7 @@ DeviceState *grlib_irqmp_create(hwaddr   base,
     qdev_prop_set_ptr(dev, "set_pil_in", set_pil_in);
     qdev_prop_set_ptr(dev, "set_pil_in_opaque", env);
 
-    if (qdev_init(dev)) {
-        return NULL;
-    }
+    qdev_init_nofail(dev);
 
     env->irq_manager = dev;
 
@@ -87,9 +85,7 @@ DeviceState *grlib_gptimer_create(hwaddr  base,
     qdev_prop_set_uint32(dev, "frequency", freq);
     qdev_prop_set_uint32(dev, "irq-line", base_irq);
 
-    if (qdev_init(dev)) {
-        return NULL;
-    }
+    qdev_init_nofail(dev);
 
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 
@@ -112,9 +108,7 @@ DeviceState *grlib_apbuart_create(hwaddr  base,
     dev = qdev_create(NULL, "grlib,apbuart");
     qdev_prop_set_chr(dev, "chrdev", serial);
 
-    if (qdev_init(dev)) {
-        return NULL;
-    }
+    qdev_init_nofail(dev);
 
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 4/8] etsec: Replace qdev_init() by qdev_init_nofail()
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
                   ` (2 preceding siblings ...)
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 3/8] leon3: " Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 5/8] serial: Factor out common serial_hds_isa_init() Markus Armbruster
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: afaerber

etsec_create() is a helper to create and realize the eTSEC.  It's
currently unused.  Similar helpers for other NICs use
qdev_init_nofail().  Match that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/fsl_etsec/etsec.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index 2fbbc6c..c57365f 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -443,10 +443,7 @@ DeviceState *etsec_create(hwaddr         base,
 
     dev = qdev_create(NULL, "eTSEC");
     qdev_set_nic_properties(dev, nd);
-
-    if (qdev_init(dev)) {
-        return NULL;
-    }
+    qdev_init_nofail(dev);
 
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, tx_irq);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 1, rx_irq);
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 5/8] serial: Factor out common serial_hds_isa_init()
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
                   ` (3 preceding siblings ...)
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 4/8] etsec: Replace " Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 6/8] serial: serial_hds_isa_init() shouldn't fail Markus Armbruster
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Blue Swirl, Anthony Liguori, Leon Alrae,
	afaerber, Aurelien Jarno, Richard Henderson

It's the same old loop copied five times, plus another instance where
it's clipped to two iterations and unrolled.

No external users of serial_isa_init() are left, so give it internal
linkage.

Maintainers of affected machines cc'ed.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/alpha/dp264.c         |  6 +-----
 hw/char/serial-isa.c     | 15 ++++++++++++++-
 hw/i386/pc.c             |  6 +-----
 hw/mips/mips_fulong2e.c  |  7 +------
 hw/mips/mips_malta.c     |  3 +--
 hw/mips/mips_r4k.c       |  6 +-----
 hw/sparc64/sun4u.c       |  7 ++-----
 include/hw/char/serial.h |  2 +-
 8 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 84a55e4..e82d61d 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -83,11 +83,7 @@ static void clipper_init(MachineState *machine)
     pci_vga_init(pci_bus);
 
     /* Serial code setup.  */
-    for (i = 0; i < MAX_SERIAL_PORTS; ++i) {
-        if (serial_hds[i]) {
-            serial_isa_init(isa_bus, i, serial_hds[i]);
-        }
-    }
+    serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
 
     /* Network setup.  e1000 is good enough, failing Tulip support.  */
     for (i = 0; i < nb_nics; i++) {
diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index c9fcb27..059ceb8 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -119,7 +119,7 @@ static void serial_register_types(void)
 
 type_init(serial_register_types)
 
-bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
+static bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
 {
     DeviceState *dev;
     ISADevice *isadev;
@@ -136,3 +136,16 @@ bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
     }
     return true;
 }
+
+void serial_hds_isa_init(ISABus *bus, int n)
+{
+    int i;
+
+    assert(n <= MAX_SERIAL_PORTS);
+
+    for (i = 0; i < n; ++i) {
+        if (serial_hds[i]) {
+            serial_isa_init(bus, i, serial_hds[i]);
+        }
+    }
+}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c7af6aa..18ed263 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1419,11 +1419,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
         pcspk_init(isa_bus, pit);
     }
 
-    for(i = 0; i < MAX_SERIAL_PORTS; i++) {
-        if (serial_hds[i]) {
-            serial_isa_init(isa_bus, i, serial_hds[i]);
-        }
-    }
+    serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
 
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         if (parallel_hds[i]) {
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 6a9ebfa..bebf7de 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -277,7 +277,6 @@ static void mips_fulong2e_init(MachineState *machine)
     PCIBus *pci_bus;
     ISABus *isa_bus;
     I2CBus *smbus;
-    int i;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     MIPSCPU *cpu;
     CPUMIPSState *env;
@@ -384,11 +383,7 @@ static void mips_fulong2e_init(MachineState *machine)
 
     rtc_init(isa_bus, 2000, NULL);
 
-    for(i = 0; i < MAX_SERIAL_PORTS; i++) {
-        if (serial_hds[i]) {
-            serial_isa_init(isa_bus, i, serial_hds[i]);
-        }
-    }
+    serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
 
     if (parallel_hds[0]) {
         parallel_init(isa_bus, 0, parallel_hds[0]);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 5845158..e57f78c 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1172,8 +1172,7 @@ void mips_malta_init(MachineState *machine)
     isa_create_simple(isa_bus, "i8042");
 
     rtc_init(isa_bus, 2000, NULL);
-    serial_isa_init(isa_bus, 0, serial_hds[0]);
-    serial_isa_init(isa_bus, 1, serial_hds[1]);
+    serial_hds_isa_init(isa_bus, 2);
     if (parallel_hds[0])
         parallel_init(isa_bus, 0, parallel_hds[0]);
     for(i = 0; i < MAX_FD; i++) {
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index a7fe0ce..1698f2d 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -283,11 +283,7 @@ void mips_r4k_init(MachineState *machine)
 
     pit = pit_init(isa_bus, 0x40, 0, NULL);
 
-    for(i = 0; i < MAX_SERIAL_PORTS; i++) {
-        if (serial_hds[i]) {
-            serial_isa_init(isa_bus, i, serial_hds[i]);
-        }
-    }
+    serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
 
     isa_vga_init(isa_bus);
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 3ff5bd8..082b8e0 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -849,11 +849,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                        NULL, 115200, serial_hds[i], DEVICE_BIG_ENDIAN);
         i++;
     }
-    for(; i < MAX_SERIAL_PORTS; i++) {
-        if (serial_hds[i]) {
-            serial_isa_init(isa_bus, i, serial_hds[i]);
-        }
-    }
+
+    serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
 
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         if (parallel_hds[i]) {
diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index f431764..15beb6b 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -92,6 +92,6 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
 
 /* serial-isa.c */
 #define TYPE_ISA_SERIAL "isa-serial"
-bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr);
+void serial_hds_isa_init(ISABus *bus, int n);
 
 #endif
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 6/8] serial: serial_hds_isa_init() shouldn't fail
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
                   ` (4 preceding siblings ...)
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 5/8] serial: Factor out common serial_hds_isa_init() Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 7/8] parallel: Factor out common parallel_hds_isa_init() Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 8/8] parallel: parallel_hds_isa_init() shouldn't fail Markus Armbruster
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Blue Swirl, Anthony Liguori, Leon Alrae,
	afaerber, Aurelien Jarno, Richard Henderson

It shouldn't fail, and no caller checks for failure.  Make failure
fatal.

Maintainers of affected machines cc'ed.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/char/serial-isa.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index 059ceb8..f3db024 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -119,22 +119,16 @@ static void serial_register_types(void)
 
 type_init(serial_register_types)
 
-static bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
+static void serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
 {
     DeviceState *dev;
     ISADevice *isadev;
 
-    isadev = isa_try_create(bus, TYPE_ISA_SERIAL);
-    if (!isadev) {
-        return false;
-    }
+    isadev = isa_create(bus, TYPE_ISA_SERIAL);
     dev = DEVICE(isadev);
     qdev_prop_set_uint32(dev, "index", index);
     qdev_prop_set_chr(dev, "chardev", chr);
-    if (qdev_init(dev) < 0) {
-        return false;
-    }
-    return true;
+    qdev_init_nofail(dev);
 }
 
 void serial_hds_isa_init(ISABus *bus, int n)
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 7/8] parallel: Factor out common parallel_hds_isa_init()
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
                   ` (5 preceding siblings ...)
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 6/8] serial: serial_hds_isa_init() shouldn't fail Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 8/8] parallel: parallel_hds_isa_init() shouldn't fail Markus Armbruster
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Blue Swirl, Anthony Liguori, Leon Alrae,
	afaerber, Aurelien Jarno

Maintainers of affected machines cc'ed.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/char/parallel.c      | 31 +++++++++++++++++++++++++++++++
 hw/i386/pc.c            |  7 +------
 hw/mips/mips_fulong2e.c |  5 +----
 hw/mips/mips_malta.c    |  4 ++--
 hw/sparc64/sun4u.c      |  7 +------
 include/hw/i386/pc.h    | 17 +----------------
 6 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index c2b553f..710cefc 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -641,3 +641,34 @@ static void parallel_register_types(void)
 }
 
 type_init(parallel_register_types)
+
+static bool parallel_init(ISABus *bus, int index, CharDriverState *chr)
+{
+    DeviceState *dev;
+    ISADevice *isadev;
+
+    isadev = isa_try_create(bus, "isa-parallel");
+    if (!isadev) {
+        return false;
+    }
+    dev = DEVICE(isadev);
+    qdev_prop_set_uint32(dev, "index", index);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    if (qdev_init(dev) < 0) {
+        return false;
+    }
+    return true;
+}
+
+void parallel_hds_isa_init(ISABus *bus, int n)
+{
+    int i;
+
+    assert(n <= MAX_PARALLEL_PORTS);
+
+    for (i = 0; i < n; i++) {
+        if (parallel_hds[i]) {
+            parallel_init(bus, i, parallel_hds[i]);
+        }
+    }
+}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 18ed263..ed623e1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1420,12 +1420,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     }
 
     serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
-
-    for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
-        if (parallel_hds[i]) {
-            parallel_init(isa_bus, i, parallel_hds[i]);
-        }
-    }
+    parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
 
     a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
     i8042 = isa_create_simple(isa_bus, "i8042");
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index bebf7de..ea73585 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -384,10 +384,7 @@ static void mips_fulong2e_init(MachineState *machine)
     rtc_init(isa_bus, 2000, NULL);
 
     serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
-
-    if (parallel_hds[0]) {
-        parallel_init(isa_bus, 0, parallel_hds[0]);
-    }
+    parallel_hds_isa_init(isa_bus, 1);
 
     /* Sound card */
     audio_init(pci_bus);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index e57f78c..533b2e6 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1173,8 +1173,8 @@ void mips_malta_init(MachineState *machine)
 
     rtc_init(isa_bus, 2000, NULL);
     serial_hds_isa_init(isa_bus, 2);
-    if (parallel_hds[0])
-        parallel_init(isa_bus, 0, parallel_hds[0]);
+    parallel_hds_isa_init(isa_bus, 1);
+
     for(i = 0; i < MAX_FD; i++) {
         fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 082b8e0..17cf61f 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -851,12 +851,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     }
 
     serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS);
-
-    for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
-        if (parallel_hds[i]) {
-            parallel_init(isa_bus, i, parallel_hds[i]);
-        }
-    }
+    parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
 
     for(i = 0; i < nb_nics; i++)
         pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 69d9cf8..8ba84d3 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -107,23 +107,8 @@ struct PcGuestInfo {
 };
 
 /* parallel.c */
-static inline bool parallel_init(ISABus *bus, int index, CharDriverState *chr)
-{
-    DeviceState *dev;
-    ISADevice *isadev;
 
-    isadev = isa_try_create(bus, "isa-parallel");
-    if (!isadev) {
-        return false;
-    }
-    dev = DEVICE(isadev);
-    qdev_prop_set_uint32(dev, "index", index);
-    qdev_prop_set_chr(dev, "chardev", chr);
-    if (qdev_init(dev) < 0) {
-        return false;
-    }
-    return true;
-}
+void parallel_hds_isa_init(ISABus *bus, int n);
 
 bool parallel_mm_init(MemoryRegion *address_space,
                       hwaddr base, int it_shift, qemu_irq irq,
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 8/8] parallel: parallel_hds_isa_init() shouldn't fail
  2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
                   ` (6 preceding siblings ...)
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 7/8] parallel: Factor out common parallel_hds_isa_init() Markus Armbruster
@ 2015-02-04 17:33 ` Markus Armbruster
  7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-02-04 17:33 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Blue Swirl, Anthony Liguori, Leon Alrae,
	afaerber, Aurelien Jarno

It shouldn't fail, and no caller checks for failure.  Make failure
fatal.

Maintainers of affected machines cc'ed.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/char/parallel.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 710cefc..4079554 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -642,22 +642,16 @@ static void parallel_register_types(void)
 
 type_init(parallel_register_types)
 
-static bool parallel_init(ISABus *bus, int index, CharDriverState *chr)
+static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
 {
     DeviceState *dev;
     ISADevice *isadev;
 
-    isadev = isa_try_create(bus, "isa-parallel");
-    if (!isadev) {
-        return false;
-    }
+    isadev = isa_create(bus, "isa-parallel");
     dev = DEVICE(isadev);
     qdev_prop_set_uint32(dev, "index", index);
     qdev_prop_set_chr(dev, "chardev", chr);
-    if (qdev_init(dev) < 0) {
-        return false;
-    }
-    return true;
+    qdev_init_nofail(dev);
 }
 
 void parallel_hds_isa_init(ISABus *bus, int n)
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 3/8] leon3: Replace unchecked qdev_init() by qdev_init_nofail()
  2015-02-04 17:33 ` [Qemu-devel] [PATCH 3/8] leon3: " Markus Armbruster
@ 2015-02-12  9:30   ` Fabien Chouteau
  0 siblings, 0 replies; 10+ messages in thread
From: Fabien Chouteau @ 2015-02-12  9:30 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: afaerber

On 02/04/2015 06:33 PM, Markus Armbruster wrote:
> grlib_irqmp_create(), grlib_gptimer_create() and
> grlib_apbuart_create() are helpers to create and realize GRLIB
> devices.  Their only caller leon3_generic_hw_init() doesn't check for
> failure.  Only the first can actually fail, and only when the caller
> fails to set up a pointer property, which is a programming error.
> 
> Replace qdev_init() by qdev_init_nofail().
> 
> Cc: Fabien Chouteau <chouteau@adacore.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Fabien Chouteau <chouteau@adacore.com>

Thanks Markus,

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-02-12  9:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-04 17:33 [Qemu-devel] [PATCH 0/8] Cleanups around unchecked qdev_init() Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 1/8] qdev: Improve qdev_init_nofail()'s error reporting Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 2/8] ide/isa: Replace unchecked qdev_init() by qdev_init_nofail() Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 3/8] leon3: " Markus Armbruster
2015-02-12  9:30   ` Fabien Chouteau
2015-02-04 17:33 ` [Qemu-devel] [PATCH 4/8] etsec: Replace " Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 5/8] serial: Factor out common serial_hds_isa_init() Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 6/8] serial: serial_hds_isa_init() shouldn't fail Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 7/8] parallel: Factor out common parallel_hds_isa_init() Markus Armbruster
2015-02-04 17:33 ` [Qemu-devel] [PATCH 8/8] parallel: parallel_hds_isa_init() shouldn't fail Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.