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

[Daniel P. Berrange]

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'/>

> +        <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.

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.


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 :|