[libvirt] [PATCH] xml: nodedev: make pci capability class element optional

[Erik Skultety]

On Thu, Mar 21, 2019 at 10:27:14AM +0300, Nikolay Shirokovskiy wrote:
> Commit 3bd4ed46 introduced this element as required which
> breaks backcompat for test driver. Let's make the element optional.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  docs/formatnode.html.in                            |  2 +-
>  docs/schemas/nodedev.rng                           | 12 ++++++-----
>  src/conf/node_device_conf.c                        | 23 +++++++++++-----------
>  src/conf/node_device_conf.h                        |  2 +-
>  src/node_device/node_device_udev.c                 |  2 +-
>  .../pci_0000_02_10_7_sriov_pf_vfs_all.xml          |  1 -
>  6 files changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
> index 5095b97..c2a8f8f 100644
> --- a/docs/formatnode.html.in
> +++ b/docs/formatnode.html.in
> @@ -71,7 +71,7 @@
>              include:
>              <dl>
>                <dt><code>class</code></dt>
> -              <dd>Combined class, subclass and
> +              <dd>Optional element for combined class, subclass and
>                  programming interface codes as 6-digit hexadecimal number.
>                  <span class="since">Since 5.2.0</span></dd>
>                <dt><code>domain</code></dt>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 0f45d79..fe6ffa0 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -133,11 +133,13 @@
>        <value>pci</value>
>      </attribute>
>
> -    <element name='class'>
> -      <data type="string">
> -        <param name="pattern">0x[0-9a-fA-F]{6}</param>
> -      </data>
> -    </element>
> +    <optional>
> +      <element name='class'>
> +        <data type="string">
> +          <param name="pattern">0x[0-9a-fA-F]{6}</param>
> +        </data>
> +      </element>
> +    </optional>
>      <element name='domain'>
>        <ref name='unsignedLong'/>
>      </element>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 19c601a..b96d10c 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -208,7 +208,8 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
>  {
>      size_t i;
>
> -    virBufferAsprintf(buf, "<class>0x%.6x</class>\n", data->pci_dev.klass);
> +    if (data->pci_dev.klass >= 0)
> +        virBufferAsprintf(buf, "<class>0x%.6x</class>\n", data->pci_dev.klass);

Can 0 be a valid class? I mean semantically, can't 0 mean something like "no
class"? I'm just wondering, whether we could report 0x000000, provided it
doesn't denote a valid class already, instead of omitting the <class> element
on the output, but then again, from mgmt app POV it doesn't make any difference
to check whether the <class> element is missing or it reports 0 for devices
that have no class specified.

Do you have some further reading on the classes? I didn't manage to find
anything that would help me decode the hex value.

The change makes sense regardless, so:
Reviewed-by: Erik Skultety <eskultet at redhat.com>

...but I'm still curious

Erik