Re: [Qemu-devel] [PATCH 07/17] s390x/pci: introduce S390PCIBus

[Marcel Apfelbaum <marcel@redhat.com>]

On 06/28/2016 06:20 PM, Cornelia Huck wrote:
> On Tue, 28 Jun 2016 17:39:30 +0300
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> On 06/24/2016 04:28 PM, Cornelia Huck wrote:
>>> From: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>>
>>> To enable S390PCIBusDevice as qdev, there should be a new bus to
>>> plug and manage all instances of S390PCIBusDevice. Due to this,
>>> S390PCIBus is introduced.
>>>
>>> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
>>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>>> ---
>>>    hw/s390x/s390-pci-bus.c | 10 ++++++++++
>>>    hw/s390x/s390-pci-bus.h |  8 ++++++++
>>>    2 files changed, 18 insertions(+)
>>>
>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>> index 0f6fcef..0c67c1e 100644
>>> --- a/hw/s390x/s390-pci-bus.c
>>> +++ b/hw/s390x/s390-pci-bus.c
>>> @@ -527,6 +527,9 @@ static int s390_pcihost_init(SysBusDevice *dev)
>>>        bus = BUS(b);
>>>        qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
>>>        phb->bus = b;
>>> +
>>> +    s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
>>> +
>>>        QTAILQ_INIT(&s->pending_sei);
>>>        return 0;
>>>    }
>>> @@ -636,9 +639,16 @@ static const TypeInfo s390_pcihost_info = {
>>>        }
>>>    };
>>>
>>> +static const TypeInfo s390_pcibus_info = {
>>> +    .name = TYPE_S390_PCI_BUS,
>>> +    .parent = TYPE_BUS,
>>
>> Hi,
>>
>> The type is named TYPE_S390_PCI_BUS, but does not
>> derive from PCI_BUS. I find it a little confusing, anyway is just a thought.
>> Maybe you should go with TYPE_S390_BUS.
>
> I think that would be even more confusing, as this is not the only bus
> on s390 :)

I suppose you mean S390 has a few bus types and this one is associated with PCI.

>
> I have trouble thinking of a better name, though. TYPE_S390PCI_BUS?
>

This would give a hint that we are referring to a special "S390PCI" bus, but is less readable.
I like the previous name better :)
Maybe TYPE_ZPCI_BUS ? Anyway, maybe just being aware of it is enough.

Thanks,
Marcel