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