[Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize

[Markus Armbruster]

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 <address@hidden>
---
 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