All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
@ 2014-10-28  7:35 Markus Armbruster
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 01/10] pci: Convert core " Markus Armbruster
                   ` (12 more replies)
  0 siblings, 13 replies; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
to realize", Paolo called the PCI conversion job "Gargantuan".  This
series attempts to crack it into manageable jobs.

The basic idea comes from qdev core: have the core deal with just
realize, but default the device models' realize() method to one that
calls the old init() method.  Unconverted device models don't set
their realize(), thus get one that calls their init().  We can then
convert device by device instead of having to convert of all of PCI in
one Gargantuan go.

Since PCI's exit() cannot fail, I chose not to add an unrealize().
Precedence: USBDeviceClass method handle_destroy(), called on USB
unrealize.

Aside: USBDeviceClass also has an unrealize() method, but it's never
set and never called.

PATCH 01 converts the interface between PCI core and qdev to realize.

PATCH 02 adds realize to the interface between PCI core and PCI device
models.  Once all device models are converted to realize, the old init
interface can be dropped, completing the Gargantuan job.

PATCH 03-04 convert device models that cannot fail initialization.

PATCH 05-10 convert a few that can fail, but are really easy to
convert.

This series is RFC because it's based on Marcel's "[PATCH v2] hw/pci:
fixed crash when using rombar=0 with romfile=path for hotplugged
devices", which is still undergoing review.

Markus Armbruster (10):
  pci: Convert core to realize
  pci: Permit incremental conversion of device models to realize
  pci: Trivial device model conversions to realize
  pcnet: pcnet_common_init() always returns 0, change to void
  pcnet: Convert to realize
  serial-pci: Convert to realize
  ide/ich: Convert to realize
  cirrus-vga: Convert to realize
  qxl: Convert to realize
  pci-assign: Convert to realize

 hw/acpi/piix4.c            |   5 +-
 hw/audio/ac97.c            |   5 +-
 hw/audio/es1370.c          |   5 +-
 hw/audio/intel-hda.c       |   6 +--
 hw/char/serial-pci.c       |  22 ++++-----
 hw/display/cirrus_vga.c    |  11 ++---
 hw/display/qxl.c           |  36 +++++++--------
 hw/display/vga-pci.c       |  11 ++---
 hw/display/vmware_vga.c    |   6 +--
 hw/i2c/smbus_ich9.c        |   5 +-
 hw/i386/kvm/pci-assign.c   |  10 ++--
 hw/ide/cmd646.c            |   5 +-
 hw/ide/ich.c               |  13 +++---
 hw/ide/piix.c              |  10 ++--
 hw/ide/via.c               |   6 +--
 hw/ipack/tpci200.c         |   6 +--
 hw/isa/i82378.c            |   6 +--
 hw/isa/piix4.c             |   5 +-
 hw/isa/vt82c686.c          |  24 ++++------
 hw/misc/pci-testdev.c      |   6 +--
 hw/net/e1000.c             |   6 +--
 hw/net/eepro100.c          |   6 +--
 hw/net/lance.c             |   3 +-
 hw/net/ne2000.c            |   6 +--
 hw/net/pcnet-pci.c         |   6 +--
 hw/net/pcnet.c             |   4 +-
 hw/net/pcnet.h             |   2 +-
 hw/net/rtl8139.c           |   6 +--
 hw/net/vmxnet3.c           |   6 +--
 hw/pci-bridge/dec.c        |   5 +-
 hw/pci-host/apb.c          |   5 +-
 hw/pci-host/bonito.c       |   6 +--
 hw/pci-host/grackle.c      |   5 +-
 hw/pci-host/piix.c         |  12 ++---
 hw/pci-host/ppce500.c      |   6 +--
 hw/pci-host/prep.c         |   6 +--
 hw/pci-host/q35.c          |   5 +-
 hw/pci-host/uninorth.c     |  20 ++++----
 hw/pci-host/versatile.c    |   5 +-
 hw/pci/pci.c               | 111 ++++++++++++++++++++++++++-------------------
 hw/usb/hcd-ehci-pci.c      |   6 +--
 hw/usb/hcd-xhci.c          |   6 +--
 hw/watchdog/wdt_i6300esb.c |   6 +--
 include/hw/pci/pci.h       |   3 +-
 44 files changed, 199 insertions(+), 256 deletions(-)

-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:32   ` Gonglei
  2014-10-30 17:02   ` Marcel Apfelbaum
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 02/10] pci: Permit incremental conversion of device models " Markus Armbruster
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Implement DeviceClass methods realize() and unrealize() instead of
init() and exit().  The core's initialization errors now get
propagated properly, and QMP sends them instead of an unspecific
"Device initialization failed" error.  Unrealize can't fail, so no
change there.

PCIDeviceClass is unchanged: it still provides init() and exit().
Therefore, device models' errors are still not propagated.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pci.c | 91 +++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index cd7a403..aef95c3 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -114,7 +114,7 @@ static const TypeInfo pcie_bus_info = {
 static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
 static void pci_update_mappings(PCIDevice *d);
 static void pci_irq_handler(void *opaque, int irq_num, int level);
-static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
+static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
 static void pci_del_option_rom(PCIDevice *pdev);
 
 static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
@@ -725,7 +725,7 @@ static void pci_init_mask_bridge(PCIDevice *d)
                                PCI_PREF_RANGE_TYPE_MASK);
 }
 
-static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
+static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev, Error **errp)
 {
     uint8_t slot = PCI_SLOT(dev->devfn);
     uint8_t func;
@@ -751,26 +751,25 @@ static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
         PCIDevice *f0 = bus->devices[PCI_DEVFN(slot, 0)];
         if (f0 && !(f0->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)) {
             /* function 0 should set multifunction bit */
-            error_report("PCI: single function device can't be populated "
-                         "in function %x.%x", slot, PCI_FUNC(dev->devfn));
-            return -1;
+            error_setg(errp, "PCI: single function device can't be populated "
+                       "in function %x.%x", slot, PCI_FUNC(dev->devfn));
+            return;
         }
-        return 0;
+        return;
     }
 
     if (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
-        return 0;
+        return;
     }
     /* function 0 indicates single function, so function > 0 must be NULL */
     for (func = 1; func < PCI_FUNC_MAX; ++func) {
         if (bus->devices[PCI_DEVFN(slot, func)]) {
-            error_report("PCI: %x.0 indicates single function, "
-                         "but %x.%x is already populated.",
-                         slot, slot, func);
-            return -1;
+            error_setg(errp, "PCI: %x.0 indicates single function, "
+                       "but %x.%x is already populated.",
+                       slot, slot, func);
+            return;
         }
     }
-    return 0;
 }
 
 static void pci_config_alloc(PCIDevice *pci_dev)
@@ -803,11 +802,13 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
 
 /* -1 for devfn means auto assign */
 static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
-                                         const char *name, int devfn)
+                                         const char *name, int devfn,
+                                         Error **errp)
 {
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
     PCIConfigReadFunc *config_read = pc->config_read;
     PCIConfigWriteFunc *config_write = pc->config_write;
+    Error *local_err = NULL;
     AddressSpace *dma_as;
 
     if (devfn < 0) {
@@ -816,12 +817,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
             if (!bus->devices[devfn])
                 goto found;
         }
-        error_report("PCI: no slot/function available for %s, all in use", name);
+        error_setg(errp, "PCI: no slot/function available for %s, all in use",
+                   name);
         return NULL;
     found: ;
     } else if (bus->devices[devfn]) {
-        error_report("PCI: slot %d function %d not available for %s, in use by %s",
-                     PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
+        error_setg(errp, "PCI: slot %d function %d not available for %s,"
+                   " in use by %s",
+                   PCI_SLOT(devfn), PCI_FUNC(devfn), name,
+                   bus->devices[devfn]->name);
         return NULL;
     }
 
@@ -865,7 +869,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     if (pc->is_bridge) {
         pci_init_mask_bridge(pci_dev);
     }
-    if (pci_init_multifunction(bus, pci_dev)) {
+    pci_init_multifunction(bus, pci_dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         do_pci_unregister_device(pci_dev);
         return NULL;
     }
@@ -896,7 +902,7 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
     pci_unregister_vga(pci_dev);
 }
 
-static int pci_unregister_device(DeviceState *dev)
+static void pci_qdev_unrealize(DeviceState *dev, Error **errp)
 {
     PCIDevice *pci_dev = PCI_DEVICE(dev);
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
@@ -909,7 +915,6 @@ static int pci_unregister_device(DeviceState *dev)
     }
 
     do_pci_unregister_device(pci_dev);
-    return 0;
 }
 
 void pci_register_bar(PCIDevice *pci_dev, int region_num,
@@ -1742,10 +1747,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
     return bus->devices[devfn];
 }
 
-static int pci_qdev_init(DeviceState *qdev)
+static void pci_qdev_realize(DeviceState *qdev, Error **errp)
 {
     PCIDevice *pci_dev = (PCIDevice *)qdev;
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
+    Error *local_err = NULL;
     PCIBus *bus;
     int rc;
     bool is_default_rom;
@@ -1758,15 +1764,16 @@ static int pci_qdev_init(DeviceState *qdev)
     bus = PCI_BUS(qdev_get_parent_bus(qdev));
     pci_dev = do_pci_register_device(pci_dev, bus,
                                      object_get_typename(OBJECT(qdev)),
-                                     pci_dev->devfn);
+                                     pci_dev->devfn, errp);
     if (pci_dev == NULL)
-        return -1;
+        return;
 
     if (pc->init) {
         rc = pc->init(pci_dev);
         if (rc != 0) {
             do_pci_unregister_device(pci_dev);
-            return rc;
+            error_setg(errp, "Device initialization failed");
+            return;
         }
     }
 
@@ -1777,13 +1784,12 @@ static int pci_qdev_init(DeviceState *qdev)
         is_default_rom = true;
     }
 
-    rc = pci_add_option_rom(pci_dev, is_default_rom);
-    if (rc != 0) {
-        pci_unregister_device(DEVICE(pci_dev));
-        return rc;
+    pci_add_option_rom(pci_dev, is_default_rom, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        pci_qdev_unrealize(DEVICE(pci_dev), NULL);
+        return;
     }
-
-    return 0;
 }
 
 PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
@@ -1923,7 +1929,8 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
 }
 
 /* Add an option rom for the device */
-static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
+static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
+                               Error **errp)
 {
     int size;
     char *path;
@@ -1932,9 +1939,9 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
     const VMStateDescription *vmsd;
 
     if (!pdev->romfile)
-        return 0;
+        return;
     if (strlen(pdev->romfile) == 0)
-        return 0;
+        return;
 
     if (!pdev->rom_bar) {
         /*
@@ -1948,7 +1955,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
          * if the rom bar is disabled.
          */
         if (DEVICE(pdev)->hotplugged) {
-            return 0;
+            return;
         }
 
         if (class == 0x0300) {
@@ -1956,7 +1963,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
         } else {
             rom_add_option(pdev->romfile, -1);
         }
-        return 0;
+        return;
     }
 
     path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile);
@@ -1966,15 +1973,13 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
 
     size = get_image_size(path);
     if (size < 0) {
-        error_report("%s: failed to find romfile \"%s\"",
-                     __func__, pdev->romfile);
+        error_setg(errp, "failed to find romfile \"%s\"", pdev->romfile);
         g_free(path);
-        return -1;
+        return;
     } else if (size == 0) {
-        error_report("%s: ignoring empty romfile \"%s\"",
-                     __func__, pdev->romfile);
+        error_setg(errp, "romfile \"%s\" is empty", pdev->romfile);
         g_free(path);
-        return -1;
+        return;
     }
     if (size & (size - 1)) {
         size = 1 << qemu_fls(size);
@@ -2000,8 +2005,6 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
     }
 
     pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
-
-    return 0;
 }
 
 static void pci_del_option_rom(PCIDevice *pdev)
@@ -2283,8 +2286,8 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
 static void pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
-    k->init = pci_qdev_init;
-    k->exit = pci_unregister_device;
+    k->realize = pci_qdev_realize;
+    k->unrealize = pci_qdev_unrealize;
     k->bus_type = TYPE_PCI_BUS;
     k->props = pci_props;
 }
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 02/10] pci: Permit incremental conversion of device models to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 01/10] pci: Convert core " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:35   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 03/10] pci: Trivial device model conversions " Markus Armbruster
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Call the new PCIDeviceClass method realize().  Default it to
pci_default_realize(), which calls old method init().

To convert a device model, make it implement realize() rather than
init().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pci.c         | 24 +++++++++++++++++++-----
 include/hw/pci/pci.h |  3 ++-
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index aef95c3..e21c0a8 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1753,7 +1753,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
     Error *local_err = NULL;
     PCIBus *bus;
-    int rc;
     bool is_default_rom;
 
     /* initialize cap_present for pci_is_express() and pci_config_size() */
@@ -1768,11 +1767,11 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     if (pci_dev == NULL)
         return;
 
-    if (pc->init) {
-        rc = pc->init(pci_dev);
-        if (rc != 0) {
+    if (pc->realize) {
+        pc->realize(pci_dev, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
             do_pci_unregister_device(pci_dev);
-            error_setg(errp, "Device initialization failed");
             return;
         }
     }
@@ -1792,6 +1791,18 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     }
 }
 
+static void pci_default_realize(PCIDevice *dev, Error **errp)
+{
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+
+    if (pc->init) {
+        if (pc->init(dev) < 0) {
+            error_setg(errp, "Device initialization failed");
+            return;
+        }
+    }
+}
+
 PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
                                     const char *name)
 {
@@ -2286,10 +2297,13 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
 static void pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+
     k->realize = pci_qdev_realize;
     k->unrealize = pci_qdev_unrealize;
     k->bus_type = TYPE_PCI_BUS;
     k->props = pci_props;
+    pc->realize = pci_default_realize;
 }
 
 AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c352c7b..83597b3 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -183,7 +183,8 @@ typedef struct PCIINTxRoute {
 typedef struct PCIDeviceClass {
     DeviceClass parent_class;
 
-    int (*init)(PCIDevice *dev);
+    void (*realize)(PCIDevice *dev, Error **errp);
+    int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
     PCIUnregisterFunc *exit;
     PCIConfigReadFunc *config_read;
     PCIConfigWriteFunc *config_write;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 03/10] pci: Trivial device model conversions to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 01/10] pci: Convert core " Markus Armbruster
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 02/10] pci: Permit incremental conversion of device models " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:38   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void Markus Armbruster
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Convert the device models where initialization obviously can't fail.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/acpi/piix4.c            |  5 ++---
 hw/audio/ac97.c            |  5 ++---
 hw/audio/es1370.c          |  5 ++---
 hw/audio/intel-hda.c       |  6 ++----
 hw/display/vga-pci.c       | 11 ++++-------
 hw/display/vmware_vga.c    |  6 ++----
 hw/i2c/smbus_ich9.c        |  5 ++---
 hw/ide/cmd646.c            |  5 ++---
 hw/ide/piix.c              | 10 ++++------
 hw/ide/via.c               |  6 ++----
 hw/ipack/tpci200.c         |  6 ++----
 hw/isa/i82378.c            |  6 ++----
 hw/isa/piix4.c             |  5 ++---
 hw/isa/vt82c686.c          | 24 ++++++++----------------
 hw/misc/pci-testdev.c      |  6 ++----
 hw/net/e1000.c             |  6 ++----
 hw/net/eepro100.c          |  6 ++----
 hw/net/ne2000.c            |  6 ++----
 hw/net/rtl8139.c           |  6 ++----
 hw/net/vmxnet3.c           |  6 ++----
 hw/pci-bridge/dec.c        |  5 ++---
 hw/pci-host/apb.c          |  5 ++---
 hw/pci-host/bonito.c       |  6 ++----
 hw/pci-host/grackle.c      |  5 ++---
 hw/pci-host/piix.c         | 12 +++++-------
 hw/pci-host/ppce500.c      |  6 ++----
 hw/pci-host/prep.c         |  6 ++----
 hw/pci-host/q35.c          |  5 ++---
 hw/pci-host/uninorth.c     | 20 ++++++++------------
 hw/pci-host/versatile.c    |  5 ++---
 hw/usb/hcd-ehci-pci.c      |  6 ++----
 hw/usb/hcd-xhci.c          |  6 ++----
 hw/watchdog/wdt_i6300esb.c |  6 ++----
 33 files changed, 85 insertions(+), 149 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0bfa814..2018467 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -418,7 +418,7 @@ static void piix4_pm_add_propeties(PIIX4PMState *s)
                                   &s->io_base, NULL);
 }
 
-static int piix4_pm_initfn(PCIDevice *dev)
+static void piix4_pm_realize(PCIDevice *dev, Error **errp)
 {
     PIIX4PMState *s = PIIX4_PM(dev);
     uint8_t *pci_conf;
@@ -468,7 +468,6 @@ static int piix4_pm_initfn(PCIDevice *dev)
     piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, s);
 
     piix4_pm_add_propeties(s);
-    return 0;
 }
 
 Object *piix4_pm_find(void)
@@ -599,7 +598,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
     AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(klass);
 
-    k->init = piix4_pm_initfn;
+    k->realize = piix4_pm_realize;
     k->config_write = pm_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 111ec0e..b173835 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1337,7 +1337,7 @@ static void ac97_on_reset (DeviceState *dev)
     mixer_reset (s);
 }
 
-static int ac97_initfn (PCIDevice *dev)
+static void ac97_realize(PCIDevice *dev, Error **errp)
 {
     AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
     uint8_t *c = s->dev.config;
@@ -1384,7 +1384,6 @@ static int ac97_initfn (PCIDevice *dev)
     pci_register_bar (&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
     AUD_register_card ("ac97", &s->card);
     ac97_on_reset (&s->dev.qdev);
-    return 0;
 }
 
 static int ac97_init (PCIBus *bus)
@@ -1403,7 +1402,7 @@ static void ac97_class_init (ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS (klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS (klass);
 
-    k->init = ac97_initfn;
+    k->realize = ac97_realize;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801AA_5;
     k->revision = 0x01;
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index e67d1ea..8e7bcf5 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -1016,7 +1016,7 @@ static void es1370_on_reset (void *opaque)
     es1370_reset (s);
 }
 
-static int es1370_initfn (PCIDevice *dev)
+static void es1370_realize(PCIDevice *dev, Error **errp)
 {
     ES1370State *s = DO_UPCAST (ES1370State, dev, dev);
     uint8_t *c = s->dev.config;
@@ -1039,7 +1039,6 @@ static int es1370_initfn (PCIDevice *dev)
 
     AUD_register_card ("es1370", &s->card);
     es1370_reset (s);
-    return 0;
 }
 
 static int es1370_init (PCIBus *bus)
@@ -1053,7 +1052,7 @@ static void es1370_class_init (ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS (klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS (klass);
 
-    k->init = es1370_initfn;
+    k->realize = es1370_realize;
     k->vendor_id = PCI_VENDOR_ID_ENSONIQ;
     k->device_id = PCI_DEVICE_ID_ENSONIQ_ES1370;
     k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 2885231..433463e 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1126,7 +1126,7 @@ static void intel_hda_reset(DeviceState *dev)
     intel_hda_update_irq(d);
 }
 
-static int intel_hda_init(PCIDevice *pci)
+static void intel_hda_realize(PCIDevice *pci, Error **errp)
 {
     IntelHDAState *d = INTEL_HDA(pci);
     uint8_t *conf = d->pci.config;
@@ -1147,8 +1147,6 @@ static int intel_hda_init(PCIDevice *pci)
 
     hda_codec_bus_init(DEVICE(pci), &d->codecs, sizeof(d->codecs),
                        intel_hda_response, intel_hda_xfer);
-
-    return 0;
 }
 
 static void intel_hda_exit(PCIDevice *pci)
@@ -1245,7 +1243,7 @@ static void intel_hda_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = intel_hda_init;
+    k->realize = intel_hda_realize;
     k->exit = intel_hda_exit;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->class_id = PCI_CLASS_MULTIMEDIA_HD_AUDIO;
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index db922f1..ddc9f73 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -189,7 +189,7 @@ static const MemoryRegionOps pci_vga_qext_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static int pci_std_vga_initfn(PCIDevice *dev)
+static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
 {
     PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev);
     VGACommonState *s = &d->vga;
@@ -232,11 +232,9 @@ static int pci_std_vga_initfn(PCIDevice *dev)
         /* compatibility with pc-0.13 and older */
         vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
     }
-
-    return 0;
 }
 
-static int pci_secondary_vga_initfn(PCIDevice *dev)
+static void pci_secondary_vga_realize(PCIDevice *dev, Error **errp)
 {
     PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev);
     VGACommonState *s = &d->vga;
@@ -268,7 +266,6 @@ static int pci_secondary_vga_initfn(PCIDevice *dev)
     pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram);
     pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
 
-    return 0;
 }
 
 static void pci_secondary_vga_reset(DeviceState *dev)
@@ -298,7 +295,7 @@ static void vga_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_std_vga_initfn;
+    k->realize = pci_std_vga_realize;
     k->romfile = "vgabios-stdvga.bin";
     k->vendor_id = PCI_VENDOR_ID_QEMU;
     k->device_id = PCI_DEVICE_ID_QEMU_VGA;
@@ -314,7 +311,7 @@ static void secondary_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_secondary_vga_initfn;
+    k->realize = pci_secondary_vga_realize;
     k->vendor_id = PCI_VENDOR_ID_QEMU;
     k->device_id = PCI_DEVICE_ID_QEMU_VGA;
     k->class_id = PCI_CLASS_DISPLAY_OTHER;
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 0c36c72..1fd3904 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1258,7 +1258,7 @@ static const MemoryRegionOps vmsvga_io_ops = {
     },
 };
 
-static int pci_vmsvga_initfn(PCIDevice *dev)
+static void pci_vmsvga_realize(PCIDevice *dev, Error **errp)
 {
     struct pci_vmsvga_state_s *s = VMWARE_SVGA(dev);
 
@@ -1283,8 +1283,6 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
         /* compatibility with pc-0.13 and older */
         vga_init_vbe(&s->chip.vga, OBJECT(dev), pci_address_space(dev));
     }
-
-    return 0;
 }
 
 static Property vga_vmware_properties[] = {
@@ -1298,7 +1296,7 @@ static void vmsvga_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_vmsvga_initfn;
+    k->realize = pci_vmsvga_realize;
     k->romfile = "vgabios-vmware.bin";
     k->vendor_id = PCI_VENDOR_ID_VMWARE;
     k->device_id = SVGA_PCI_DEVICE_ID;
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index 0803dc4..91d4d32 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -71,7 +71,7 @@ static void ich9_smbus_write_config(PCIDevice *d, uint32_t address,
     }
 }
 
-static int ich9_smbus_initfn(PCIDevice *d)
+static void ich9_smbus_realize(PCIDevice *d, Error **errp)
 {
     ICH9SMBState *s = ICH9_SMB_DEVICE(d);
 
@@ -84,7 +84,6 @@ static int ich9_smbus_initfn(PCIDevice *d)
     pm_smbus_init(&d->qdev, &s->smb);
     pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
                      &s->smb.io);
-    return 0;
 }
 
 static void ich9_smb_class_init(ObjectClass *klass, void *data)
@@ -98,7 +97,7 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_SERIAL_SMBUS;
     dc->vmsd = &vmstate_ich9_smbus;
     dc->desc = "ICH9 SMBUS Bridge";
-    k->init = ich9_smbus_initfn;
+    k->realize = ich9_smbus_realize;
     k->config_write = ich9_smbus_write_config;
     /*
      * Reason: part of ICH9 southbridge, needs to be wired up by
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c8b0322..dafa9ec 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -326,7 +326,7 @@ static void cmd646_pci_config_write(PCIDevice *d, uint32_t addr, uint32_t val,
 }
 
 /* CMD646 PCI IDE controller */
-static int pci_cmd646_ide_initfn(PCIDevice *dev)
+static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
 {
     PCIIDEState *d = PCI_IDE(dev);
     uint8_t *pci_conf = dev->config;
@@ -374,7 +374,6 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 
     vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
     qemu_register_reset(cmd646_reset, d);
-    return 0;
 }
 
 static void pci_cmd646_ide_exitfn(PCIDevice *dev)
@@ -410,7 +409,7 @@ static void cmd646_ide_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_cmd646_ide_initfn;
+    k->realize = pci_cmd646_ide_realize;
     k->exit = pci_cmd646_ide_exitfn;
     k->vendor_id = PCI_VENDOR_ID_CMD;
     k->device_id = PCI_DEVICE_ID_CMD_646;
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 574b9c1..6bef60c 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -148,7 +148,7 @@ static void pci_piix_init_ports(PCIIDEState *d) {
     }
 }
 
-static int pci_piix_ide_initfn(PCIDevice *dev)
+static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 {
     PCIIDEState *d = PCI_IDE(dev);
     uint8_t *pci_conf = dev->config;
@@ -163,8 +163,6 @@ static int pci_piix_ide_initfn(PCIDevice *dev)
     vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
 
     pci_piix_init_ports(d);
-
-    return 0;
 }
 
 int pci_piix3_xen_ide_unplug(DeviceState *dev)
@@ -238,7 +236,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_piix_ide_initfn;
+    k->realize = pci_piix_ide_realize;
     k->exit = pci_piix_ide_exitfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
@@ -258,7 +256,7 @@ static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_piix_ide_initfn;
+    k->realize = pci_piix_ide_realize;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
@@ -276,7 +274,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_piix_ide_initfn;
+    k->realize = pci_piix_ide_realize;
     k->exit = pci_piix_ide_exitfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 4d8089d..7a5151c 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -172,7 +172,7 @@ static void vt82c686b_init_ports(PCIIDEState *d) {
 }
 
 /* via ide func */
-static int vt82c686b_ide_initfn(PCIDevice *dev)
+static void vt82c686b_ide_realize(PCIDevice *dev, Error **errp)
 {
     PCIIDEState *d = PCI_IDE(dev);
     uint8_t *pci_conf = dev->config;
@@ -187,8 +187,6 @@ static int vt82c686b_ide_initfn(PCIDevice *dev)
     vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d);
 
     vt82c686b_init_ports(d);
-
-    return 0;
 }
 
 static void vt82c686b_ide_exitfn(PCIDevice *dev)
@@ -215,7 +213,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = vt82c686b_ide_initfn;
+    k->realize = vt82c686b_ide_realize;
     k->exit = vt82c686b_ide_exitfn;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_IDE;
diff --git a/hw/ipack/tpci200.c b/hw/ipack/tpci200.c
index b7031a0..1df02ee 100644
--- a/hw/ipack/tpci200.c
+++ b/hw/ipack/tpci200.c
@@ -573,7 +573,7 @@ static const MemoryRegionOps tpci200_las3_ops = {
     }
 };
 
-static int tpci200_initfn(PCIDevice *pci_dev)
+static void tpci200_realize(PCIDevice *pci_dev, Error **errp)
 {
     TPCI200State *s = TPCI200(pci_dev);
     uint8_t *c = s->dev.config;
@@ -609,8 +609,6 @@ static int tpci200_initfn(PCIDevice *pci_dev)
 
     ipack_bus_new_inplace(&s->bus, sizeof(s->bus), DEVICE(pci_dev), NULL,
                           N_MODULES, tpci200_set_irq);
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_tpci200 = {
@@ -632,7 +630,7 @@ static void tpci200_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = tpci200_initfn;
+    k->realize = tpci200_realize;
     k->vendor_id = PCI_VENDOR_ID_TEWS;
     k->device_id = PCI_DEVICE_ID_TEWS_TPCI200;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index a7d9aa6..9fcba4f 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -58,7 +58,7 @@ static void i82378_request_pic_irq(void *opaque, int irq, int level)
     qemu_set_irq(s->i8259[irq], level);
 }
 
-static int i82378_initfn(PCIDevice *pci)
+static void i82378_realize(PCIDevice *pci, Error **errp)
 {
     DeviceState *dev = DEVICE(pci);
     I82378State *s = I82378(dev);
@@ -106,8 +106,6 @@ static int i82378_initfn(PCIDevice *pci)
 
     /* timer */
     isa_create_simple(isabus, "mc146818rtc");
-
-    return 0;
 }
 
 static void i82378_init(Object *obj)
@@ -124,7 +122,7 @@ static void i82378_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = i82378_initfn;
+    k->realize = i82378_realize;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82378;
     k->revision = 0x03;
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 1aa17d7..25cf73a 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -82,14 +82,13 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static int piix4_initfn(PCIDevice *dev)
+static void piix4_realize(PCIDevice *dev, Error **errp)
 {
     PIIX4State *d = DO_UPCAST(PIIX4State, dev, dev);
 
     isa_bus_new(&d->dev.qdev, pci_address_space_io(dev));
     piix4_dev = &d->dev;
     qemu_register_reset(piix4_reset, d);
-    return 0;
 }
 
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
@@ -106,7 +105,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = piix4_initfn;
+    k->realize = piix4_realize;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index e0c235c..e7cf474 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -248,7 +248,7 @@ static const VMStateDescription vmstate_acpi = {
  * just register a PCI device now, functionalities will be implemented later.
  */
 
-static int vt82c686b_ac97_initfn(PCIDevice *dev)
+static void vt82c686b_ac97_realize(PCIDevice *dev, Error **errp)
 {
     VT686AC97State *s = DO_UPCAST(VT686AC97State, dev, dev);
     uint8_t *pci_conf = s->dev.config;
@@ -258,8 +258,6 @@ static int vt82c686b_ac97_initfn(PCIDevice *dev)
     pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_CAP_LIST |
                  PCI_STATUS_DEVSEL_MEDIUM);
     pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
-
-    return 0;
 }
 
 void vt82c686b_ac97_init(PCIBus *bus, int devfn)
@@ -275,7 +273,7 @@ static void via_ac97_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = vt82c686b_ac97_initfn;
+    k->realize = vt82c686b_ac97_realize;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_AC97;
     k->revision = 0x50;
@@ -291,7 +289,7 @@ static const TypeInfo via_ac97_info = {
     .class_init    = via_ac97_class_init,
 };
 
-static int vt82c686b_mc97_initfn(PCIDevice *dev)
+static void vt82c686b_mc97_realize(PCIDevice *dev, Error **errp)
 {
     VT686MC97State *s = DO_UPCAST(VT686MC97State, dev, dev);
     uint8_t *pci_conf = s->dev.config;
@@ -300,8 +298,6 @@ static int vt82c686b_mc97_initfn(PCIDevice *dev)
                  PCI_COMMAND_VGA_PALETTE);
     pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
     pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
-
-    return 0;
 }
 
 void vt82c686b_mc97_init(PCIBus *bus, int devfn)
@@ -317,7 +313,7 @@ static void via_mc97_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = vt82c686b_mc97_initfn;
+    k->realize = vt82c686b_mc97_realize;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_MC97;
     k->class_id = PCI_CLASS_COMMUNICATION_OTHER;
@@ -334,7 +330,7 @@ static const TypeInfo via_mc97_info = {
 };
 
 /* vt82c686 pm init */
-static int vt82c686b_pm_initfn(PCIDevice *dev)
+static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 {
     VT686PMState *s = DO_UPCAST(VT686PMState, dev, dev);
     uint8_t *pci_conf;
@@ -364,8 +360,6 @@ static int vt82c686b_pm_initfn(PCIDevice *dev)
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer, &s->io);
     acpi_pm1_evt_init(&s->ar, pm_tmr_timer, &s->io);
     acpi_pm1_cnt_init(&s->ar, &s->io, 2);
-
-    return 0;
 }
 
 I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
@@ -394,7 +388,7 @@ static void via_pm_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = vt82c686b_pm_initfn;
+    k->realize = vt82c686b_pm_realize;
     k->config_write = pm_write_config;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_ACPI;
@@ -424,7 +418,7 @@ static const VMStateDescription vmstate_via = {
 };
 
 /* init the PCI-to-ISA bridge */
-static int vt82c686b_initfn(PCIDevice *d)
+static void vt82c686b_realize(PCIDevice *d, Error **errp)
 {
     VT82C686BState *vt82c = DO_UPCAST(VT82C686BState, dev, d);
     uint8_t *pci_conf;
@@ -453,8 +447,6 @@ static int vt82c686b_initfn(PCIDevice *d)
                                 &vt82c->superio);
 
     qemu_register_reset(vt82c686b_reset, d);
-
-    return 0;
 }
 
 ISABus *vt82c686b_init(PCIBus *bus, int devfn)
@@ -471,7 +463,7 @@ static void via_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = vt82c686b_initfn;
+    k->realize = vt82c686b_realize;
     k->config_write = vt82c686b_write_config;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_ISA_BRIDGE;
diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c
index c78a63e..26b9b86 100644
--- a/hw/misc/pci-testdev.c
+++ b/hw/misc/pci-testdev.c
@@ -233,7 +233,7 @@ static const MemoryRegionOps pci_testdev_pio_ops = {
     },
 };
 
-static int pci_testdev_init(PCIDevice *pci_dev)
+static void pci_testdev_realize(PCIDevice *pci_dev, Error **errp)
 {
     PCITestDevState *d = PCI_TEST_DEV(pci_dev);
     uint8_t *pci_conf;
@@ -275,8 +275,6 @@ static int pci_testdev_init(PCIDevice *pci_dev)
         assert(r >= 0);
         test->hasnotifier = true;
     }
-
-    return 0;
 }
 
 static void
@@ -306,7 +304,7 @@ static void pci_testdev_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_testdev_init;
+    k->realize = pci_testdev_realize;
     k->exit = pci_testdev_uninit;
     k->vendor_id = PCI_VENDOR_ID_REDHAT;
     k->device_id = PCI_DEVICE_ID_REDHAT_TEST;
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..af8eef5 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1529,7 +1529,7 @@ static NetClientInfo net_e1000_info = {
     .link_status_changed = e1000_set_link_status,
 };
 
-static int pci_e1000_init(PCIDevice *pci_dev)
+static void pci_e1000_realize(PCIDevice *pci_dev, Error **errp)
 {
     DeviceState *dev = DEVICE(pci_dev);
     E1000State *d = E1000(pci_dev);
@@ -1571,8 +1571,6 @@ static int pci_e1000_init(PCIDevice *pci_dev)
 
     d->autoneg_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, e1000_autoneg_timer, d);
     d->mit_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, e1000_mit_timer, d);
-
-    return 0;
 }
 
 static void qdev_e1000_reset(DeviceState *dev)
@@ -1604,7 +1602,7 @@ static void e1000_class_init(ObjectClass *klass, void *data)
     E1000BaseClass *e = E1000_DEVICE_CLASS(klass);
     const E1000Info *info = data;
 
-    k->init = pci_e1000_init;
+    k->realize = pci_e1000_realize;
     k->exit = pci_e1000_uninit;
     k->romfile = "efi-e1000.rom";
     k->vendor_id = PCI_VENDOR_ID_INTEL;
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 4877bfd..e72a3b1 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1856,7 +1856,7 @@ static NetClientInfo net_eepro100_info = {
     .cleanup = nic_cleanup,
 };
 
-static int e100_nic_init(PCIDevice *pci_dev)
+static void e100_nic_realize(PCIDevice *pci_dev, Error **errp)
 {
     EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
     E100PCIDeviceInfo *info = eepro100_get_class(s);
@@ -1900,8 +1900,6 @@ static int e100_nic_init(PCIDevice *pci_dev)
     memcpy(s->vmstate, &vmstate_eepro100, sizeof(vmstate_eepro100));
     s->vmstate->name = qemu_get_queue(s->nic)->model;
     vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
-
-    return 0;
 }
 
 static void eepro100_instance_init(Object *obj)
@@ -2091,7 +2089,7 @@ static void eepro100_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
     k->romfile = "pxe-eepro100.rom";
-    k->init = e100_nic_init;
+    k->realize = e100_nic_realize;
     k->exit = pci_nic_uninit;
     k->device_id = info->device_id;
     k->revision = info->revision;
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 3ab2d03..449d87a 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -717,7 +717,7 @@ static NetClientInfo net_ne2000_info = {
     .cleanup = ne2000_cleanup,
 };
 
-static int pci_ne2000_init(PCIDevice *pci_dev)
+static void pci_ne2000_realize(PCIDevice *pci_dev, Error **errp)
 {
     PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
     NE2000State *s;
@@ -737,8 +737,6 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
     s->nic = qemu_new_nic(&net_ne2000_info, &s->c,
                           object_get_typename(OBJECT(pci_dev)), pci_dev->qdev.id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->c.macaddr.a);
-
-    return 0;
 }
 
 static void pci_ne2000_exit(PCIDevice *pci_dev)
@@ -771,7 +769,7 @@ static void ne2000_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_ne2000_init;
+    k->realize = pci_ne2000_realize;
     k->exit = pci_ne2000_exit;
     k->romfile = "efi-ne2k_pci.rom",
     k->vendor_id = PCI_VENDOR_ID_REALTEK;
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 8b8a1b1..e296bbc 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3494,7 +3494,7 @@ static NetClientInfo net_rtl8139_info = {
     .link_status_changed = rtl8139_set_link_status,
 };
 
-static int pci_rtl8139_init(PCIDevice *dev)
+static void pci_rtl8139_realize(PCIDevice *dev, Error **errp)
 {
     RTL8139State *s = RTL8139(dev);
     DeviceState *d = DEVICE(dev);
@@ -3537,8 +3537,6 @@ static int pci_rtl8139_init(PCIDevice *dev)
     s->TimerExpire = 0;
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, rtl8139_timer, s);
     rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-
-    return 0;
 }
 
 static void rtl8139_instance_init(Object *obj)
@@ -3560,7 +3558,7 @@ static void rtl8139_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_rtl8139_init;
+    k->realize = pci_rtl8139_realize;
     k->exit = pci_rtl8139_uninit;
     k->romfile = "efi-rtl8139.rom";
     k->vendor_id = PCI_VENDOR_ID_REALTEK;
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 8eea589..ee5df86 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2132,7 +2132,7 @@ static const MemoryRegionOps b1_ops = {
     },
 };
 
-static int vmxnet3_pci_init(PCIDevice *pci_dev)
+static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
 {
     DeviceState *dev = DEVICE(pci_dev);
     VMXNET3State *s = VMXNET3(pci_dev);
@@ -2171,8 +2171,6 @@ static int vmxnet3_pci_init(PCIDevice *pci_dev)
 
     register_savevm(dev, "vmxnet3-msix", -1, 1,
                     vmxnet3_msix_save, vmxnet3_msix_load, s);
-
-    return 0;
 }
 
 static void vmxnet3_instance_init(Object *obj)
@@ -2508,7 +2506,7 @@ static void vmxnet3_class_init(ObjectClass *class, void *data)
     DeviceClass *dc = DEVICE_CLASS(class);
     PCIDeviceClass *c = PCI_DEVICE_CLASS(class);
 
-    c->init = vmxnet3_pci_init;
+    c->realize = vmxnet3_pci_realize;
     c->exit = vmxnet3_pci_uninit;
     c->vendor_id = PCI_VENDOR_ID_VMWARE;
     c->device_id = PCI_DEVICE_ID_VMWARE_VMXNET3;
diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c
index a6ca940..28d0ff9 100644
--- a/hw/pci-bridge/dec.c
+++ b/hw/pci-bridge/dec.c
@@ -107,10 +107,9 @@ static int pci_dec_21154_device_init(SysBusDevice *dev)
     return 0;
 }
 
-static int dec_21154_pci_host_init(PCIDevice *d)
+static void dec_21154_pci_host_realize(PCIDevice *d, Error **errp)
 {
     /* PCI2PCI bridge same values as PearPC - check this */
-    return 0;
 }
 
 static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
@@ -118,7 +117,7 @@ static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = dec_21154_pci_host_init;
+    k->realize = dec_21154_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_DEC;
     k->device_id = PCI_DEVICE_ID_DEC_21154;
     k->revision = 0x02;
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index f573875..89b5040 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -791,14 +791,13 @@ static int pci_pbm_init_device(SysBusDevice *dev)
     return 0;
 }
 
-static int pbm_pci_host_init(PCIDevice *d)
+static void pbm_pci_host_realize(PCIDevice *d, Error **errp)
 {
     pci_set_word(d->config + PCI_COMMAND,
                  PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
     pci_set_word(d->config + PCI_STATUS,
                  PCI_STATUS_FAST_BACK | PCI_STATUS_66MHZ |
                  PCI_STATUS_DEVSEL_MEDIUM);
-    return 0;
 }
 
 static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
@@ -806,7 +805,7 @@ static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = pbm_pci_host_init;
+    k->realize = pbm_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_SUN;
     k->device_id = PCI_DEVICE_ID_SUN_SABRE;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 56292ad..8bdd569 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -705,7 +705,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
     return 0;
 }
 
-static int bonito_initfn(PCIDevice *dev)
+static void bonito_realize(PCIDevice *dev, Error **errp)
 {
     PCIBonitoState *s = DO_UPCAST(PCIBonitoState, dev, dev);
     SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
@@ -766,8 +766,6 @@ static int bonito_initfn(PCIDevice *dev)
     pci_set_byte(dev->config + PCI_MAX_LAT, 0x00);
 
     qemu_register_reset(bonito_reset, s);
-
-    return 0;
 }
 
 PCIBus *bonito_init(qemu_irq *pic)
@@ -799,7 +797,7 @@ static void bonito_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = bonito_initfn;
+    k->realize = bonito_realize;
     k->vendor_id = 0xdf53;
     k->device_id = 0x00d5;
     k->revision = 0x01;
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 6c7cfdb..bfe707a 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -114,10 +114,9 @@ static int pci_grackle_init_device(SysBusDevice *dev)
     return 0;
 }
 
-static int grackle_pci_host_init(PCIDevice *d)
+static void grackle_pci_host_realize(PCIDevice *d, Error **errp)
 {
     d->config[0x09] = 0x01;
-    return 0;
 }
 
 static void grackle_pci_class_init(ObjectClass *klass, void *data)
@@ -125,7 +124,7 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init      = grackle_pci_host_init;
+    k->realize   = grackle_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_MOTOROLA;
     k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
     k->revision  = 0x00;
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1530038..72f1ee2 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -295,14 +295,13 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
     sysbus_init_ioports(sbd, 0xcfc, 4);
 }
 
-static int i440fx_initfn(PCIDevice *dev)
+static void i440fx_realize(PCIDevice *dev, Error **errp)
 {
     PCII440FXState *d = I440FX_PCI_DEVICE(dev);
 
     dev->config[I440FX_SMRAM] = 0x02;
 
     cpu_smm_register(&i440fx_set_smm, d);
-    return 0;
 }
 
 PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
@@ -631,7 +630,7 @@ static const MemoryRegionOps rcr_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN
 };
 
-static int piix3_initfn(PCIDevice *dev)
+static void piix3_realize(PCIDevice *dev, Error **errp)
 {
     PIIX3State *d = DO_UPCAST(PIIX3State, dev, dev);
 
@@ -643,7 +642,6 @@ static int piix3_initfn(PCIDevice *dev)
                                         &d->rcr_mem, 1);
 
     qemu_register_reset(piix3_reset, d);
-    return 0;
 }
 
 static void piix3_class_init(ObjectClass *klass, void *data)
@@ -654,7 +652,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
     dc->hotpluggable   = false;
-    k->init         = piix3_initfn;
+    k->realize      = piix3_realize;
     k->config_write = piix3_write_config;
     k->vendor_id    = PCI_VENDOR_ID_INTEL;
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
@@ -682,7 +680,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
     dc->desc        = "ISA bridge";
     dc->vmsd        = &vmstate_piix3;
     dc->hotpluggable   = false;
-    k->init         = piix3_initfn;
+    k->realize      = piix3_realize;
     k->config_write = piix3_write_config_xen;
     k->vendor_id    = PCI_VENDOR_ID_INTEL;
     /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
@@ -707,7 +705,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = i440fx_initfn;
+    k->realize = i440fx_realize;
     k->config_write = i440fx_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82441;
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 1b4c0f0..85d2bfb 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -331,7 +331,7 @@ static const VMStateDescription vmstate_ppce500_pci = {
 
 #include "exec/address-spaces.h"
 
-static int e500_pcihost_bridge_initfn(PCIDevice *d)
+static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp)
 {
     PPCE500PCIBridgeState *b = PPC_E500_PCI_BRIDGE(d);
     PPCE500CCSRState *ccsr = CCSR(container_get(qdev_get_machine(),
@@ -345,8 +345,6 @@ static int e500_pcihost_bridge_initfn(PCIDevice *d)
     memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", &ccsr->ccsr_space,
                              0, int128_get64(ccsr->ccsr_space.size));
     pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0);
-
-    return 0;
 }
 
 static int e500_pcihost_initfn(SysBusDevice *dev)
@@ -399,7 +397,7 @@ static void e500_host_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = e500_pcihost_bridge_initfn;
+    k->realize = e500_pcihost_bridge_realize;
     k->vendor_id = PCI_VENDOR_ID_FREESCALE;
     k->device_id = PCI_DEVICE_ID_MPC8533E;
     k->class_id = PCI_CLASS_PROCESSOR_POWERPC;
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 1de3681..6cea6ff 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -289,7 +289,7 @@ static void raven_pcihost_initfn(Object *obj)
     qdev_prop_set_bit(pci_dev, "multifunction", false);
 }
 
-static int raven_init(PCIDevice *d)
+static void raven_realize(PCIDevice *d, Error **errp)
 {
     RavenPCIState *s = RAVEN_PCI_DEVICE(d);
     char *filename;
@@ -330,8 +330,6 @@ static int raven_init(PCIDevice *d)
             g_free(filename);
         }
     }
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_raven = {
@@ -349,7 +347,7 @@ static void raven_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = raven_init;
+    k->realize = raven_realize;
     k->vendor_id = PCI_VENDOR_ID_MOTOROLA;
     k->device_id = PCI_DEVICE_ID_MOTOROLA_RAVEN;
     k->revision = 0x00;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b20bad8..df60e61 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -390,7 +390,7 @@ static void mch_init_dmar(MCHPCIState *mch)
     pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch->iommu);
 }
 
-static int mch_init(PCIDevice *d)
+static void mch_realize(PCIDevice *d, Error **errp)
 {
     int i;
     MCHPCIState *mch = MCH_PCI_DEVICE(d);
@@ -418,7 +418,6 @@ static int mch_init(PCIDevice *d)
     if (qemu_opt_get_bool(qemu_get_machine_opts(), "iommu", false)) {
         mch_init_dmar(mch);
     }
-    return 0;
 }
 
 uint64_t mch_mcfg_base(void)
@@ -436,7 +435,7 @@ static void mch_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = mch_init;
+    k->realize = mch_realize;
     k->config_write = mch_write_config;
     dc->reset = mch_reset;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index 21f805f..53f2b59 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -315,37 +315,33 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
     return h->bus;
 }
 
-static int unin_main_pci_host_init(PCIDevice *d)
+static void unin_main_pci_host_realize(PCIDevice *d, Error **errp)
 {
     d->config[0x0C] = 0x08; // cache_line_size
     d->config[0x0D] = 0x10; // latency_timer
     d->config[0x34] = 0x00; // capabilities_pointer
-    return 0;
 }
 
-static int unin_agp_pci_host_init(PCIDevice *d)
+static void unin_agp_pci_host_realize(PCIDevice *d, Error **errp)
 {
     d->config[0x0C] = 0x08; // cache_line_size
     d->config[0x0D] = 0x10; // latency_timer
     //    d->config[0x34] = 0x80; // capabilities_pointer
-    return 0;
 }
 
-static int u3_agp_pci_host_init(PCIDevice *d)
+static void u3_agp_pci_host_realize(PCIDevice *d, Error **errp)
 {
     /* cache line size */
     d->config[0x0C] = 0x08;
     /* latency timer */
     d->config[0x0D] = 0x10;
-    return 0;
 }
 
-static int unin_internal_pci_host_init(PCIDevice *d)
+static void unin_internal_pci_host_realize(PCIDevice *d, Error **errp)
 {
     d->config[0x0C] = 0x08; // cache_line_size
     d->config[0x0D] = 0x10; // latency_timer
     d->config[0x34] = 0x00; // capabilities_pointer
-    return 0;
 }
 
 static void unin_main_pci_host_class_init(ObjectClass *klass, void *data)
@@ -353,7 +349,7 @@ static void unin_main_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init      = unin_main_pci_host_init;
+    k->realize   = unin_main_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_APPLE;
     k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_PCI;
     k->revision  = 0x00;
@@ -377,7 +373,7 @@ static void u3_agp_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init      = u3_agp_pci_host_init;
+    k->realize   = u3_agp_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_APPLE;
     k->device_id = PCI_DEVICE_ID_APPLE_U3_AGP;
     k->revision  = 0x00;
@@ -401,7 +397,7 @@ static void unin_agp_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init      = unin_agp_pci_host_init;
+    k->realize   = unin_agp_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_APPLE;
     k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_AGP;
     k->revision  = 0x00;
@@ -425,7 +421,7 @@ static void unin_internal_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init      = unin_internal_pci_host_init;
+    k->realize   = unin_internal_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_APPLE;
     k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_I_PCI;
     k->revision  = 0x00;
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 71ff0de..6d23553 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -456,12 +456,11 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
     object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp);
 }
 
-static int versatile_pci_host_init(PCIDevice *d)
+static void versatile_pci_host_realize(PCIDevice *d, Error **errp)
 {
     pci_set_word(d->config + PCI_STATUS,
                  PCI_STATUS_66MHZ | PCI_STATUS_DEVSEL_MEDIUM);
     pci_set_byte(d->config + PCI_LATENCY_TIMER, 0x10);
-    return 0;
 }
 
 static void versatile_pci_host_class_init(ObjectClass *klass, void *data)
@@ -469,7 +468,7 @@ static void versatile_pci_host_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = versatile_pci_host_init;
+    k->realize = versatile_pci_host_realize;
     k->vendor_id = PCI_VENDOR_ID_XILINX;
     k->device_id = PCI_DEVICE_ID_XILINX_XC2VP30;
     k->class_id = PCI_CLASS_PROCESSOR_CO;
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 490f2b6..4c80707 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -26,7 +26,7 @@ typedef struct EHCIPCIInfo {
     bool companion;
 } EHCIPCIInfo;
 
-static int usb_ehci_pci_initfn(PCIDevice *dev)
+static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
 {
     EHCIPCIState *i = PCI_EHCI(dev);
     EHCIState *s = &i->ehci;
@@ -66,8 +66,6 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
 
     usb_ehci_realize(s, DEVICE(dev), NULL);
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
-
-    return 0;
 }
 
 static void usb_ehci_pci_init(Object *obj)
@@ -139,7 +137,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = usb_ehci_pci_initfn;
+    k->realize = usb_ehci_pci_realize;
     k->exit = usb_ehci_pci_exit;
     k->class_id = PCI_CLASS_SERIAL_USB;
     k->config_write = usb_ehci_pci_write_config;
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index a27c9d3..07f40d5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3562,7 +3562,7 @@ static void usb_xhci_init(XHCIState *xhci)
     }
 }
 
-static int usb_xhci_initfn(struct PCIDevice *dev)
+static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
 {
     int i, ret;
 
@@ -3636,8 +3636,6 @@ static int usb_xhci_initfn(struct PCIDevice *dev)
                   &xhci->mem, 0, OFF_MSIX_PBA,
                   0x90);
     }
-
-    return 0;
 }
 
 static void usb_xhci_exit(PCIDevice *dev)
@@ -3869,7 +3867,7 @@ static void xhci_class_init(ObjectClass *klass, void *data)
     dc->props   = xhci_properties;
     dc->reset   = xhci_reset;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
-    k->init         = usb_xhci_initfn;
+    k->realize      = usb_xhci_realize;
     k->exit         = usb_xhci_exit;
     k->vendor_id    = PCI_VENDOR_ID_NEC;
     k->device_id    = PCI_DEVICE_ID_NEC_UPD720200;
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index 687c8b1..708e23d 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -408,7 +408,7 @@ static const VMStateDescription vmstate_i6300esb = {
     }
 };
 
-static int i6300esb_init(PCIDevice *dev)
+static void i6300esb_realize(PCIDevice *dev, Error **errp)
 {
     I6300State *d = DO_UPCAST(I6300State, dev, dev);
 
@@ -421,8 +421,6 @@ static int i6300esb_init(PCIDevice *dev)
                           "i6300esb", 0x10);
     pci_register_bar(&d->dev, 0, 0, &d->io_mem);
     /* qemu_register_coalesced_mmio (addr, 0x10); ? */
-
-    return 0;
 }
 
 static WatchdogTimerModel model = {
@@ -437,7 +435,7 @@ static void i6300esb_class_init(ObjectClass *klass, void *data)
 
     k->config_read = i6300esb_config_read;
     k->config_write = i6300esb_config_write;
-    k->init = i6300esb_init;
+    k->realize = i6300esb_realize;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_ESB_9;
     k->class_id = PCI_CLASS_SYSTEM_OTHER;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (2 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 03/10] pci: Trivial device model conversions " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:42   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 05/10] pcnet: Convert to realize Markus Armbruster
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

The next commit will exploit the fact it never fails.  This one makes
it obvious.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/lance.c     | 3 ++-
 hw/net/pcnet-pci.c | 3 ++-
 hw/net/pcnet.c     | 4 +---
 hw/net/pcnet.h     | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/net/lance.c b/hw/net/lance.c
index a1c49f1..3663340 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -134,7 +134,8 @@ static int lance_init(SysBusDevice *sbd)
 
     s->phys_mem_read = ledma_memory_read;
     s->phys_mem_write = ledma_memory_write;
-    return pcnet_common_init(dev, s, &net_lance_info);
+    pcnet_common_init(dev, s, &net_lance_info);
+    return 0;
 }
 
 static void lance_reset(DeviceState *dev)
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index fb5f5d6..50eb069 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -335,7 +335,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     s->phys_mem_write = pci_physical_memory_write;
     s->dma_opaque = pci_dev;
 
-    return pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
+    pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
+    return 0;
 }
 
 static void pci_reset(DeviceState *dev)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index d344c15..5a081c4 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1724,7 +1724,7 @@ void pcnet_common_cleanup(PCNetState *d)
     d->nic = NULL;
 }
 
-int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
+void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 {
     int i;
     uint16_t checksum;
@@ -1763,6 +1763,4 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
     *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
 
     s->lnkst = 0x40; /* initial link state: up */
-
-    return 0;
 }
diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
index f8e8a6f..cfc196e 100644
--- a/hw/net/pcnet.h
+++ b/hw/net/pcnet.h
@@ -64,6 +64,6 @@ int pcnet_can_receive(NetClientState *nc);
 ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_);
 void pcnet_set_link_status(NetClientState *nc);
 void pcnet_common_cleanup(PCNetState *d);
-int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
+void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
 extern const VMStateDescription vmstate_pcnet;
 #endif
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 05/10] pcnet: Convert to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (3 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 06/10] serial-pci: " Markus Armbruster
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/pcnet-pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index 50eb069..e5122a0 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -297,7 +297,7 @@ static NetClientInfo net_pci_pcnet_info = {
     .cleanup = pci_pcnet_cleanup,
 };
 
-static int pci_pcnet_init(PCIDevice *pci_dev)
+static void pci_pcnet_realize(PCIDevice *pci_dev, Error **errp)
 {
     PCIPCNetState *d = PCI_PCNET(pci_dev);
     PCNetState *s = &d->state;
@@ -336,7 +336,6 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     s->dma_opaque = pci_dev;
 
     pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
-    return 0;
 }
 
 static void pci_reset(DeviceState *dev)
@@ -366,7 +365,7 @@ static void pcnet_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_pcnet_init;
+    k->realize = pci_pcnet_realize;
     k->exit = pci_pcnet_uninit;
     k->romfile = "efi-pcnet.rom",
     k->vendor_id = PCI_VENDOR_ID_AMD;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 06/10] serial-pci: Convert to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (4 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 05/10] pcnet: Convert to realize Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:43   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 07/10] ide/ich: " Markus Armbruster
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/char/serial-pci.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index f05c9b4..467c3b4 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -48,7 +48,7 @@ typedef struct PCIMultiSerialState {
     uint8_t      prog_if;
 } PCIMultiSerialState;
 
-static int serial_pci_init(PCIDevice *dev)
+static void serial_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
     SerialState *s = &pci->state;
@@ -57,9 +57,8 @@ static int serial_pci_init(PCIDevice *dev)
     s->baudbase = 115200;
     serial_realize_core(s, &err);
     if (err != NULL) {
-        qerror_report_err(err);
-        error_free(err);
-        return -1;
+        error_propagate(errp, err);
+        return;
     }
 
     pci->dev.config[PCI_CLASS_PROG] = pci->prog_if;
@@ -68,7 +67,6 @@ static int serial_pci_init(PCIDevice *dev)
 
     memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s, "serial", 8);
     pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
-    return 0;
 }
 
 static void multi_serial_irq_mux(void *opaque, int n, int level)
@@ -85,7 +83,7 @@ static void multi_serial_irq_mux(void *opaque, int n, int level)
     pci_set_irq(&pci->dev, pending);
 }
 
-static int multi_serial_pci_init(PCIDevice *dev)
+static void multi_serial_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
     PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
@@ -116,9 +114,8 @@ static int multi_serial_pci_init(PCIDevice *dev)
         s->baudbase = 115200;
         serial_realize_core(s, &err);
         if (err != NULL) {
-            qerror_report_err(err);
-            error_free(err);
-            return -1;
+            error_propagate(errp, err);
+            return;
         }
         s->irq = pci->irqs[i];
         pci->name[i] = g_strdup_printf("uart #%d", i+1);
@@ -126,7 +123,6 @@ static int multi_serial_pci_init(PCIDevice *dev)
                               pci->name[i], 8);
         memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
     }
-    return 0;
 }
 
 static void serial_pci_exit(PCIDevice *dev)
@@ -203,7 +199,7 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
-    pc->init = serial_pci_init;
+    pc->realize = serial_pci_realize;
     pc->exit = serial_pci_exit;
     pc->vendor_id = PCI_VENDOR_ID_REDHAT;
     pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL;
@@ -218,7 +214,7 @@ static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
-    pc->init = multi_serial_pci_init;
+    pc->realize = multi_serial_pci_realize;
     pc->exit = multi_serial_pci_exit;
     pc->vendor_id = PCI_VENDOR_ID_REDHAT;
     pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL2;
@@ -233,7 +229,7 @@ static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
-    pc->init = multi_serial_pci_init;
+    pc->realize = multi_serial_pci_realize;
     pc->exit = multi_serial_pci_exit;
     pc->vendor_id = PCI_VENDOR_ID_REDHAT;
     pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL4;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 07/10] ide/ich: Convert to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (5 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 06/10] serial-pci: " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:44   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 08/10] cirrus-vga: " Markus Armbruster
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/ich.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index fb1d095..b1d8874 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -98,7 +98,7 @@ static void pci_ich9_reset(DeviceState *dev)
     ahci_reset(&d->ahci);
 }
 
-static int pci_ich9_ahci_init(PCIDevice *dev)
+static void pci_ich9_ahci_realize(PCIDevice *dev, Error **errp)
 {
     struct AHCIPCIState *d;
     int sata_cap_offset;
@@ -123,10 +123,11 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
     pci_register_bar(dev, ICH9_MEM_BAR, PCI_BASE_ADDRESS_SPACE_MEMORY,
                      &d->ahci.mem);
 
-    sata_cap_offset = pci_add_capability(dev, PCI_CAP_ID_SATA,
-                                         ICH9_SATA_CAP_OFFSET, SATA_CAP_SIZE);
+    sata_cap_offset = pci_add_capability2(dev, PCI_CAP_ID_SATA,
+                                          ICH9_SATA_CAP_OFFSET, SATA_CAP_SIZE,
+                                          errp);
     if (sata_cap_offset < 0) {
-        return sata_cap_offset;
+        return;
     }
 
     sata_cap = dev->config + sata_cap_offset;
@@ -139,8 +140,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
      * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9
      * AHCI device puts the MSI capability first, pointing to 0x80. */
     msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
-
-    return 0;
 }
 
 static void pci_ich9_uninit(PCIDevice *dev)
@@ -158,7 +157,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_ich9_ahci_init;
+    k->realize = pci_ich9_ahci_realize;
     k->exit = pci_ich9_uninit;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801IR;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 08/10] cirrus-vga: Convert to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (6 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 07/10] ide/ich: " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:44   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 09/10] qxl: " Markus Armbruster
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/display/cirrus_vga.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 8a5b76c..1ad1073 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2969,7 +2969,7 @@ static const TypeInfo isa_cirrus_vga_info = {
  *
  ***************************************/
 
-static int pci_cirrus_vga_initfn(PCIDevice *dev)
+static void pci_cirrus_vga_realize(PCIDevice *dev, Error **errp)
 {
      PCICirrusVGAState *d = DO_UPCAST(PCICirrusVGAState, dev, dev);
      CirrusVGAState *s = &d->cirrus_vga;
@@ -2980,9 +2980,9 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
        Also accept 8 MB/16 MB for backward compatibility. */
      if (s->vga.vram_size_mb != 4 && s->vga.vram_size_mb != 8 &&
          s->vga.vram_size_mb != 16) {
-         error_report("Invalid cirrus_vga ram size '%u'",
-                      s->vga.vram_size_mb);
-         return -1;
+         error_setg(errp, "Invalid cirrus_vga ram size '%u'",
+                    s->vga.vram_size_mb);
+         return;
      }
      /* setup VGA */
      vga_common_init(&s->vga, OBJECT(dev), true);
@@ -3007,7 +3007,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
      if (device_id == CIRRUS_ID_CLGD5446) {
          pci_register_bar(&d->dev, 1, 0, &s->cirrus_mmio_io);
      }
-     return 0;
 }
 
 static Property pci_vga_cirrus_properties[] = {
@@ -3021,7 +3020,7 @@ static void cirrus_vga_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_cirrus_vga_initfn;
+    k->realize = pci_cirrus_vga_realize;
     k->romfile = VGABIOS_CIRRUS_FILENAME;
     k->vendor_id = PCI_VENDOR_ID_CIRRUS;
     k->device_id = CIRRUS_ID_CLGD5446;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 09/10] qxl: Convert to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (7 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 08/10] cirrus-vga: " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:46   ` Gonglei
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 10/10] pci-assign: " Markus Armbruster
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/display/qxl.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b540dd6..090b66c 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1917,7 +1917,7 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl)
     qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1);
 }
 
