Re: [Qemu-devel] [PATCH v3 0/28] qom: add QEMU Object Model type hierarchy to qdev

[Anthony Liguori <aliguori@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 13245 bytes --]

On 01/25/2012 12:31 PM, Blue Swirl wrote:
> On Tue, Jan 24, 2012 at 19:32, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> This series makes qdev a proper Object and converts qdev's type inheritance to
>> QOM inheritance.
>>
>> The first half of the series are manual cleanups/refactorings.  The second half
>> is mostly scripted conversion, separated out into reviewable and bisectable
>> chunks.
>>
>> I've tested this series extensively for the pc target including bisectability.
>> I have not tested any other targets yet so your mileage may vary.
>>
>> Since the original RFC, I've fixed any of the not-for-upstream issues.  This
>> also includes some uni-north patches that are in ppc-next.
>>
>> Paolo and Andreas have asked that I declare a date when I'll merge this so that
>> people can be prepared for the rebase fall out.  I will merge this on Friday
>> unless there is a major problem found in review.
>
> I get this with Sparc64:
> $ qemu-system-sparc64
> qemu-system-sparc64: PCI: slot 0 function 0 not available for pbm, in use by\

The casts in qdev are not type safe (because they don't do RTTI).  You're 
probably getting the sysbus version of pbm but then casting to a PCI device 
which is resulting in a garbage name pointer.

As we switch to use the QOM dynamic casts, a similar situation would trigger an 
abort().

> qemu-system-sparc64: Initialization of device pbm failed
>
> Maybe because device "pbm" exists for both as SysBusDevice and as
> PCIDevice. Text "in use by" is suspiciously clipped.
>
> Sparc32, PPC32 and PPC64 seem to be OK.

This should address that and the other occurrences.

Regards,

Anthony Liguori

>
>> This is also available at:
>>
>> https://github.com/aliguori/qemu/tree/qom-upstream.10
>>
>> For full context, the whole tree is located at:
>>
>> https://github.com/aliguori/qemu/commits/qom-rebase.7
>>
>> git diffstat origin/master HEAD
>>   Makefile                   |    2
>>   Makefile.hw                |    1
>>   Makefile.objs              |    3
>>   Makefile.target            |    2
>>   configure                  |    2
>>   hw/9pfs/virtio-9p-device.c |   44 ++--
>>   hw/a9mpcore.c              |   21 +
>>   hw/ac97.c                  |   39 ++-
>>   hw/acpi_piix4.c            |   59 +++--
>>   hw/ads7846.c               |   17 +
>>   hw/alpha_typhoon.c         |   17 +
>>   hw/apb_pci.c               |   71 ++++--
>>   hw/apic.c                  |   21 +
>>   hw/apic_common.c           |   88 +++++---
>>   hw/apic_internal.h         |   32 +-
>>   hw/applesmc.c              |   18 +
>>   hw/arm11mpcore.c           |   68 +++---
>>   hw/arm_l2x0.c              |   25 +-
>>   hw/arm_mptimer.c           |   23 +-
>>   hw/arm_sysctl.c            |   31 +-
>>   hw/arm_timer.c             |   46 +++-
>>   hw/armv7m.c                |   25 +-
>>   hw/armv7m_nvic.c           |   21 +
>>   hw/bitbang_i2c.c           |   17 +
>>   hw/bonito.c                |   47 ++--
>>   hw/ccid-card-emulated.c    |   27 +-
>>   hw/ccid-card-passthru.c    |   27 +-
>>   hw/ccid.h                  |   28 +-
>>   hw/cirrus_vga.c            |   50 ++--
>>   hw/container.c             |   17 +
>>   hw/cs4231.c                |   27 +-
>>   hw/cs4231a.c               |   20 +
>>   hw/debugcon.c              |   16 +
>>   hw/dec_pci.c               |   74 ++++--
>>   hw/ds1225y.c               |   29 +-
>>   hw/ds1338.c                |   31 +-
>>   hw/e1000.c                 |   45 ++--
>>   hw/eccmemctl.c             |   29 +-
>>   hw/eepro100.c              |  202 +++++++++++-------
>>   hw/empty_slot.c            |   15 +
>>   hw/es1370.c                |   36 +--
>>   hw/escc.c                  |   43 ++-
>>   hw/esp.c                   |   27 +-
>>   hw/etraxfs_eth.c           |   33 +--
>>   hw/etraxfs_pic.c           |   25 +-
>>   hw/etraxfs_ser.c           |   17 +
>>   hw/etraxfs_timer.c         |   16 +
>>   hw/fdc.c                   |   84 +++++--
>>   hw/fw_cfg.c                |   33 +--
>>   hw/g364fb.c                |   33 +--
>>   hw/grackle_pci.c           |   44 ++--
>>   hw/grlib_apbuart.c         |   25 +-
>>   hw/grlib_gptimer.c         |   31 +-
>>   hw/grlib_irqmp.c           |   29 +-
>>   hw/gt64xxx.c               |   39 ++-
>>   hw/gus.c                   |   20 +
>>   hw/hda-audio.c             |   60 +++--
>>   hw/hpet.c                  |   33 +--
>>   hw/i2c.c                   |  111 +++++++---
>>   hw/i2c.h                   |   52 ++--
>>   hw/i82374.c                |   19 +
>>   hw/i82378.c                |   31 +-
>>   hw/i8254.c                 |   22 +-
>>   hw/i8259.c                 |   15 +
>>   hw/i8259_common.c          |   45 ++--
>>   hw/i8259_internal.h        |   27 +-
>>   hw/ide/cmd646.c            |   41 ++-
>>   hw/ide/ich.c               |   37 +--
>>   hw/ide/internal.h          |   20 +
>>   hw/ide/isa.c               |   20 +
>>   hw/ide/piix.c              |   91 +++++---
>>   hw/ide/qdev.c              |  111 ++++++----
>>   hw/ide/via.c               |   27 +-
>>   hw/integratorcp.c          |   40 ++-
>>   hw/intel-hda.c             |   91 +++++---
>>   hw/intel-hda.h             |   26 +-
>>   hw/ioapic.c                |   17 +
>>   hw/ioapic_common.c         |   49 +++-
>>   hw/ioapic_internal.h       |   26 +-
>>   hw/ioh3420.c               |   59 +++--
>>   hw/isa-bus.c               |   57 +++--
>>   hw/isa.h                   |   23 +-
>>   hw/ivshmem.c               |   47 ++--
>>   hw/kvm/apic.c              |   19 +
>>   hw/kvm/clock.c             |   19 +
>>   hw/kvm/i8259.c             |   19 +
>>   hw/kvm/ioapic.c            |   23 +-
>>   hw/lan9118.c               |   31 +-
>>   hw/lance.c                 |   33 +--
>>   hw/lm32_juart.c            |   19 +
>>   hw/lm32_pic.c              |   19 +
>>   hw/lm32_sys.c              |   29 +-
>>   hw/lm32_timer.c            |   31 +-
>>   hw/lm32_uart.c             |   19 +
>>   hw/lm832x.c                |   33 +--
>>   hw/lsi53c895a.c            |   33 +--
>>   hw/m48t59.c                |   51 +++-
>>   hw/macio.c                 |   50 +++-
>>   hw/marvell_88w8618_audio.c |   35 +--
>>   hw/max111x.c               |   34 ++-
>>   hw/max7310.c               |   35 +--
>>   hw/mc146818rtc.c           |   22 +-
>>   hw/milkymist-ac97.c        |   19 +
>>   hw/milkymist-hpdmc.c       |   19 +
>>   hw/milkymist-memcard.c     |   19 +
>>   hw/milkymist-minimac2.c    |   37 ++-
>>   hw/milkymist-pfpu.c        |   19 +
>>   hw/milkymist-softusb.c     |   43 ++-
>>   hw/milkymist-sysctl.c      |   43 ++-
>>   hw/milkymist-tmu2.c        |   19 +
>>   hw/milkymist-uart.c        |   19 +
>>   hw/milkymist-vgafb.c       |   31 +-
>>   hw/mips_malta.c            |   19 +
>>   hw/mipsnet.c               |   33 +--
>>   hw/mpc8544_guts.c          |   15 +
>>   hw/mst_fpga.c              |   19 +
>>   hw/musicpal.c              |  153 ++++++++++----
>>   hw/nand.c                  |   33 +--
>>   hw/ne2000-isa.c            |   18 +
>>   hw/ne2000.c                |   37 ++-
>>   hw/omap_gpio.c             |   70 ++++--
>>   hw/omap_intc.c             |   62 +++--
>>   hw/onenand.c               |   35 ++-
>>   hw/opencores_eth.c         |   31 +-
>>   hw/openpic.c               |   30 --
>>   hw/openpic.h               |    2
>>   hw/parallel.c              |   16 +
>>   hw/pc.c                    |   20 +
>>   hw/pci.c                   |  140 +++++-------
>>   hw/pci.h                   |   80 +++----
>>   hw/pci_bridge.c            |    2
>>   hw/pcie.c                  |    2
>>   hw/pckbd.c                 |   18 +
>>   hw/pcnet-pci.c             |   39 ++-
>>   hw/pcnet.c                 |    2
>>   hw/piix4.c                 |   35 +--
>>   hw/piix_pci.c              |  126 +++++++----
>>   hw/pl011.c                 |   36 ++-
>>   hw/pl022.c                 |   15 +
>>   hw/pl031.c                 |   19 +
>>   hw/pl041.c                 |   34 +--
>>   hw/pl050.c                 |   34 ++-
>>   hw/pl061.c                 |   34 ++-
>>   hw/pl080.c                 |   38 ++-
>>   hw/pl110.c                 |   57 +++--
>>   hw/pl181.c                 |   23 +-
>>   hw/pl190.c                 |   21 +
>>   hw/ppc4xx_pci.c            |   38 ++-
>>   hw/ppc_newworld.c          |    2
>>   hw/ppce500_pci.c           |   38 ++-
>>   hw/ppce500_spin.c          |   15 +
>>   hw/prep_pci.c              |   52 ++--
>>   hw/pxa2xx.c                |   96 ++++++--
>>   hw/pxa2xx_dma.c            |   29 +-
>>   hw/pxa2xx_gpio.c           |   29 +-
>>   hw/pxa2xx_pic.c            |   19 +
>>   hw/pxa2xx_timer.c          |   66 +++---
>>   hw/qdev-properties.c       |   20 -
>>   hw/qdev.c                  |  123 ++++++++---
>>   hw/qdev.h                  |   40 +++
>>   hw/qxl.c                   |   66 +++---
>>   hw/realview.c              |   15 +
>>   hw/realview_gic.c          |   16 +
>>   hw/rtl8139.c               |   43 ++-
>>   hw/s390-virtio-bus.c       |  110 +++++++---
>>   hw/sb16.c                  |   20 +
>>   hw/sbi.c                   |   19 +
>>   hw/scsi-bus.c              |   95 ++++++--
>>   hw/scsi-disk.c             |  159 ++++++++------
>>   hw/scsi-generic.c          |   29 +-
>>   hw/scsi.h                  |   31 +-
>>   hw/serial.c                |   18 +
>>   hw/sga.c                   |   15 -
>>   hw/sh_pci.c                |   37 ++-
>>   hw/slavio_intctl.c         |   19 +
>>   hw/slavio_misc.c           |   34 ++-
>>   hw/slavio_timer.c          |   29 +-
>>   hw/smbus.c                 |   85 +++++--
>>   hw/smbus.h                 |   42 ++-
>>   hw/smbus_eeprom.c          |   27 +-
>>   hw/smc91c111.c             |   31 +-
>>   hw/spapr_llan.c            |   41 ++-
>>   hw/spapr_pci.c             |   31 ++
>>   hw/spapr_vio.c             |   72 ++++--
>>   hw/spapr_vio.h             |   35 ++-
>>   hw/spapr_vscsi.c           |   37 ++-
>>   hw/spapr_vty.c             |   37 ++-
>>   hw/sparc32_dma.c           |   31 +-
>>   hw/spitz.c                 |   94 +++++---
>>   hw/ssd0303.c               |   33 +--
>>   hw/ssd0323.c               |   17 +
>>   hw/ssi-sd.c                |   17 +
>>   hw/ssi.c                   |   23 +-
>>   hw/ssi.h                   |   18 +
>>   hw/stellaris.c             |   65 +++++-
>>   hw/stellaris_enet.c        |   27 +-
>>   hw/strongarm.c             |  124 +++++++----
>>   hw/sun4c_intctl.c          |   19 +
>>   hw/sun4m.c                 |   78 +++++--
>>   hw/sun4m_iommu.c           |   29 +-
>>   hw/sun4u.c                 |   71 ++++--
>>   hw/sysbus.c                |   40 ++-
>>   hw/sysbus.h                |   25 +-
>>   hw/tcx.c                   |   37 ++-
>>   hw/tmp105.c                |   37 ++-
>>   hw/tosa.c                  |   50 ++--
>>   hw/tusb6010.c              |   17 +
>>   hw/twl92230.c              |   35 +--
>>   hw/unin_pci.c              |  204 +++++++++++++-----
>>   hw/usb-audio.c             |   38 ++-
>>   hw/usb-bt.c                |   33 +--
>>   hw/usb-bus.c               |  190 +++++++++++++----
>>   hw/usb-ccid.c              |  114 +++++++---
>>   hw/usb-desc.c              |   22 +-
>>   hw/usb-ehci.c              |   63 +++--
>>   hw/usb-hid.c               |  108 +++++-----
>>   hw/usb-hub.c               |   35 +--
>>   hw/usb-msd.c               |   41 ++-
>>   hw/usb-net.c               |   43 ++-
>>   hw/usb-ohci.c              |   58 +++--
>>   hw/usb-serial.c            |   74 +++---
>>   hw/usb-uhci.c              |  189 +++++++++++------
>>   hw/usb-wacom.c             |   36 +--
>>   hw/usb-xhci.c              |   37 ++-
>>   hw/usb.c                   |   24 --
>>   hw/usb.h                   |   50 +++-
>>   hw/versatile_pci.c         |   53 +++-
>>   hw/versatilepb.c           |   19 +
>>   hw/vga-isa.c               |   18 +
>>   hw/vga-pci.c               |   27 +-
>>   hw/virtio-console.c        |   69 +++---
>>   hw/virtio-net.c            |    2
>>   hw/virtio-pci.c            |  210 +++++++++++--------
>>   hw/virtio-serial-bus.c     |   68 +++---
>>   hw/virtio-serial.h         |   81 ++++---
>>   hw/vmmouse.c               |   22 +-
>>   hw/vmport.c                |   16 +
>>   hw/vmware_vga.c            |   34 +--
>>   hw/vt82c686.c              |  120 ++++++-----
>>   hw/wdt_i6300esb.c          |   33 +--
>>   hw/wdt_ib700.c             |   18 +
>>   hw/wm8750.c                |   37 ++-
>>   hw/xen_platform.c          |   34 +--
>>   hw/xilinx_axidma.c         |   27 +-
>>   hw/xilinx_axienet.c        |   35 ++-
>>   hw/xilinx_ethlite.c        |   31 +-
>>   hw/xilinx_intc.c           |   25 +-
>>   hw/xilinx_timer.c          |   27 +-
>>   hw/xilinx_uartlite.c       |   16 +
>>   hw/xio3130_downstream.c    |   59 +++--
>>   hw/xio3130_upstream.c      |   53 ++--
>>   hw/z2.c                    |   52 +++-
>>   hw/zaurus.c                |   27 +-
>>   include/qemu/object.h      |  436 ++++++++++++++++++++++++++++++++++++++++
>>   qom/Makefile               |    1
>>   qom/object.c               |  485 +++++++++++++++++++++++++++++++++++++++++++++
>>   usb-bsd.c                  |   29 +-
>>   usb-linux.c                |   37 +--
>>   usb-redir.c                |   33 +--
>>   259 files changed, 7764 insertions(+), 3755 deletions(-)
>>
>>


[-- Attachment #2: 0001-qdev-change-ambiguous-qdev-names.patch --]
[-- Type: text/x-patch, Size: 2459 bytes --]

>From 196d73de2cc0687b9d5b159d9421ae85c8deff68 Mon Sep 17 00:00:00 2001
From: Anthony Liguori <aliguori@us.ibm.com>
Date: Wed, 25 Jan 2012 13:37:36 -0600
Subject: [PATCH] qdev: change ambiguous qdev names

Reported-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/apb_pci.c   |    4 ++--
 hw/dec_pci.c   |    2 +-
 hw/spapr_pci.c |    2 +-
 qom/object.c   |    5 +++++
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 173dab3..1a45420 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -350,7 +350,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
         sysbus_connect_irq(s, i, pic[i]);
     }
 
-    pci_create_simple(d->bus, 0, "pbm");
+    pci_create_simple(d->bus, 0, "pbm-pci");
 
     /* APB secondary busses */
     pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 0), true,
