[libvirt] [PATCH 2/2] nodedev: Expose PCI header type

[Martin Kletzander]

On Thu, Mar 17, 2016 at 10:00:36AM +0000, Daniel P. Berrange wrote:
>On Wed, Mar 16, 2016 at 10:46:10AM +0100, Martin Kletzander wrote:
>> 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 at redhat.com>
>> >>---
>> >> 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.
>
>We have nested capabilities already. This would be a nested capability
>below the main PCI capability, which scopes it to just PCI devices.
>
>> >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.
>
>That doesn't really seem to add any value then since an application can
>already see there are multiple functions in the same slot via the address
>info we provide
>

If that's always the case, then yes.  And I don't know about any
occasion when that wouldn't be true, so I'll remove that in v2.

>
>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 :|
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160317/5a3ecff8/attachment-0001.sig>