-static int qxl_init_common(PCIQXLDevice *qxl)
+static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
 {
     uint8_t* config = qxl->pci.config;
     uint32_t pci_device_rev;
@@ -1949,9 +1949,9 @@ static int qxl_init_common(PCIQXLDevice *qxl)
         io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
         break;
     default:
-        error_report("Invalid revision %d for qxl device (max %d)",
-                     qxl->revision, QXL_DEFAULT_REVISION);
-        return -1;
+        error_setg(errp, "Invalid revision %d for qxl device (max %d)",
+                   qxl->revision, QXL_DEFAULT_REVISION);
+        return;
     }
 
     pci_set_byte(&config[PCI_REVISION_ID], pci_device_rev);
@@ -2015,9 +2015,9 @@ static int qxl_init_common(PCIQXLDevice *qxl)
 
     qxl->ssd.qxl.base.sif = &qxl_interface.base;
     if (qemu_spice_add_display_interface(&qxl->ssd.qxl, qxl->vga.con) != 0) {
-        error_report("qxl interface %d.%d not supported by spice-server",
-                     SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR);
-        return -1;
+        error_setg(errp, "qxl interface %d.%d not supported by spice-server",
+                   SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR);
+        return;
     }
     qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
@@ -2025,15 +2025,13 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     qxl_reset_state(qxl);
 
     qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
