Re: [libvirt] [PATCH] expose SR IOV physical/virtual function relationships
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
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|