From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36927) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCdeo-0001mY-MW for qemu-devel@nongnu.org; Tue, 27 Mar 2012 17:10:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCdem-0005Bo-QJ for qemu-devel@nongnu.org; Tue, 27 Mar 2012 17:10:22 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:57524) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCdem-0005BS-LT for qemu-devel@nongnu.org; Tue, 27 Mar 2012 17:10:20 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 27 Mar 2012 17:10:16 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 964C438C806B for ; Tue, 27 Mar 2012 17:10:13 -0400 (EDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q2RLA86b223654 for ; Tue, 27 Mar 2012 17:10:10 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q2RL91Ff018334 for ; Tue, 27 Mar 2012 17:09:01 -0400 Message-ID: <4F722C63.4040203@us.ibm.com> Date: Tue, 27 Mar 2012 16:08:51 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1332866328-25443-1-git-send-email-pbonzini@redhat.com> <1332866328-25443-3-git-send-email-pbonzini@redhat.com> In-Reply-To: <1332866328-25443-3-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/4] qdev: add children before qdev_init List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aliguori@linux.vnet.ibm.com, andreas.faerber@web.de, qemu-devel@nongnu.org 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 Reviewed-by: Anthony Liguori 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; > }