[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 04/12] pci: new iommu_group functions

On 06/25/2013 06:34 AM, Daniel P. Berrange wrote:
> On Mon, Jun 24, 2013 at 11:05:30PM -0400, Laine Stump wrote:
>> Any device which belongs to an "IOMMU group" (used by vfio) will
>> have links to all devices of its group listed in
>> /sys/bus/pci/$device/iommu_group/devices;
>> /sys/bus/pci/$device/iommu_group is actually a link to
>> /sys/kernel/iommu_groups/$n, where $n is the group number (there
>> will be a corresponding device node at /dev/vfio/$n once the
>> devices are bound to the vfio-pci driver)
>> The following functions are added:
> Naming convention is normally <object><action>, which I would
> interpret here as 'virPCIDeviceIOMMUGroup' as the object,
> since you're passing a 'virPCIDevice' or 'virPCIDeviceAddress'
> type as parameter.
>> virPCIDeviceGetIOMMUGroupList
>>   Gets a virPCIDeviceList with one virPCIDeviceList for each device
>>   in the same IOMMU group as the provided virPCIDevice (a copy of the
>>   original device object is included in the list.
> ack to this name
>> virPCIIOMMUGroupIterate
>>   Calls the function @actor once for each device in the group that
>>   contains the given virPCIDeviceAddress.
> virPCIDeviceIOMMUGroupIterate

Well, actually it would be virPCIDeviceAddressIOMMUGroupIterate, which
is really quite a mouthful (and leads to very long lines). Due to the
extreme length, and based on seeing some other function that had just a
"virPCI" prefix, I chose the shorter name (although I secretly wondered
if you would raise an objection).

I definitely see the advantage of consistent naming, even if it's not
(yet) done consistently across all functions, so I'll change these names.

>> virPCIGetIOMMUGroupAddresses
>>   Fills in a virPCIDeviceAddressPtr * with an array of
>>   virPCIDeviceAddress, one for each device in the iommu group of the
>>   provided virPCIDeviceAddress (including a copy of the original).
> virPCIDeviceGetIOMMUGroupAddresses


>> virPCIGetIOMMUGroupNum
>>   Returns the group number as an int (a valid group number will always
>>   be 0 or greater).  If there is no iommu_group link in the device's
>>   directory (usually indicating that vfio isn't loaded), -2 will be
>>   returned. On any real error, -1 will be returned.
> virPCIDeviceGetIOMMUGroupNum


Going through all this, I've noticed that we *still* have multiple "pci
address" objects. For example virDevicePCIAddress (defined in /conf,
includes a "multifunction" attribute) and virPCIDeviceAddress (defined
in /util, no "multifunction"), and that many larger objects that include
a PCI address just define the individual attributes instead of using one
of the predefined objects. This leads to code that constructs one flavor
of PCI address/device from another just to call a function that requires
the other flavor. That *really* needs cleaning up...

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]