diff mbox

[2/4] qdev: add children before qdev_init

Message ID 1332866328-25443-3-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini March 27, 2012, 4:38 p.m. UTC
We want the composition tree to to be in order by the time we call
qdev_init, so that a single set of the toplevel realize property can
propagate all the way down the composition tree.

This is not the case so far.  Unfortunately, this is incompatible
with calling qdev_init in the constructor wrappers for devices,
so for now we need to unattach some devices that are created through
those wrappers.  This will be fixed by removing qdev_init and instead
setting the toplevel realize property after machine init.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pc_piix.c      |   18 +-----------------
 hw/piix_pci.c     |    3 +--
 hw/ppc_prep.c     |    2 +-
 hw/qdev-monitor.c |    8 ++++----
 4 files changed, 7 insertions(+), 24 deletions(-)

Comments

Anthony Liguori March 27, 2012, 9:08 p.m. UTC | #1
On 03/27/2012 11:38 AM, Paolo Bonzini wrote:
> We want the composition tree to to be in order by the time we call
> qdev_init, so that a single set of the toplevel realize property can
> propagate all the way down the composition tree.
>
> This is not the case so far.  Unfortunately, this is incompatible
> with calling qdev_init in the constructor wrappers for devices,
> so for now we need to unattach some devices that are created through
> those wrappers.  This will be fixed by removing qdev_init and instead
> setting the toplevel realize property after machine init.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   hw/pc_piix.c      |   18 +-----------------
>   hw/piix_pci.c     |    3 +--
>   hw/ppc_prep.c     |    2 +-
>   hw/qdev-monitor.c |    8 ++++----
>   4 files changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 3f99f9a..747c40f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -146,7 +146,6 @@ static void pc_init1(MemoryRegion *system_memory,
>       MemoryRegion *ram_memory;
>       MemoryRegion *pci_memory;
>       MemoryRegion *rom_memory;
> -    DeviceState *dev;
>
>       pc_cpus_init(cpu_model);
>
> @@ -224,11 +223,7 @@ static void pc_init1(MemoryRegion *system_memory,
>
>       pc_register_ferr_irq(gsi[13]);
>
> -    dev = pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
> -    if (dev) {
> -        object_property_add_child(object_get_root(), "vga", OBJECT(dev), NULL);
> -    }
> -
> +    pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
>       if (xen_enabled()) {
>           pci_create_simple(pci_bus, -1, "xen-platform");
>       }
> @@ -255,17 +250,6 @@ static void pc_init1(MemoryRegion *system_memory,
>           }
>           idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
>           idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
> -
> -        /* FIXME there's some major spaghetti here.  Somehow we create the
> -         * devices on the PIIX before we actually create it.  We create the
> -         * PIIX3 deep in the recess of the i440fx creation too and then lose
> -         * the DeviceState.
> -         *
> -         * For now, let's "fix" this by making judicious use of paths.  This
> -         * is not generally the right way to do this.
> -         */
> -        object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
> -                                  "rtc", (Object *)rtc_state, NULL);
>       } else {
>           for(i = 0; i<  MAX_IDE_BUS; i++) {
>               ISADevice *dev;
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index e0268fe..9017565 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -276,8 +276,8 @@ static PCIBus *i440fx_common_init(const char *device_name,
>       b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
>                       address_space_io, 0);
>       s->bus = b;
> -    qdev_init_nofail(dev);
>       object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
> +    qdev_init_nofail(dev);
>
>       d = pci_create_simple(b, 0, device_name);
>       *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
> @@ -316,7 +316,6 @@ static PCIBus *i440fx_common_init(const char *device_name,
>           pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
>                   PIIX_NUM_PIRQS);
>       }
> -    object_property_add_child(OBJECT(dev), "piix3", OBJECT(piix3), NULL);
>       piix3->pic = pic;
>       *isa_bus = DO_UPCAST(ISABus, qbus,
>                            qdev_get_child_bus(&piix3->dev.qdev, "isa.0"));
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 06d589d..86c9336 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -615,8 +615,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
>       sys = sysbus_from_qdev(dev);
>       pcihost = DO_UPCAST(PCIHostState, busdev, sys);
>       pcihost->address_space = get_system_memory();
> -    qdev_init_nofail(dev);
>       object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
> +    qdev_init_nofail(dev);
>       pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>       if (pci_bus == NULL) {
>           fprintf(stderr, "Couldn't create PCI host controller.\n");
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 2e82962..031cb83 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -458,10 +458,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>           qdev_free(qdev);
>           return NULL;
>       }
> -    if (qdev_init(qdev)<  0) {
> -        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> -        return NULL;
> -    }
>       if (qdev->id) {
>           object_property_add_child(qdev_get_peripheral(), qdev->id,
>                                     OBJECT(qdev), NULL);
> @@ -472,6 +468,10 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>                                     OBJECT(qdev), NULL);
>           g_free(name);
>       }
> +    if (qdev_init(qdev)<  0) {
> +        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> +        return NULL;
> +    }
>       qdev->opts = opts;
>       return qdev;
>   }
diff mbox

