Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Markus Armbruster]

Paul Brook <address@hidden> writes:

>> Paul Brook <address@hidden> writes:
>> >> The system emulators for each arch are using inconsistent
>> >> naming for the default PCI bus "pci" vs "pci.0". Since it
>> >> is conceivable we'll have multiple PCI buses in the future
>> >> standardize on "pci.0" for all architectures. This ensures
>> >> mgmt apps can rely on a name when assigning PCI devices an
>> >> address on the bus using eg '-device e1000,bus=pci.0,addr=3'
>> > 
>> > No. Bus names are local to the parent device.  None of the host bridges
>> > support multiple bridges, so the ".0" suffix makes no sense.  The parent
>> > device has no idea whether it owns the "default" pci bus or not.
>> > If you have multiple PCI busses then you can identify them by the device
>> > path.
>> 
>> From qbus_create_inplace():
>> 
>>     if (name) {
>>         /* use supplied name */
>>         bus->name = qemu_strdup(name);
>>     } else if (parent && parent->id) {
>>         /* parent device has id -> use it for bus name */
>>         len = strlen(parent->id) + 16;
>>         buf = qemu_malloc(len);
>>         snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
>>         bus->name = buf;
>>     } else {
>>         /* no id -> use lowercase bus type for bus name */
>>         len = strlen(info->name) + 16;
>>         buf = qemu_malloc(len);
>>         len = snprintf(buf, len, "%s.%d", info->name,
>>                        parent ? parent->num_child_bus : 0);
>>         for (i = 0; i < len; i++)
>>             buf[i] = qemu_tolower(buf[i]);
>>         bus->name = buf;
>>     }
>> 
>> If appending ".0" really makes no sense when the device has just one
>> bus, then we shouldn't append it in cases 2 & 3.
>
> IMO the code you quote is completely bogus.  All devices should specify names 
> for their child busses, failure to do so is a bug.
>
> These bus names are local to the parent device. Trying to use them as global 
> identifiers is just plain wrong.  A given device [type] should always have 
> the 
> same set of properties/child busses regardless of where it occurs in the 
> device tree.

I agree.

Case in point: For a bug fix I'm working on, I need to find the drives
connected to an IDE controller.  Should be easy enough: go from
controller qdev to its qbus, iterate over the qdevs on that bus.  I need
to call qdev_get_child_bus() for the first step, obviously.  But what to
pass as name argument?  -EMAGIC.

Note that I *only* object to case 2 (derive bus name from parent
device's ID).  Case 3 (derive bus name from bus type) is fine with me.

> Using a single counter for all busses is also wrong. The order of bus 
> creation 
> is a device implementation detail, under no circumstances should it be part 
> of 
> the user visible API.  Consider a device that has both a PCI and I2C bus. It 
> makes no sense to call there pci.0 and i2c.1.

Yes, that's a bit ugly.