-
-    return 0;
 }
 
-static int qxl_init_primary(PCIDevice *dev)
+static void qxl_realize_primary(PCIDevice *dev, Error **errp)
 {
     PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
     VGACommonState *vga = &qxl->vga;
-    int rc;
+    Error *local_err = NULL;
 
     qxl->id = 0;
     qxl_init_ramsize(qxl);
@@ -2050,18 +2048,18 @@ static int qxl_init_primary(PCIDevice *dev)
     vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
     qemu_spice_display_init_common(&qxl->ssd);
 
-    rc = qxl_init_common(qxl);
-    if (rc != 0) {
-        return rc;
+    qxl_realize_common(qxl, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
     }
 
     qxl->ssd.dcl.ops = &display_listener_ops;
     qxl->ssd.dcl.con = vga->con;
     register_displaychangelistener(&qxl->ssd.dcl);
-    return rc;
 }
 
-static int qxl_init_secondary(PCIDevice *dev)
+static void qxl_realize_secondary(PCIDevice *dev, Error **errp)
 {
     static int device_id = 1;
     PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
@@ -2074,7 +2072,7 @@ static int qxl_init_secondary(PCIDevice *dev)
     qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram);
     qxl->vga.con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
 
-    return qxl_init_common(qxl);
+    qxl_realize_common(qxl, errp);
 }
 
 static void qxl_pre_save(void *opaque)