@@ -448,7 +448,7 @@ static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
 }
 
 static DeviceInfo pbm_pci_host_info = {
-    .name = "pbm",
+    .name = "pbm-pci",
     .size = sizeof(PCIDevice),
     .class_init = pbm_pci_host_class_init,
 };
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index f0ecaff..333bad9 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -131,7 +131,7 @@ static void pci_dec_21154_device_class_init(ObjectClass *klass, void *data)
 }
 
 static DeviceInfo pci_dec_21154_device_info = {
-    .name = "dec-21154",
+    .name = "dec-21154-sysbus",
     .size = sizeof(DECState),
     .class_init = pci_dec_21154_device_class_init,
 };
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index b6ac0d4..2a82eae 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -222,7 +222,7 @@ static void spapr_main_pci_host_class_init(ObjectClass *klass, void *data)
 }
 
 static DeviceInfo spapr_main_pci_host_info = {
-    .name = "spapr-pci-host-bridge",
+    .name = "spapr-pci-host-bridge-pci",
     .size = sizeof(PCIDevice),
     .class_init = spapr_main_pci_host_class_init,
 };
diff --git a/qom/object.c b/qom/object.c
index ef37e08..a12895f 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -87,6 +87,11 @@ TypeImpl *type_register(const TypeInfo *info)
 
     g_assert(info->name != NULL);
 
+    if (type_table_lookup(info->name) != NULL) {
+        fprintf(stderr, "Registering `%s' which already exists\n", info->name);
+        abort();
+    }
+
     ti->name = g_strdup(info->name);
     ti->parent = g_strdup(info->parent);
 
-- 
1.7.4.1