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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list