On Tue, Mar 15, 2016 at 04:37:55PM +0000, Daniel P. Berrange wrote:
On Tue, Mar 15, 2016 at 05:23:30PM +0100, Martin Kletzander wrote:If we expose this information, which is one byte in every PCI config file, we let all mgmt apps know whether the device itself is an endpoint or not so it's easier for them to decide whether such device can be passed through into a VM (endpoint) or not (*-bridge). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1317531 Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- docs/schemas/nodedev.rng | 17 ++++++++++ src/conf/node_device_conf.c | 37 +++++++++++++++++++++ src/conf/node_device_conf.h | 2 ++ src/libvirt_private.syms | 3 ++ src/node_device/node_device_udev.c | 5 +++ src/util/virpci.c | 38 ++++++++++++++++++++++ src/util/virpci.h | 12 +++++++ .../pci_0000_00_02_0_header_type.xml | 16 +++++++++ .../pci_0000_00_1c_0_header_type.xml | 22 +++++++++++++ tests/nodedevxml2xmltest.c | 2 ++ 10 files changed, 154 insertions(+) create mode 100644 tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml create mode 100644 tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 744dccdf5fa9..7aec2adf48e9 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -169,6 +169,23 @@ </optional> <optional> + <element name='header'> + <attribute name='type'> + <choice> + <value>endpoint</value> + <value>pci-bridge</value> + <value>cardbus-bridge</value> + </choice> + </attribute>We already have nested <capability> elements in the node device data, and reporting this header type just looks like another set of capabilities to me. ie <capability type='endpoint'/>
But the devices that this is valid for are exactly only PCI ones. And all PCI ones. That's why I included it in capability type='pci'. It's not capability by itself, it's just information extracted from one Byte in PCI config.
+ <optional> + <element name='multifunction'>Your commit message mentioned nothing about adding a <multifunction/> element. It looks tangential to the goal of adding info about the device type, so should be a separate patch.
OK, I should've explained it more clearly. This information is taken from the first bit of the PCI config header type field. That's what I meant in the commit message as exposing all the information we can get from that one Byte as it's part of that. That's also why that element is included *inside* the header element as it's part of that information.
In any case we already have a
<capability type='virt_functions'>
<address domain='0x0000' bus='0x05' slot='0x10' function='0x0'/>
<address domain='0x0000' bus='0x05' slot='0x10' function='0x4'/>
<address domain='0x0000' bus='0x05' slot='0x11' function='0x0'/>
<address domain='0x0000' bus='0x05' slot='0x11' function='0x4'/>
</capability>
which is reported against physical functions. What's missing
is that we omit the capability entirely if no functions are
currently enabled. We also don't report the total number of
functions that are possible. IMHO we should address it those
problems rather than addign a new element.
The 'multifunction' element I added has nothing to do with virtual functions. It merely indicates that the physical device is supposed to be multifunction; as in there is another device with the same address, but different value of 'function' part of that address. I think explaining all this in the commit message would be enough as I believe the patch is still correct. Let me know if that's fine or if I misunderstood, thanks.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list