@@ -2287,7 +2285,7 @@ static void qxl_primary_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = qxl_init_primary;
+    k->realize = qxl_realize_primary;
     k->romfile = "vgabios-qxl.bin";
     k->vendor_id = REDHAT_PCI_VENDOR_ID;
     k->device_id = QXL_DEVICE_ID_STABLE;
@@ -2312,7 +2310,7 @@ static void qxl_secondary_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = qxl_init_secondary;
+    k->realize = qxl_realize_secondary;
     k->vendor_id = REDHAT_PCI_VENDOR_ID;
     k->device_id = QXL_DEVICE_ID_STABLE;
     k->class_id = PCI_CLASS_DISPLAY_OTHER;
-- 
1.9.3

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

* [Qemu-devel] [PATCH RFC 10/10] pci-assign: Convert to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (8 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 09/10] qxl: " Markus Armbruster
@ 2014-10-28  7:35 ` Markus Armbruster
  2014-10-28  8:49   ` Gonglei
  2014-10-28  8:27 ` [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion " Gonglei
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  7:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, mst

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/kvm/pci-assign.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index bb206da..5a92800 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1742,7 +1742,7 @@ static void reset_assigned_device(DeviceState *dev)
     assigned_dev_pci_write_config(pci_dev, PCI_COMMAND, 0, 1);
 }
 
-static int assigned_initfn(struct PCIDevice *pci_dev)
+static void assigned_realize(struct PCIDevice *pci_dev, Error **errp)
 {
     AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint8_t e_intx;
@@ -1825,7 +1825,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 
     assigned_dev_load_option_rom(dev);
 
-    return 0;
+    return;
 
 assigned_out:
     deassign_device(dev);
@@ -1835,9 +1835,7 @@ out:
 
 exit_with_error:
     assert(local_err);
-    qerror_report_err(local_err);
-    error_free(local_err);
-    return -1;
+    error_propagate(errp, local_err);
 }
 
 static void assigned_exitfn(struct PCIDevice *pci_dev)
@@ -1873,7 +1871,7 @@ static void assign_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init         = assigned_initfn;
+    k->realize      = assigned_realize;
     k->exit         = assigned_exitfn;
     k->config_read  = assigned_dev_pci_read_config;
     k->config_write = assigned_dev_pci_write_config;
-- 
1.9.3

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (9 preceding siblings ...)
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 10/10] pci-assign: " Markus Armbruster
@ 2014-10-28  8:27 ` Gonglei
  2014-10-30 14:01 ` Andreas Färber
  2014-11-02 11:20 ` Michael S. Tsirkin
  12 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:27 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
