[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges |
Date: |
Mon, 13 Aug 2012 09:16:30 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
"Michael S. Tsirkin" <address@hidden> writes:
> On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas Färber wrote:
>> Uglify the parent field to enforce QOM-style access via casts.
>> Don't just typedef PCIHostState, either use it directly or embed it.
>>
>> Signed-off-by: Andreas Färber <address@hidden>
>
>
> IMHO only one chunk from this patch should be applied (below).
> Below it is split out but needs to be rebased on top of patches
> 1-13.
I understand what your objection is but it's unreasonable IMHO. The
purpose of QOM is to bring consistency across large swaths of code in
QEMU that have historically done things there own way.
This means expressing concepts like inheritence and casting in the same
way across the board. The common way (the QOM way) is to make the
parent type the first member of the struct (typically named parent or
parent_obj) and then to use cast macros to upcast and downcast.
This patch is 100% correct in that regard and I'm going to apply it once
Andreas makes the change I requested.
For my part, I'm long over due in writing up a device authoring style
guide that I promised a few weeks ago. I'll write that up this
afternoon and send it out today.
We can debate the merits of this sort of thing in the style guide.
Regards,
Anthony Liguori
>
> -->
>
> From: Andreas Färber <address@hidden>
>
> piix: minor code simplification
>
> There's no need to deal with qdev internals in piix - we get device
> state from qdev_create so just use that.
>
> Signed-off-by: Andreas Färber <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
>
> ---
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index c497a01..18554a6 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -274,7 +274,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
> dev = qdev_create(NULL, "i440FX-pcihost");
> s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
> s->address_space = address_space_mem;
> - b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
> + b = pci_bus_new(&dev, NULL, pci_address_space,
> address_space_io, 0);
> s->bus = b;
> object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev),
> NULL);
- [Qemu-devel] [PATCH for-1.2 v5 09/14] prep_pci: QOM'ify Raven PCI host bridge, (continued)
- [Qemu-devel] [PATCH for-1.2 v5 09/14] prep_pci: QOM'ify Raven PCI host bridge, Andreas Färber, 2012/08/01
- [Qemu-devel] [PATCH for-1.2 v5 10/14] spapr_pci: QOM'ify sPAPR PCI host bridge, Andreas Färber, 2012/08/01
- [Qemu-devel] [PATCH for-1.2 v5 11/14] unin_pci: QOM'ify UniNorth PCI host bridges, Andreas Färber, 2012/08/01
- [Qemu-devel] [PATCH for-1.2 v5 12/14] pci_host: Turn into SysBus-derived QOM type, Andreas Färber, 2012/08/01
- [Qemu-devel] [PATCH for-1.2 v5 13/14] pci: Derive PCI host bridges from TYPE_PCI_HOST_BRIDGE, Andreas Färber, 2012/08/01
- [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges, Andreas Färber, 2012/08/01
- Re: [Qemu-devel] [PATCH for-1.2 v5 00/14] pci_host: Convert to QOM, Michael S. Tsirkin, 2012/08/02