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

[Nikolay Shirokovskiy]

On 21.03.2019 14:32, Erik Skultety wrote:
> 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.

I found https://wiki.osdev.org/PCI. So looks like 0x000000 should be 
treated as 'Unclassified/Non-VGA-Compatible devices'.

Nikolay

> 
> The change makes sense regardless, so:
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> 
> ...but I'm still curious
> 
> Erik
>