> to realize", Paolo called the PCI conversion job "Gargantuan".  This
> series attempts to crack it into manageable jobs.
> 
> The basic idea comes from qdev core: have the core deal with just
> realize, but default the device models' realize() method to one that
> calls the old init() method.  Unconverted device models don't set
> their realize(), thus get one that calls their init().  We can then
> convert device by device instead of having to convert of all of PCI in
> one Gargantuan go.
> 

Good solution I think :)

Best regards,
-Gonglei

> Since PCI's exit() cannot fail, I chose not to add an unrealize().
> Precedence: USBDeviceClass method handle_destroy(), called on USB
> unrealize.
> 
> Aside: USBDeviceClass also has an unrealize() method, but it's never
> set and never called.
> 
> PATCH 01 converts the interface between PCI core and qdev to realize.
> 
> PATCH 02 adds realize to the interface between PCI core and PCI device
> models.  Once all device models are converted to realize, the old init
> interface can be dropped, completing the Gargantuan job.
> 
> PATCH 03-04 convert device models that cannot fail initialization.
> 
> PATCH 05-10 convert a few that can fail, but are really easy to
> convert.
> 
> This series is RFC because it's based on Marcel's "[PATCH v2] hw/pci:
> fixed crash when using rombar=0 with romfile=path for hotplugged
> devices", which is still undergoing review.
> 
> Markus Armbruster (10):
>   pci: Convert core to realize
>   pci: Permit incremental conversion of device models to realize
>   pci: Trivial device model conversions to realize
>   pcnet: pcnet_common_init() always returns 0, change to void
>   pcnet: Convert to realize
>   serial-pci: Convert to realize
>   ide/ich: Convert to realize
>   cirrus-vga: Convert to realize
>   qxl: Convert to realize
>   pci-assign: Convert to realize
> 
>  hw/acpi/piix4.c            |   5 +-
>  hw/audio/ac97.c            |   5 +-
>  hw/audio/es1370.c          |   5 +-
>  hw/audio/intel-hda.c       |   6 +--
>  hw/char/serial-pci.c       |  22 ++++-----
>  hw/display/cirrus_vga.c    |  11 ++---
>  hw/display/qxl.c           |  36 +++++++--------
>  hw/display/vga-pci.c       |  11 ++---
>  hw/display/vmware_vga.c    |   6 +--
>  hw/i2c/smbus_ich9.c        |   5 +-
>  hw/i386/kvm/pci-assign.c   |  10 ++--
>  hw/ide/cmd646.c            |   5 +-
>  hw/ide/ich.c               |  13 +++---
>  hw/ide/piix.c              |  10 ++--
>  hw/ide/via.c               |   6 +--
>  hw/ipack/tpci200.c         |   6 +--
>  hw/isa/i82378.c            |   6 +--
>  hw/isa/piix4.c             |   5 +-
>  hw/isa/vt82c686.c          |  24 ++++------
>  hw/misc/pci-testdev.c      |   6 +--
>  hw/net/e1000.c             |   6 +--
>  hw/net/eepro100.c          |   6 +--
>  hw/net/lance.c             |   3 +-
>  hw/net/ne2000.c            |   6 +--
>  hw/net/pcnet-pci.c         |   6 +--
>  hw/net/pcnet.c             |   4 +-
>  hw/net/pcnet.h             |   2 +-
>  hw/net/rtl8139.c           |   6 +--
>  hw/net/vmxnet3.c           |   6 +--
>  hw/pci-bridge/dec.c        |   5 +-
>  hw/pci-host/apb.c          |   5 +-
>  hw/pci-host/bonito.c       |   6 +--
>  hw/pci-host/grackle.c      |   5 +-
>  hw/pci-host/piix.c         |  12 ++---
>  hw/pci-host/ppce500.c      |   6 +--
>  hw/pci-host/prep.c         |   6 +--
>  hw/pci-host/q35.c          |   5 +-
>  hw/pci-host/uninorth.c     |  20 ++++----
>  hw/pci-host/versatile.c    |   5 +-
>  hw/pci/pci.c               | 111 ++++++++++++++++++++++++++-------------------
>  hw/usb/hcd-ehci-pci.c      |   6 +--
>  hw/usb/hcd-xhci.c          |   6 +--
>  hw/watchdog/wdt_i6300esb.c |   6 +--
>  include/hw/pci/pci.h       |   3 +-
>  44 files changed, 199 insertions(+), 256 deletions(-)
> 

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