Patch

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3f99f9a..747c40f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -146,7 +146,6 @@  static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
-    DeviceState *dev;
 
     pc_cpus_init(cpu_model);
 
@@ -224,11 +223,7 @@  static void pc_init1(MemoryRegion *system_memory,
 
     pc_register_ferr_irq(gsi[13]);
 
-    dev = pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
-    if (dev) {
-        object_property_add_child(object_get_root(), "vga", OBJECT(dev), NULL);
-    }
-
+    pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
     if (xen_enabled()) {
         pci_create_simple(pci_bus, -1, "xen-platform");
     }
@@ -255,17 +250,6 @@  static void pc_init1(MemoryRegion *system_memory,
         }
         idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
         idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
-
-        /* FIXME there's some major spaghetti here.  Somehow we create the
-         * devices on the PIIX before we actually create it.  We create the
-         * PIIX3 deep in the recess of the i440fx creation too and then lose
-         * the DeviceState.
-         *
-         * For now, let's "fix" this by making judicious use of paths.  This
-         * is not generally the right way to do this.
-         */
-        object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
-                                  "rtc", (Object *)rtc_state, NULL);
     } else {
         for(i = 0; i < MAX_IDE_BUS; i++) {
             ISADevice *dev;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index e0268fe..9017565 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -276,8 +276,8 @@  static PCIBus *i440fx_common_init(const char *device_name,
     b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
                     address_space_io, 0);
     s->bus = b;
-    qdev_init_nofail(dev);
     object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
+    qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, device_name);
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
@@ -316,7 +316,6 @@  static PCIBus *i440fx_common_init(const char *device_name,
         pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
                 PIIX_NUM_PIRQS);
     }
-    object_property_add_child(OBJECT(dev), "piix3", OBJECT(piix3), NULL);
     piix3->pic = pic;
     *isa_bus = DO_UPCAST(ISABus, qbus,
                          qdev_get_child_bus(&piix3->dev.qdev, "isa.0"));
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 06d589d..86c9336 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -615,8 +615,8 @@  static void ppc_prep_init (ram_addr_t ram_size,
     sys = sysbus_from_qdev(dev);
     pcihost = DO_UPCAST(PCIHostState, busdev, sys);
     pcihost->address_space = get_system_memory();
-    qdev_init_nofail(dev);
     object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
+    qdev_init_nofail(dev);
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (pci_bus == NULL) {
         fprintf(stderr, "Couldn't create PCI host controller.\n");
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 2e82962..031cb83 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -458,10 +458,6 @@  DeviceState *qdev_device_add(QemuOpts *opts)
         qdev_free(qdev);
         return NULL;
     }
-    if (qdev_init(qdev) < 0) {
-        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
-        return NULL;
-    }
     if (qdev->id) {
         object_property_add_child(qdev_get_peripheral(), qdev->id,
                                   OBJECT(qdev), NULL);
@@ -472,6 +468,10 @@  DeviceState *qdev_device_add(QemuOpts *opts)
                                   OBJECT(qdev), NULL);
         g_free(name);
     }        
+    if (qdev_init(qdev) < 0) {
+        qerror_report(QERR_DEVICE_INIT_FAILED, driver);
+        return NULL;
+    }
     qdev->opts = opts;
     return qdev;
 }