[libvirt] [PATCH] expose SR IOV physical/virtual function relationships

[Dave Allan]

Daniel P. Berrange wrote:
> On Tue, Dec 01, 2009 at 12:03:49PM -0500, Dave Allan wrote:
>> Daniel P. Berrange wrote:
>>> On Tue, Dec 01, 2009 at 10:33:08AM -0500, Dave Allan wrote:
>>>> Attached is a patch that exposes the relationships between physical and 
>>>> virtual functions on SR IOV capable devices.
>>>>
>>>
>>>> +            if (data->pci_dev.physical_function) {
>>>> +                virBufferEscapeString(&buf, "    
>>>> <physical_function>%s</physical_function>\n",
>>>> +                                      data->pci_dev.physical_function);
>>>> +            }
>>>> +            if (data->pci_dev.num_virtual_functions > 0) {
>>>> +                for (i = 0 ; i < data->pci_dev.num_virtual_functions ; 
>>>> i++) {
>>>> +                    virBufferEscapeString(&buf, "    
>>>> <virtual_function>%s</virtual_function>\n",
>>>> +                                          
>>>> data->pci_dev.virtual_functions[i]);
>>>> +                }
>>>> +            }
>>>
>>> What is the actual content of those two new elements ?  Are they
>>> the names of the corresponding device, suitble for a 
>>> virNodeDevLooupByName() ?
>> They are the PCI device BDF or config address, e.g.:
>>
>>     <virtual_function>0000:01:10.0</virtual_function>
>>
>> so, no, they aren't suitable for lookup by name.  Using the names of the 
>> corresponding devices was my first attempt, but there is a dependency 
>> problem there.  At the time that we process one device, the others 
>> aren't necessarily created in libvirt, so it's not possible to look them 
>> up.  If we wanted to go that route, we'd have to do additional work at 
>> the time of dumping the xml, which I think is a little kludgy.  I'd 
>> rather we created an API to lookup a libvirt device by its BDF.
> 
> I don't much like including the raw BDF format, because it is effectively
> adding a 3rd way of specifying PCI addresses in libvirt XML. We already
> have 2 different ways, which is 1 too many
> 
> Node dev XML
> 
>   <capability type='pci'>
>     <domain>0</domain>
>     <bus>0</bus>
>     <slot>31</slot>
>     <function>1</function>
>   </capability>
> 
> 
> Domain XML for host devices & guest addresses
> 
>   <address domain='0x0000' bus='0x00' slot='0x1f' function='0x5'/>
> 
> Yes, my bad for screwing up the nodedev  XML format when I designed
> it, stupidly choosing decimal instead of hex :-(
> 
> I think we should make the virtual/physical function follow the domain
> XML style, with an <address/> element. Perhaps also use nested capabilities
> in the way we did for NPIV host/vport stuff
> 
>   <capability type='physical_function'>
>     <address domain='0x0000' bus='0x00' slot='0x1f' function='0x1'/>
>   </capability>
> 
>   <capability type='virtual_functions'>
>     <address domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
>     <address domain='0x0000' bus='0x00' slot='0x1f' function='0x3'/>
>     <address domain='0x0000' bus='0x00' slot='0x1f' function='0x4'/>
>   </capability>
> 
> 
> I'd even be inclined to retro-fit the existing <capability type='pci'>
> XML to duplicate the address info in this saner format eg
> 
>   <capability type='pci'>
>     <address domain='0x0000' bus='0x00' slot='0x1f' function='0x1' />
>     <domain>0</domain>
>     <bus>0</bus>
>     <slot>31</slot>
>     <function>1</function>
>   </capability>
> 
> 
> So, a PCI card with SR-IOV would end up looking like
> 
>   <capability type='pci'>
>     <domain>0</domain>
>     <bus>0</bus>
>     <slot>31</slot>
>     <function>1</function>
>     <address domain='0x0000' bus='0x00' slot='0x1f' function='0x1' />
> 
>     <capability type='virtual_functions'>
>       <address domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
>       <address domain='0x0000' bus='0x00' slot='0x1f' function='0x3'/>
>       <address domain='0x0000' bus='0x00' slot='0x1f' function='0x4'/>
>     </capability>
>   </capability>
> 
> The nice thing about this kind of nesting is that it would let us show
> that a card is capable of exposing virtual functions, even if it does
> not currently have any enabled
> 
> 
> Regards,
> Daniel

Here's an updated patch reflecting the above suggestions.

Dave

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Add-SR-IOV-physical-and-virtual-function-relationshi.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091204/f79fe299/attachment-0001.ksh>