* Re: [Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 01/10] pci: Convert core " Markus Armbruster
@ 2014-10-28  8:32   ` Gonglei
  2014-10-28  9:38     ` Markus Armbruster
  2014-10-30 17:02   ` Marcel Apfelbaum
  1 sibling, 1 reply; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:32 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Implement DeviceClass methods realize() and unrealize() instead of
> init() and exit().  The core's initialization errors now get
> propagated properly, and QMP sends them instead of an unspecific
> "Device initialization failed" error.  Unrealize can't fail, so no
> change there.
> 
> PCIDeviceClass is unchanged: it still provides init() and exit().
> Therefore, device models' errors are still not propagated.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/pci/pci.c | 91 +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 47 insertions(+), 44 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index cd7a403..aef95c3 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -114,7 +114,7 @@ static const TypeInfo pcie_bus_info = {
>  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_irq_handler(void *opaque, int irq_num, int level);
> -static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> +static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
>  static void pci_del_option_rom(PCIDevice *pdev);
>  
>  static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
> @@ -725,7 +725,7 @@ static void pci_init_mask_bridge(PCIDevice *d)
>                                 PCI_PREF_RANGE_TYPE_MASK);
>  }
>  
> -static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev, Error **errp)
>  {
>      uint8_t slot = PCI_SLOT(dev->devfn);
>      uint8_t func;
> @@ -751,26 +751,25 @@ static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
>          PCIDevice *f0 = bus->devices[PCI_DEVFN(slot, 0)];
>          if (f0 && !(f0->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)) {
>              /* function 0 should set multifunction bit */
> -            error_report("PCI: single function device can't be populated "
> -                         "in function %x.%x", slot, PCI_FUNC(dev->devfn));
> -            return -1;
> +            error_setg(errp, "PCI: single function device can't be populated "
> +                       "in function %x.%x", slot, PCI_FUNC(dev->devfn));
> +            return;
>          }
> -        return 0;
> +        return;
>      }
>  
>      if (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> -        return 0;
> +        return;
>      }
>      /* function 0 indicates single function, so function > 0 must be NULL */
>      for (func = 1; func < PCI_FUNC_MAX; ++func) {
>          if (bus->devices[PCI_DEVFN(slot, func)]) {
> -            error_report("PCI: %x.0 indicates single function, "
> -                         "but %x.%x is already populated.",
> -                         slot, slot, func);
> -            return -1;
> +            error_setg(errp, "PCI: %x.0 indicates single function, "
> +                       "but %x.%x is already populated.",
> +                       slot, slot, func);
> +            return;
>          }
>      }
> -    return 0;
>  }
>  
>  static void pci_config_alloc(PCIDevice *pci_dev)
> @@ -803,11 +802,13 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
>  
>  /* -1 for devfn means auto assign */
>  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> -                                         const char *name, int devfn)
> +                                         const char *name, int devfn,
> +                                         Error **errp)
>  {
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
>      PCIConfigReadFunc *config_read = pc->config_read;
>      PCIConfigWriteFunc *config_write = pc->config_write;
> +    Error *local_err = NULL;
>      AddressSpace *dma_as;
>  
>      if (devfn < 0) {
> @@ -816,12 +817,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>              if (!bus->devices[devfn])
>                  goto found;
>          }
> -        error_report("PCI: no slot/function available for %s, all in use", name);
> +        error_setg(errp, "PCI: no slot/function available for %s, all in use",
> +                   name);
>          return NULL;
>      found: ;
>      } else if (bus->devices[devfn]) {
> -        error_report("PCI: slot %d function %d not available for %s, in use by %s",
> -                     PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
> +        error_setg(errp, "PCI: slot %d function %d not available for %s,"
> +                   " in use by %s",
> +                   PCI_SLOT(devfn), PCI_FUNC(devfn), name,
> +                   bus->devices[devfn]->name);
>          return NULL;
>      }
>  
> @@ -865,7 +869,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      if (pc->is_bridge) {
>          pci_init_mask_bridge(pci_dev);
>      }
> -    if (pci_init_multifunction(bus, pci_dev)) {
> +    pci_init_multifunction(bus, pci_dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
>          do_pci_unregister_device(pci_dev);
>          return NULL;
>      }
> @@ -896,7 +902,7 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
>      pci_unregister_vga(pci_dev);
>  }
>  
> -static int pci_unregister_device(DeviceState *dev)
> +static void pci_qdev_unrealize(DeviceState *dev, Error **errp)
>  {
>      PCIDevice *pci_dev = PCI_DEVICE(dev);
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> @@ -909,7 +915,6 @@ static int pci_unregister_device(DeviceState *dev)
>      }
>  
>      do_pci_unregister_device(pci_dev);
> -    return 0;
>  }
>  
>  void pci_register_bar(PCIDevice *pci_dev, int region_num,
> @@ -1742,10 +1747,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
>      return bus->devices[devfn];
>  }
>  
> -static int pci_qdev_init(DeviceState *qdev)
> +static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>  {
>      PCIDevice *pci_dev = (PCIDevice *)qdev;
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> +    Error *local_err = NULL;
>      PCIBus *bus;
>      int rc;
>      bool is_default_rom;
> @@ -1758,15 +1764,16 @@ static int pci_qdev_init(DeviceState *qdev)
>      bus = PCI_BUS(qdev_get_parent_bus(qdev));
>      pci_dev = do_pci_register_device(pci_dev, bus,
>                                       object_get_typename(OBJECT(qdev)),
> -                                     pci_dev->devfn);
> +                                     pci_dev->devfn, errp);
>      if (pci_dev == NULL)

> -        return -1;
> +        return;
>  

Maybe we can use '{}' for if statement follow Qemu's coding style.

>      if (pc->init) {
>          rc = pc->init(pci_dev);
>          if (rc != 0) {
>              do_pci_unregister_device(pci_dev);
> -            return rc;
> +            error_setg(errp, "Device initialization failed");
> +            return;
>          }
>      }
>  
> @@ -1777,13 +1784,12 @@ static int pci_qdev_init(DeviceState *qdev)
>          is_default_rom = true;
>      }
>  
> -    rc = pci_add_option_rom(pci_dev, is_default_rom);
> -    if (rc != 0) {
> -        pci_unregister_device(DEVICE(pci_dev));
> -        return rc;
> +    pci_add_option_rom(pci_dev, is_default_rom, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        pci_qdev_unrealize(DEVICE(pci_dev), NULL);
> +        return;
>      }
> -
> -    return 0;
>  }
>  
>  PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> @@ -1923,7 +1929,8 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
>  }
>  
>  /* Add an option rom for the device */
> -static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
> +static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
> +                               Error **errp)
>  {
>      int size;
>      char *path;
> @@ -1932,9 +1939,9 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>      const VMStateDescription *vmsd;
>  
>      if (!pdev->romfile)
> -        return 0;
> +        return;
>      if (strlen(pdev->romfile) == 0)
> -        return 0;
> +        return;
>  

The above both too.

>      if (!pdev->rom_bar) {
>          /*
> @@ -1948,7 +1955,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>           * if the rom bar is disabled.
>           */
>          if (DEVICE(pdev)->hotplugged) {
> -            return 0;
> +            return;
>          }
>  
>          if (class == 0x0300) {
> @@ -1956,7 +1963,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>          } else {
>              rom_add_option(pdev->romfile, -1);
>          }
> -        return 0;
> +        return;
>      }
>  
>      path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile);
> @@ -1966,15 +1973,13 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>  
>      size = get_image_size(path);
>      if (size < 0) {
> -        error_report("%s: failed to find romfile \"%s\"",
> -                     __func__, pdev->romfile);
> +        error_setg(errp, "failed to find romfile \"%s\"", pdev->romfile);
>          g_free(path);
> -        return -1;
> +        return;
>      } else if (size == 0) {
> -        error_report("%s: ignoring empty romfile \"%s\"",
> -                     __func__, pdev->romfile);
> +        error_setg(errp, "romfile \"%s\" is empty", pdev->romfile);
>          g_free(path);
> -        return -1;
> +        return;
>      }
>      if (size & (size - 1)) {
>          size = 1 << qemu_fls(size);
> @@ -2000,8 +2005,6 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>      }
>  
>      pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
> -
> -    return 0;
>  }
>  
>  static void pci_del_option_rom(PCIDevice *pdev)
> @@ -2283,8 +2286,8 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
>  static void pci_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *k = DEVICE_CLASS(klass);
> -    k->init = pci_qdev_init;
> -    k->exit = pci_unregister_device;
> +    k->realize = pci_qdev_realize;
> +    k->unrealize = pci_qdev_unrealize;
>      k->bus_type = TYPE_PCI_BUS;
>      k->props = pci_props;
>  }

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

* Re: [Qemu-devel] [PATCH RFC 02/10] pci: Permit incremental conversion of device models to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 02/10] pci: Permit incremental conversion of device models " Markus Armbruster
@ 2014-10-28  8:35   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:35 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Call the new PCIDeviceClass method realize().  Default it to
> pci_default_realize(), which calls old method init().
> 
> To convert a device model, make it implement realize() rather than
> init().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/pci/pci.c         | 24 +++++++++++++++++++-----
>  include/hw/pci/pci.h |  3 ++-
>  2 files changed, 21 insertions(+), 6 deletions(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index aef95c3..e21c0a8 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1753,7 +1753,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
>      Error *local_err = NULL;
>      PCIBus *bus;
> -    int rc;
>      bool is_default_rom;
>  
>      /* initialize cap_present for pci_is_express() and pci_config_size() */
> @@ -1768,11 +1767,11 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>      if (pci_dev == NULL)
>          return;
>  
> -    if (pc->init) {
> -        rc = pc->init(pci_dev);
> -        if (rc != 0) {
> +    if (pc->realize) {
> +        pc->realize(pci_dev, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
>              do_pci_unregister_device(pci_dev);
> -            error_setg(errp, "Device initialization failed");
>              return;
>          }
>      }
> @@ -1792,6 +1791,18 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>      }
>  }
>  
> +static void pci_default_realize(PCIDevice *dev, Error **errp)
> +{
> +    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +
> +    if (pc->init) {
> +        if (pc->init(dev) < 0) {
> +            error_setg(errp, "Device initialization failed");
> +            return;
> +        }
> +    }
> +}
> +
>  PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
>                                      const char *name)
>  {
> @@ -2286,10 +2297,13 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
>  static void pci_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *k = DEVICE_CLASS(klass);
> +    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
> +
>      k->realize = pci_qdev_realize;
>      k->unrealize = pci_qdev_unrealize;
>      k->bus_type = TYPE_PCI_BUS;
>      k->props = pci_props;
> +    pc->realize = pci_default_realize;
>  }
>  
>  AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index c352c7b..83597b3 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -183,7 +183,8 @@ typedef struct PCIINTxRoute {
>  typedef struct PCIDeviceClass {
>      DeviceClass parent_class;
>  
> -    int (*init)(PCIDevice *dev);
> +    void (*realize)(PCIDevice *dev, Error **errp);
> +    int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
>      PCIUnregisterFunc *exit;
>      PCIConfigReadFunc *config_read;
>      PCIConfigWriteFunc *config_write;

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

* Re: [Qemu-devel] [PATCH RFC 03/10] pci: Trivial device model conversions to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 03/10] pci: Trivial device model conversions " Markus Armbruster
@ 2014-10-28  8:38   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:38 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Convert the device models where initialization obviously can't fail.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/acpi/piix4.c            |  5 ++---
>  hw/audio/ac97.c            |  5 ++---
>  hw/audio/es1370.c          |  5 ++---
>  hw/audio/intel-hda.c       |  6 ++----
>  hw/display/vga-pci.c       | 11 ++++-------
>  hw/display/vmware_vga.c    |  6 ++----
>  hw/i2c/smbus_ich9.c        |  5 ++---
>  hw/ide/cmd646.c            |  5 ++---
>  hw/ide/piix.c              | 10 ++++------
>  hw/ide/via.c               |  6 ++----
>  hw/ipack/tpci200.c         |  6 ++----
>  hw/isa/i82378.c            |  6 ++----
>  hw/isa/piix4.c             |  5 ++---
>  hw/isa/vt82c686.c          | 24 ++++++++----------------
>  hw/misc/pci-testdev.c      |  6 ++----
>  hw/net/e1000.c             |  6 ++----
>  hw/net/eepro100.c          |  6 ++----
>  hw/net/ne2000.c            |  6 ++----
>  hw/net/rtl8139.c           |  6 ++----
>  hw/net/vmxnet3.c           |  6 ++----
>  hw/pci-bridge/dec.c        |  5 ++---
>  hw/pci-host/apb.c          |  5 ++---
>  hw/pci-host/bonito.c       |  6 ++----
>  hw/pci-host/grackle.c      |  5 ++---
>  hw/pci-host/piix.c         | 12 +++++-------
>  hw/pci-host/ppce500.c      |  6 ++----
>  hw/pci-host/prep.c         |  6 ++----
>  hw/pci-host/q35.c          |  5 ++---
>  hw/pci-host/uninorth.c     | 20 ++++++++------------
>  hw/pci-host/versatile.c    |  5 ++---
>  hw/usb/hcd-ehci-pci.c      |  6 ++----
>  hw/usb/hcd-xhci.c          |  6 ++----
>  hw/watchdog/wdt_i6300esb.c |  6 ++----
>  33 files changed, 85 insertions(+), 149 deletions(-)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void Markus Armbruster
@ 2014-10-28  8:42   ` Gonglei
  2014-10-28  9:41     ` Markus Armbruster
  0 siblings, 1 reply; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:42 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> The next commit will exploit the fact it never fails.  This one makes
> it obvious.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/net/lance.c     | 3 ++-
>  hw/net/pcnet-pci.c | 3 ++-
>  hw/net/pcnet.c     | 4 +---
>  hw/net/pcnet.h     | 2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/net/lance.c b/hw/net/lance.c
> index a1c49f1..3663340 100644
> --- a/hw/net/lance.c
> +++ b/hw/net/lance.c
> @@ -134,7 +134,8 @@ static int lance_init(SysBusDevice *sbd)
>  
>      s->phys_mem_read = ledma_memory_read;
>      s->phys_mem_write = ledma_memory_write;
> -    return pcnet_common_init(dev, s, &net_lance_info);
> +    pcnet_common_init(dev, s, &net_lance_info);
> +    return 0;
>  }
>  
>  static void lance_reset(DeviceState *dev)
> diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
> index fb5f5d6..50eb069 100644
> --- a/hw/net/pcnet-pci.c
> +++ b/hw/net/pcnet-pci.c
> @@ -335,7 +335,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
>      s->phys_mem_write = pci_physical_memory_write;
>      s->dma_opaque = pci_dev;
>  
> -    return pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
> +    pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
> +    return 0;
>  }
>  
>  static void pci_reset(DeviceState *dev)
> diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
> index d344c15..5a081c4 100644
> --- a/hw/net/pcnet.c
> +++ b/hw/net/pcnet.c
> @@ -1724,7 +1724,7 @@ void pcnet_common_cleanup(PCNetState *d)
>      d->nic = NULL;
>  }
>  
> -int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
> +void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)

Do we need consider to pass an Error **errp argument to it?

Best regards,
-Gonglei

>  {
>      int i;
>      uint16_t checksum;
> @@ -1763,6 +1763,4 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
>      *(uint16_t *)&s->prom[12] = cpu_to_le16(checksum);
>  
>      s->lnkst = 0x40; /* initial link state: up */
> -
> -    return 0;
>  }
> diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
> index f8e8a6f..cfc196e 100644
> --- a/hw/net/pcnet.h
> +++ b/hw/net/pcnet.h
> @@ -64,6 +64,6 @@ int pcnet_can_receive(NetClientState *nc);
>  ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_);
>  void pcnet_set_link_status(NetClientState *nc);
>  void pcnet_common_cleanup(PCNetState *d);
> -int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
> +void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
>  extern const VMStateDescription vmstate_pcnet;
>  #endif

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

* Re: [Qemu-devel] [PATCH RFC 06/10] serial-pci: Convert to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 06/10] serial-pci: " Markus Armbruster
@ 2014-10-28  8:43   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:43 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/char/serial-pci.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 07/10] ide/ich: Convert to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 07/10] ide/ich: " Markus Armbruster
@ 2014-10-28  8:44   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:44 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/ide/ich.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 08/10] cirrus-vga: Convert to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 08/10] cirrus-vga: " Markus Armbruster
@ 2014-10-28  8:44   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:44 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/display/cirrus_vga.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 09/10] qxl: Convert to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 09/10] qxl: " Markus Armbruster
@ 2014-10-28  8:46   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:46 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/display/qxl.c | 36 +++++++++++++++++-------------------
>  1 file changed, 17 insertions(+), 19 deletions(-)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 10/10] pci-assign: Convert to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 10/10] pci-assign: " Markus Armbruster
@ 2014-10-28  8:49   ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28  8:49 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, kraxel, qemu-devel, afaerber, mst

On 2014/10/28 15:35, Markus Armbruster wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/kvm/pci-assign.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize
  2014-10-28  8:32   ` Gonglei
@ 2014-10-28  9:38     ` Markus Armbruster
  2014-10-30 16:58       ` Marcel Apfelbaum
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  9:38 UTC (permalink / raw)
  To: Gonglei; +Cc: pbonzini, mst, kraxel, afaerber, qemu-devel

Gonglei <arei.gonglei@huawei.com> writes:

> On 2014/10/28 15:35, Markus Armbruster wrote:
>
>> Implement DeviceClass methods realize() and unrealize() instead of
>> init() and exit().  The core's initialization errors now get
>> propagated properly, and QMP sends them instead of an unspecific
>> "Device initialization failed" error.  Unrealize can't fail, so no
>> change there.
>> 
>> PCIDeviceClass is unchanged: it still provides init() and exit().
>> Therefore, device models' errors are still not propagated.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/pci/pci.c | 91 +++++++++++++++++++++++++++++++-----------------------------
>>  1 file changed, 47 insertions(+), 44 deletions(-)
>> 
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index cd7a403..aef95c3 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
[...]
>>  void pci_register_bar(PCIDevice *pci_dev, int region_num,
>> @@ -1742,10 +1747,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
>>      return bus->devices[devfn];
>>  }
>>  
>> -static int pci_qdev_init(DeviceState *qdev)
>> +static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>>  {
>>      PCIDevice *pci_dev = (PCIDevice *)qdev;
>>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
>> +    Error *local_err = NULL;
>>      PCIBus *bus;
>>      int rc;
>>      bool is_default_rom;
>> @@ -1758,15 +1764,16 @@ static int pci_qdev_init(DeviceState *qdev)
>>      bus = PCI_BUS(qdev_get_parent_bus(qdev));
>>      pci_dev = do_pci_register_device(pci_dev, bus,
>>                                       object_get_typename(OBJECT(qdev)),
>> -                                     pci_dev->devfn);
>> +                                     pci_dev->devfn, errp);
>>      if (pci_dev == NULL)
>
>> -        return -1;
>> +        return;
>>  
>
> Maybe we can use '{}' for if statement follow Qemu's coding style.

scripts/checkpatch.pl is happy with the patch as is.

I prefer to add braces only when I touch the conditional.

Naturally, I also add them when a maintainer asks me to :)

>>      if (pc->init) {
>>          rc = pc->init(pci_dev);
>>          if (rc != 0) {
>>              do_pci_unregister_device(pci_dev);
>> -            return rc;
>> +            error_setg(errp, "Device initialization failed");
>> +            return;
>>          }
>>      }
>>  
[...]

Thanks!

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

* Re: [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void
  2014-10-28  8:42   ` Gonglei
@ 2014-10-28  9:41     ` Markus Armbruster
  2014-10-28 10:35       ` Gonglei
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-10-28  9:41 UTC (permalink / raw)
  To: Gonglei; +Cc: pbonzini, mst, kraxel, afaerber, qemu-devel

Gonglei <arei.gonglei@huawei.com> writes:

> On 2014/10/28 15:35, Markus Armbruster wrote:
>
>> The next commit will exploit the fact it never fails.  This one makes
>> it obvious.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/net/lance.c     | 3 ++-
>>  hw/net/pcnet-pci.c | 3 ++-
>>  hw/net/pcnet.c     | 4 +---
>>  hw/net/pcnet.h     | 2 +-
>>  4 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/net/lance.c b/hw/net/lance.c
>> index a1c49f1..3663340 100644
>> --- a/hw/net/lance.c
>> +++ b/hw/net/lance.c
>> @@ -134,7 +134,8 @@ static int lance_init(SysBusDevice *sbd)
>>  
>>      s->phys_mem_read = ledma_memory_read;
>>      s->phys_mem_write = ledma_memory_write;
>> -    return pcnet_common_init(dev, s, &net_lance_info);
>> +    pcnet_common_init(dev, s, &net_lance_info);
>> +    return 0;
>>  }
>>  
>>  static void lance_reset(DeviceState *dev)
>> diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
>> index fb5f5d6..50eb069 100644
>> --- a/hw/net/pcnet-pci.c
>> +++ b/hw/net/pcnet-pci.c
>> @@ -335,7 +335,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
>>      s->phys_mem_write = pci_physical_memory_write;
>>      s->dma_opaque = pci_dev;
>>  
>> -    return pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>> +    pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>> +    return 0;
>>  }
>>  
>>  static void pci_reset(DeviceState *dev)
>> diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
>> index d344c15..5a081c4 100644
>> --- a/hw/net/pcnet.c
>> +++ b/hw/net/pcnet.c
>> @@ -1724,7 +1724,7 @@ void pcnet_common_cleanup(PCNetState *d)
>>      d->nic = NULL;
>>  }
>>  
>> -int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
>> +void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
>
> Do we need consider to pass an Error **errp argument to it?

This function can't fail.  The point of thimy patch is to make "can't
fail" obvious.  If we add an errp parameter, the caller needs to check
it, for robustness.  I prefer to keep things simple, and add the error
checking only when it's actually needed.

> Best regards,
> -Gonglei

Thanks!

[...]

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

* Re: [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void
  2014-10-28  9:41     ` Markus Armbruster
@ 2014-10-28 10:35       ` Gonglei
  0 siblings, 0 replies; 32+ messages in thread
From: Gonglei @ 2014-10-28 10:35 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, mst, kraxel, afaerber, qemu-devel

On 2014/10/28 17:41, Markus Armbruster wrote:

> Gonglei <arei.gonglei@huawei.com> writes:
> 
>> On 2014/10/28 15:35, Markus Armbruster wrote:
>>
>>> The next commit will exploit the fact it never fails.  This one makes
>>> it obvious.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  hw/net/lance.c     | 3 ++-
>>>  hw/net/pcnet-pci.c | 3 ++-
>>>  hw/net/pcnet.c     | 4 +---
>>>  hw/net/pcnet.h     | 2 +-
>>>  4 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/net/lance.c b/hw/net/lance.c
>>> index a1c49f1..3663340 100644
>>> --- a/hw/net/lance.c
>>> +++ b/hw/net/lance.c
>>> @@ -134,7 +134,8 @@ static int lance_init(SysBusDevice *sbd)
>>>  
>>>      s->phys_mem_read = ledma_memory_read;
>>>      s->phys_mem_write = ledma_memory_write;
>>> -    return pcnet_common_init(dev, s, &net_lance_info);
>>> +    pcnet_common_init(dev, s, &net_lance_info);
>>> +    return 0;
>>>  }
>>>  
>>>  static void lance_reset(DeviceState *dev)
>>> diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
>>> index fb5f5d6..50eb069 100644
>>> --- a/hw/net/pcnet-pci.c
>>> +++ b/hw/net/pcnet-pci.c
>>> @@ -335,7 +335,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
>>>      s->phys_mem_write = pci_physical_memory_write;
>>>      s->dma_opaque = pci_dev;
>>>  
>>> -    return pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>>> +    pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>>> +    return 0;
>>>  }
>>>  
>>>  static void pci_reset(DeviceState *dev)
>>> diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
>>> index d344c15..5a081c4 100644
>>> --- a/hw/net/pcnet.c
>>> +++ b/hw/net/pcnet.c
>>> @@ -1724,7 +1724,7 @@ void pcnet_common_cleanup(PCNetState *d)
>>>      d->nic = NULL;
>>>  }
>>>  
>>> -int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
>>> +void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
>>
>> Do we need consider to pass an Error **errp argument to it?
> 
> This function can't fail.  The point of thimy patch is to make "can't
> fail" obvious.  If we add an errp parameter, the caller needs to check
> it, for robustness. 

Yes, it is.

> I prefer to keep things simple, and add the error
> checking only when it's actually needed.
> 

OK, it's fine :)

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (10 preceding siblings ...)
  2014-10-28  8:27 ` [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion " Gonglei
@ 2014-10-30 14:01 ` Andreas Färber
  2014-11-03  7:40   ` Markus Armbruster
  2014-11-02 11:20 ` Michael S. Tsirkin
  12 siblings, 1 reply; 32+ messages in thread
From: Andreas Färber @ 2014-10-30 14:01 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: pbonzini, arei.gonglei, kraxel, mst

Hi Markus,

Am 28.10.2014 um 08:35 schrieb Markus Armbruster:
> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
> to realize", Paolo called the PCI conversion job "Gargantuan".  This
> series attempts to crack it into manageable jobs.

Thanks for giving this a stab! What kept me from diving into the PCI
converstion was that I first invested into qtests for the non-default
PCI devices. How many of the converted devices are actually covered in
qtest?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize
  2014-10-28  9:38     ` Markus Armbruster
@ 2014-10-30 16:58       ` Marcel Apfelbaum
  0 siblings, 0 replies; 32+ messages in thread
From: Marcel Apfelbaum @ 2014-10-30 16:58 UTC (permalink / raw)
  To: Gonglei, Markus Armbruster; +Cc: qemu-devel, pbonzini, kraxel, afaerber, mst

On Tue, 2014-10-28 at 10:38 +0100, Markus Armbruster wrote:
> Gonglei <arei.gonglei@huawei.com> writes:
> 
> > On 2014/10/28 15:35, Markus Armbruster wrote:
> >
> >> Implement DeviceClass methods realize() and unrealize() instead of
> >> init() and exit().  The core's initialization errors now get
> >> propagated properly, and QMP sends them instead of an unspecific
> >> "Device initialization failed" error.  Unrealize can't fail, so no
> >> change there.
> >> 
> >> PCIDeviceClass is unchanged: it still provides init() and exit().
> >> Therefore, device models' errors are still not propagated.
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  hw/pci/pci.c | 91 +++++++++++++++++++++++++++++++-----------------------------
> >>  1 file changed, 47 insertions(+), 44 deletions(-)
> >> 
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index cd7a403..aef95c3 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> [...]
> >>  void pci_register_bar(PCIDevice *pci_dev, int region_num,
> >> @@ -1742,10 +1747,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
> >>      return bus->devices[devfn];
> >>  }
> >>  
> >> -static int pci_qdev_init(DeviceState *qdev)
> >> +static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> >>  {
> >>      PCIDevice *pci_dev = (PCIDevice *)qdev;
> >>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> >> +    Error *local_err = NULL;
> >>      PCIBus *bus;
> >>      int rc;
> >>      bool is_default_rom;
> >> @@ -1758,15 +1764,16 @@ static int pci_qdev_init(DeviceState *qdev)
> >>      bus = PCI_BUS(qdev_get_parent_bus(qdev));
> >>      pci_dev = do_pci_register_device(pci_dev, bus,
> >>                                       object_get_typename(OBJECT(qdev)),
> >> -                                     pci_dev->devfn);
> >> +                                     pci_dev->devfn, errp);
> >>      if (pci_dev == NULL)
> >
> >> -        return -1;
> >> +        return;
> >>  
> >
> > Maybe we can use '{}' for if statement follow Qemu's coding style.
I suggest adding a trivial patch on top of Markus's series

Thanks,
Marcel

> 
> scripts/checkpatch.pl is happy with the patch as is.
> 
> I prefer to add braces only when I touch the conditional.
> 
> Naturally, I also add them when a maintainer asks me to :)
> 
> >>      if (pc->init) {
> >>          rc = pc->init(pci_dev);
> >>          if (rc != 0) {
> >>              do_pci_unregister_device(pci_dev);
> >> -            return rc;
> >> +            error_setg(errp, "Device initialization failed");
> >> +            return;
> >>          }
> >>      }
> >>  
> [...]
> 
> Thanks!
> 

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

* Re: [Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize
  2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 01/10] pci: Convert core " Markus Armbruster
  2014-10-28  8:32   ` Gonglei
@ 2014-10-30 17:02   ` Marcel Apfelbaum
  1 sibling, 0 replies; 32+ messages in thread
From: Marcel Apfelbaum @ 2014-10-30 17:02 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: mst, qemu-devel, arei.gonglei, kraxel, pbonzini, afaerber

On Tue, 2014-10-28 at 08:35 +0100, Markus Armbruster wrote:
> Implement DeviceClass methods realize() and unrealize() instead of
> init() and exit().  The core's initialization errors now get
> propagated properly, and QMP sends them instead of an unspecific
> "Device initialization failed" error.  Unrealize can't fail, so no
> change there.
> 
> PCIDeviceClass is unchanged: it still provides init() and exit().
> Therefore, device models' errors are still not propagated.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/pci/pci.c | 91 +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 47 insertions(+), 44 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index cd7a403..aef95c3 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -114,7 +114,7 @@ static const TypeInfo pcie_bus_info = {
>  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_irq_handler(void *opaque, int irq_num, int level);
> -static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> +static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
>  static void pci_del_option_rom(PCIDevice *pdev);
>  
>  static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
> @@ -725,7 +725,7 @@ static void pci_init_mask_bridge(PCIDevice *d)
>                                 PCI_PREF_RANGE_TYPE_MASK);
>  }
>  
> -static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
> +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev, Error **errp)
>  {
>      uint8_t slot = PCI_SLOT(dev->devfn);
>      uint8_t func;
> @@ -751,26 +751,25 @@ static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
>          PCIDevice *f0 = bus->devices[PCI_DEVFN(slot, 0)];
>          if (f0 && !(f0->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)) {
>              /* function 0 should set multifunction bit */
> -            error_report("PCI: single function device can't be populated "
> -                         "in function %x.%x", slot, PCI_FUNC(dev->devfn));
> -            return -1;
> +            error_setg(errp, "PCI: single function device can't be populated "
> +                       "in function %x.%x", slot, PCI_FUNC(dev->devfn));
> +            return;
>          }
> -        return 0;
> +        return;
>      }
>  
>      if (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> -        return 0;
> +        return;
>      }
>      /* function 0 indicates single function, so function > 0 must be NULL */
>      for (func = 1; func < PCI_FUNC_MAX; ++func) {
>          if (bus->devices[PCI_DEVFN(slot, func)]) {
> -            error_report("PCI: %x.0 indicates single function, "
> -                         "but %x.%x is already populated.",
> -                         slot, slot, func);
> -            return -1;
> +            error_setg(errp, "PCI: %x.0 indicates single function, "
> +                       "but %x.%x is already populated.",
> +                       slot, slot, func);
> +            return;
>          }
>      }
> -    return 0;
>  }
>  
>  static void pci_config_alloc(PCIDevice *pci_dev)
> @@ -803,11 +802,13 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
>  
>  /* -1 for devfn means auto assign */
>  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> -                                         const char *name, int devfn)
> +                                         const char *name, int devfn,
> +                                         Error **errp)
>  {
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
>      PCIConfigReadFunc *config_read = pc->config_read;
>      PCIConfigWriteFunc *config_write = pc->config_write;
> +    Error *local_err = NULL;
>      AddressSpace *dma_as;
>  
>      if (devfn < 0) {
> @@ -816,12 +817,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>              if (!bus->devices[devfn])
>                  goto found;
>          }
> -        error_report("PCI: no slot/function available for %s, all in use", name);
> +        error_setg(errp, "PCI: no slot/function available for %s, all in use",
> +                   name);
>          return NULL;
>      found: ;
>      } else if (bus->devices[devfn]) {
> -        error_report("PCI: slot %d function %d not available for %s, in use by %s",
> -                     PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name);
> +        error_setg(errp, "PCI: slot %d function %d not available for %s,"
> +                   " in use by %s",
> +                   PCI_SLOT(devfn), PCI_FUNC(devfn), name,
> +                   bus->devices[devfn]->name);
>          return NULL;
>      }
>  
> @@ -865,7 +869,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      if (pc->is_bridge) {
>          pci_init_mask_bridge(pci_dev);
>      }
> -    if (pci_init_multifunction(bus, pci_dev)) {
> +    pci_init_multifunction(bus, pci_dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
>          do_pci_unregister_device(pci_dev);
>          return NULL;
>      }
> @@ -896,7 +902,7 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
>      pci_unregister_vga(pci_dev);
>  }
>  
> -static int pci_unregister_device(DeviceState *dev)
> +static void pci_qdev_unrealize(DeviceState *dev, Error **errp)
>  {
>      PCIDevice *pci_dev = PCI_DEVICE(dev);
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> @@ -909,7 +915,6 @@ static int pci_unregister_device(DeviceState *dev)
>      }
>  
>      do_pci_unregister_device(pci_dev);
> -    return 0;
>  }
>  
>  void pci_register_bar(PCIDevice *pci_dev, int region_num,
> @@ -1742,10 +1747,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
>      return bus->devices[devfn];
>  }
>  
> -static int pci_qdev_init(DeviceState *qdev)
> +static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>  {
>      PCIDevice *pci_dev = (PCIDevice *)qdev;
>      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> +    Error *local_err = NULL;
>      PCIBus *bus;
>      int rc;
>      bool is_default_rom;
> @@ -1758,15 +1764,16 @@ static int pci_qdev_init(DeviceState *qdev)
>      bus = PCI_BUS(qdev_get_parent_bus(qdev));
>      pci_dev = do_pci_register_device(pci_dev, bus,
>                                       object_get_typename(OBJECT(qdev)),
> -                                     pci_dev->devfn);
> +                                     pci_dev->devfn, errp);
>      if (pci_dev == NULL)
> -        return -1;
> +        return;
>  
>      if (pc->init) {
>          rc = pc->init(pci_dev);
>          if (rc != 0) {
>              do_pci_unregister_device(pci_dev);
> -            return rc;
> +            error_setg(errp, "Device initialization failed");
> +            return;
>          }
>      }
>  
> @@ -1777,13 +1784,12 @@ static int pci_qdev_init(DeviceState *qdev)
>          is_default_rom = true;
>      }
>  
> -    rc = pci_add_option_rom(pci_dev, is_default_rom);
> -    if (rc != 0) {
> -        pci_unregister_device(DEVICE(pci_dev));
> -        return rc;
> +    pci_add_option_rom(pci_dev, is_default_rom, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        pci_qdev_unrealize(DEVICE(pci_dev), NULL);
> +        return;
>      }
> -
> -    return 0;
>  }
>  
>  PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> @@ -1923,7 +1929,8 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
>  }
>  
>  /* Add an option rom for the device */
> -static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
> +static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
> +                               Error **errp)
>  {
>      int size;
>      char *path;
> @@ -1932,9 +1939,9 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>      const VMStateDescription *vmsd;
>  
>      if (!pdev->romfile)
> -        return 0;
> +        return;
>      if (strlen(pdev->romfile) == 0)
> -        return 0;
> +        return;
>  
>      if (!pdev->rom_bar) {
>          /*
> @@ -1948,7 +1955,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>           * if the rom bar is disabled.
>           */
>          if (DEVICE(pdev)->hotplugged) {
> -            return 0;
> +            return;
>          }
>  
>          if (class == 0x0300) {
> @@ -1956,7 +1963,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>          } else {
>              rom_add_option(pdev->romfile, -1);
>          }
> -        return 0;
> +        return;
>      }
>  
>      path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile);
> @@ -1966,15 +1973,13 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>  
>      size = get_image_size(path);
>      if (size < 0) {
> -        error_report("%s: failed to find romfile \"%s\"",
> -                     __func__, pdev->romfile);
> +        error_setg(errp, "failed to find romfile \"%s\"", pdev->romfile);
>          g_free(path);
> -        return -1;
> +        return;
>      } else if (size == 0) {
> -        error_report("%s: ignoring empty romfile \"%s\"",
> -                     __func__, pdev->romfile);
> +        error_setg(errp, "romfile \"%s\" is empty", pdev->romfile);
>          g_free(path);
> -        return -1;
> +        return;
>      }
>      if (size & (size - 1)) {
>          size = 1 << qemu_fls(size);
> @@ -2000,8 +2005,6 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>      }
>  
>      pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
> -
> -    return 0;
>  }
>  
>  static void pci_del_option_rom(PCIDevice *pdev)
> @@ -2283,8 +2286,8 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
>  static void pci_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *k = DEVICE_CLASS(klass);
> -    k->init = pci_qdev_init;
> -    k->exit = pci_unregister_device;
> +    k->realize = pci_qdev_realize;
> +    k->unrealize = pci_qdev_unrealize;
>      k->bus_type = TYPE_PCI_BUS;
>      k->props = pci_props;
>  }

Code-wise looks clean to me. QOM-wise I still cannot definitely say
that I know all the pitfalls.
That being said,

Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
                   ` (11 preceding siblings ...)
  2014-10-30 14:01 ` Andreas Färber
@ 2014-11-02 11:20 ` Michael S. Tsirkin
  2014-11-03  7:22   ` Markus Armbruster
  12 siblings, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2014-11-02 11:20 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: pbonzini, arei.gonglei, kraxel, qemu-devel, afaerber

On Tue, Oct 28, 2014 at 08:35:29AM +0100, Markus Armbruster wrote:
> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
> to realize", Paolo called the PCI conversion job "Gargantuan".  This
> series attempts to crack it into manageable jobs.
> 
> The basic idea comes from qdev core: have the core deal with just
> realize, but default the device models' realize() method to one that
> calls the old init() method.  Unconverted device models don't set
> their realize(), thus get one that calls their init().  We can then
> convert device by device instead of having to convert of all of PCI in
> one Gargantuan go.
> 
> Since PCI's exit() cannot fail, I chose not to add an unrealize().
> Precedence: USBDeviceClass method handle_destroy(), called on USB
> unrealize.
> 
> Aside: USBDeviceClass also has an unrealize() method, but it's never
> set and never called.
> 
> PATCH 01 converts the interface between PCI core and qdev to realize.
> 
> PATCH 02 adds realize to the interface between PCI core and PCI device
> models.  Once all device models are converted to realize, the old init
> interface can be dropped, completing the Gargantuan job.
> 
> PATCH 03-04 convert device models that cannot fail initialization.
> 
> PATCH 05-10 convert a few that can fail, but are really easy to
> convert.
> 
> This series is RFC because it's based on Marcel's "[PATCH v2] hw/pci:
> fixed crash when using rombar=0 with romfile=path for hotplugged
> devices", which is still undergoing review.

I've applied that one, and your patchset looks good to me,
But of course we can't apply new refactorings right now after soft
freeze.  Unless this fixes some bug?

> Markus Armbruster (10):
>   pci: Convert core to realize
>   pci: Permit incremental conversion of device models to realize
>   pci: Trivial device model conversions to realize
>   pcnet: pcnet_common_init() always returns 0, change to void
>   pcnet: Convert to realize
>   serial-pci: Convert to realize
>   ide/ich: Convert to realize
>   cirrus-vga: Convert to realize
>   qxl: Convert to realize
>   pci-assign: Convert to realize
> 
>  hw/acpi/piix4.c            |   5 +-
>  hw/audio/ac97.c            |   5 +-
>  hw/audio/es1370.c          |   5 +-
>  hw/audio/intel-hda.c       |   6 +--
>  hw/char/serial-pci.c       |  22 ++++-----
>  hw/display/cirrus_vga.c    |  11 ++---
>  hw/display/qxl.c           |  36 +++++++--------
>  hw/display/vga-pci.c       |  11 ++---
>  hw/display/vmware_vga.c    |   6 +--
>  hw/i2c/smbus_ich9.c        |   5 +-
>  hw/i386/kvm/pci-assign.c   |  10 ++--
>  hw/ide/cmd646.c            |   5 +-
>  hw/ide/ich.c               |  13 +++---
>  hw/ide/piix.c              |  10 ++--
>  hw/ide/via.c               |   6 +--
>  hw/ipack/tpci200.c         |   6 +--
>  hw/isa/i82378.c            |   6 +--
>  hw/isa/piix4.c             |   5 +-
>  hw/isa/vt82c686.c          |  24 ++++------
>  hw/misc/pci-testdev.c      |   6 +--
>  hw/net/e1000.c             |   6 +--
>  hw/net/eepro100.c          |   6 +--
>  hw/net/lance.c             |   3 +-
>  hw/net/ne2000.c            |   6 +--
>  hw/net/pcnet-pci.c         |   6 +--
>  hw/net/pcnet.c             |   4 +-
>  hw/net/pcnet.h             |   2 +-
>  hw/net/rtl8139.c           |   6 +--
>  hw/net/vmxnet3.c           |   6 +--
>  hw/pci-bridge/dec.c        |   5 +-
>  hw/pci-host/apb.c          |   5 +-
>  hw/pci-host/bonito.c       |   6 +--
>  hw/pci-host/grackle.c      |   5 +-
>  hw/pci-host/piix.c         |  12 ++---
>  hw/pci-host/ppce500.c      |   6 +--
>  hw/pci-host/prep.c         |   6 +--
>  hw/pci-host/q35.c          |   5 +-
>  hw/pci-host/uninorth.c     |  20 ++++----
>  hw/pci-host/versatile.c    |   5 +-
>  hw/pci/pci.c               | 111 ++++++++++++++++++++++++++-------------------
>  hw/usb/hcd-ehci-pci.c      |   6 +--
>  hw/usb/hcd-xhci.c          |   6 +--
>  hw/watchdog/wdt_i6300esb.c |   6 +--
>  include/hw/pci/pci.h       |   3 +-
>  44 files changed, 199 insertions(+), 256 deletions(-)
> 
> -- 
> 1.9.3

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2014-11-02 11:20 ` Michael S. Tsirkin
@ 2014-11-03  7:22   ` Markus Armbruster
  0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2014-11-03  7:22 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: pbonzini, arei.gonglei, kraxel, afaerber, qemu-devel

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Oct 28, 2014 at 08:35:29AM +0100, Markus Armbruster wrote:
>> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
>> to realize", Paolo called the PCI conversion job "Gargantuan".  This
>> series attempts to crack it into manageable jobs.
>> 
>> The basic idea comes from qdev core: have the core deal with just
>> realize, but default the device models' realize() method to one that
>> calls the old init() method.  Unconverted device models don't set
>> their realize(), thus get one that calls their init().  We can then
>> convert device by device instead of having to convert of all of PCI in
>> one Gargantuan go.
>> 
>> Since PCI's exit() cannot fail, I chose not to add an unrealize().
>> Precedence: USBDeviceClass method handle_destroy(), called on USB
>> unrealize.
>> 
>> Aside: USBDeviceClass also has an unrealize() method, but it's never
>> set and never called.
>> 
>> PATCH 01 converts the interface between PCI core and qdev to realize.
>> 
>> PATCH 02 adds realize to the interface between PCI core and PCI device
>> models.  Once all device models are converted to realize, the old init
>> interface can be dropped, completing the Gargantuan job.
>> 
>> PATCH 03-04 convert device models that cannot fail initialization.
>> 
>> PATCH 05-10 convert a few that can fail, but are really easy to
>> convert.
>> 
>> This series is RFC because it's based on Marcel's "[PATCH v2] hw/pci:
>> fixed crash when using rombar=0 with romfile=path for hotplugged
>> devices", which is still undergoing review.
>
> I've applied that one, and your patchset looks good to me,
> But of course we can't apply new refactorings right now after soft
> freeze.  Unless this fixes some bug?

I'm okay with delaying my series some.  I'd appreciate a ping when
you're ready to accept "for 2.3" patches into your tree.

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2014-10-30 14:01 ` Andreas Färber
@ 2014-11-03  7:40   ` Markus Armbruster
  2015-01-19 15:21     ` Markus Armbruster
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2014-11-03  7:40 UTC (permalink / raw)
  To: Andreas Färber; +Cc: pbonzini, arei.gonglei, mst, qemu-devel, kraxel

Andreas Färber <afaerber@suse.de> writes:

> Hi Markus,
>
> Am 28.10.2014 um 08:35 schrieb Markus Armbruster:
>> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
>> to realize", Paolo called the PCI conversion job "Gargantuan".  This
>> series attempts to crack it into manageable jobs.
>
> Thanks for giving this a stab! What kept me from diving into the PCI
> converstion was that I first invested into qtests for the non-default
> PCI devices. How many of the converted devices are actually covered in
> qtest?

Can't tell offhand, but I can find out.

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2014-11-03  7:40   ` Markus Armbruster
@ 2015-01-19 15:21     ` Markus Armbruster
  2015-01-30  9:17       ` Markus Armbruster
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2015-01-19 15:21 UTC (permalink / raw)
  To: Andreas Färber; +Cc: pbonzini, arei.gonglei, mst, qemu-devel, kraxel

Markus Armbruster <armbru@redhat.com> writes:

> Andreas Färber <afaerber@suse.de> writes:
>
>> Hi Markus,
>>
>> Am 28.10.2014 um 08:35 schrieb Markus Armbruster:
>>> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
>>> to realize", Paolo called the PCI conversion job "Gargantuan".  This
>>> series attempts to crack it into manageable jobs.
>>
>> Thanks for giving this a stab! What kept me from diving into the PCI
>> converstion was that I first invested into qtests for the non-default
>> PCI devices. How many of the converted devices are actually covered in
>> qtest?
>
> Can't tell offhand, but I can find out.

The vast majority of my conversions are utterly trivial [PATCH 03]:
change return type to void, rename, put into ->realize instead of
->init.  I could enumerate the devices so changed and look for qtests,
but I feel it's rather pointless busywork.  But if you should insist...

The remaining conversions are all very, very simple:

* PATCH 05: "pcnet"

  Utterly trivial after trivial PATCH 04 changed a helper's return type
  to void.

  There's pcnet-test.c, which basically tests "-device pcnet doesn't
  explode right away".

* PATCH 06: "pci-serial", "pci-serial-2x", "pci-serial-4x"

  Two error paths trivially converted from qerror_report_err() to
  error_propagate().

  Not covered in qtest as far as I can tell.

* PATCH 07: "ich9-ahci"

  One pci_add_capability() replaced by pci_add_capability2().  The
  former is a wrapper around the latter which additionally passes any
  error to error_report(), then frees it.  Straightforward conversion of
  the error path to error_setg().

  Not covered in qtest as far as I can tell.

* PATCH 08: "cirrus-vga"

  One error path trivially converted from error_report() to
  error_setg().

  There's display-vga-test.c, which tests "-device cirrus-vga doesn't
  explode right away".

* PATCH 09: "qxl-vga", "qxl"

  Two error paths trivially converted from error_report() to
  error_setg().

  Not covered in qtest as far as I can tell.

* PATCH 10: "kvm-pci-assign"

  Two error paths trivially converted from qerror_report_err() to
  error_propagate().

  Not covered in qtest as far as I can tell.

I'm prepared to drop conversions you consider risky without test
coverage (although I have a hard time seeing risks, considering how
silly-simple my conversions are).  But I'd really like to get the core
changes plus some examples in.

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

* Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
  2015-01-19 15:21     ` Markus Armbruster
@ 2015-01-30  9:17       ` Markus Armbruster
  0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2015-01-30  9:17 UTC (permalink / raw)
  To: Andreas Färber; +Cc: pbonzini, arei.gonglei, mst, qemu-devel, kraxel

Markus Armbruster <armbru@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Andreas Färber <afaerber@suse.de> writes:
>>
>>> Hi Markus,
>>>
>>> Am 28.10.2014 um 08:35 schrieb Markus Armbruster:
>>>> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
>>>> to realize", Paolo called the PCI conversion job "Gargantuan".  This
>>>> series attempts to crack it into manageable jobs.
>>>
>>> Thanks for giving this a stab! What kept me from diving into the PCI
>>> converstion was that I first invested into qtests for the non-default
>>> PCI devices. How many of the converted devices are actually covered in
>>> qtest?
>>
>> Can't tell offhand, but I can find out.
>
> The vast majority of my conversions are utterly trivial [PATCH 03]:
> change return type to void, rename, put into ->realize instead of
> ->init.  I could enumerate the devices so changed and look for qtests,
> but I feel it's rather pointless busywork.  But if you should insist...
>
> The remaining conversions are all very, very simple:
>
> * PATCH 05: "pcnet"
>
>   Utterly trivial after trivial PATCH 04 changed a helper's return type
>   to void.
>
>   There's pcnet-test.c, which basically tests "-device pcnet doesn't
>   explode right away".
>
> * PATCH 06: "pci-serial", "pci-serial-2x", "pci-serial-4x"
>
>   Two error paths trivially converted from qerror_report_err() to
>   error_propagate().
>
>   Not covered in qtest as far as I can tell.
>
> * PATCH 07: "ich9-ahci"
>
>   One pci_add_capability() replaced by pci_add_capability2().  The
>   former is a wrapper around the latter which additionally passes any
>   error to error_report(), then frees it.  Straightforward conversion of
>   the error path to error_setg().
>
>   Not covered in qtest as far as I can tell.
>
> * PATCH 08: "cirrus-vga"
>
>   One error path trivially converted from error_report() to
>   error_setg().
>
>   There's display-vga-test.c, which tests "-device cirrus-vga doesn't
>   explode right away".
>
> * PATCH 09: "qxl-vga", "qxl"
>
>   Two error paths trivially converted from error_report() to
>   error_setg().
>
>   Not covered in qtest as far as I can tell.
>
> * PATCH 10: "kvm-pci-assign"
>
>   Two error paths trivially converted from qerror_report_err() to
>   error_propagate().
>
>   Not covered in qtest as far as I can tell.
>
> I'm prepared to drop conversions you consider risky without test
> coverage (although I have a hard time seeing risks, considering how
> silly-simple my conversions are).  But I'd really like to get the core
> changes plus some examples in.

This series passes all tests added by my "qtest: Generic PCI device
test" series.  That series covers realizing every PCI device, except for

* blacklisted devices, and

* devices not included in the x86 targets (because the test runs only
  for the x86 targets, just like all of $(check-qtest-pci-y)).

Devices in the above list of not entirely trivial conversions that
aren't covered:

* "kvm-pci-assign" is blacklisted because it requires a suitable host
  device to pass through.

* "qxl" is blacklisted because "-device qxl" crashes.  However,
  "qxl-vga" executes the patched code as well, and *is* covered.

Andreas, if you object to any of my conversions without additional test
coverage, please enumerate the devices whose conversion you want me to
drop.

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

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

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-28  7:35 [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize Markus Armbruster
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 01/10] pci: Convert core " Markus Armbruster
2014-10-28  8:32   ` Gonglei
2014-10-28  9:38     ` Markus Armbruster
2014-10-30 16:58       ` Marcel Apfelbaum
2014-10-30 17:02   ` Marcel Apfelbaum
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 02/10] pci: Permit incremental conversion of device models " Markus Armbruster
2014-10-28  8:35   ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 03/10] pci: Trivial device model conversions " Markus Armbruster
2014-10-28  8:38   ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void Markus Armbruster
2014-10-28  8:42   ` Gonglei
2014-10-28  9:41     ` Markus Armbruster
2014-10-28 10:35       ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 05/10] pcnet: Convert to realize Markus Armbruster
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 06/10] serial-pci: " Markus Armbruster
2014-10-28  8:43   ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 07/10] ide/ich: " Markus Armbruster
2014-10-28  8:44   ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 08/10] cirrus-vga: " Markus Armbruster
2014-10-28  8:44   ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 09/10] qxl: " Markus Armbruster
2014-10-28  8:46   ` Gonglei
2014-10-28  7:35 ` [Qemu-devel] [PATCH RFC 10/10] pci-assign: " Markus Armbruster
2014-10-28  8:49   ` Gonglei
2014-10-28  8:27 ` [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion " Gonglei
2014-10-30 14:01 ` Andreas Färber
2014-11-03  7:40   ` Markus Armbruster
2015-01-19 15:21     ` Markus Armbruster
2015-01-30  9:17       ` Markus Armbruster
2014-11-02 11:20 ` Michael S. Tsirkin
2014-11-03  7:22   ` 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.