[libvirt PATCH 1/6] conf: add support for <acpi index='NNN'/> for PCI devices

[Laine Stump]

On 4/7/21 8:40 AM, Daniel P. Berrangé wrote:
> On Wed, Apr 07, 2021 at 09:23:50AM +0200, Peter Krempa wrote:
>> On Wed, Apr 07, 2021 at 09:17:36 +0200, Peter Krempa wrote:
>>> On Tue, Apr 06, 2021 at 16:31:32 +0100, Daniel Berrange wrote:
>>>> PCI devices can be associated with a unique integer index that is
>>>> exposed via ACPI. In Linux OS with systemd, this value is used for
>>>> provide a NIC device naming scheme that is stable across changes
>>>> in PCI slot configuration.
>>>>
>>>> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>>>> ---
>>>>   docs/formatdomain.rst         |  6 +++
>>>>   docs/schemas/domaincommon.rng | 73 +++++++++++++++++++++++++++++++++++
>>>>   src/conf/device_conf.h        |  3 ++
>>>>   src/conf/domain_conf.c        | 12 ++++++
>>>>   4 files changed, 94 insertions(+)
>>>>
>>>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>>>> index 7ba32ea9c1..5db0aac77a 100644
>>>> --- a/docs/formatdomain.rst
>>>> +++ b/docs/formatdomain.rst
>>>> @@ -4363,6 +4363,7 @@ Network interfaces
>>>>          <mac address='52:54:00:5d:c7:9e'/>
>>>>          <boot order='1'/>
>>>>          <rom bar='off'/>
>>>> +       <acpi index='4'/>
>>>>        </interface>
>>>>      </devices>
>>>>      ...
>>>> @@ -4389,6 +4390,11 @@ when it's in the reserved VMware range by adding a ``type="static"`` attribute
>>>>   to the ``<mac/>`` element. Note that this attribute is useless if the provided
>>>>   MAC address is outside of the reserved VMWare ranges.
>>>>   
>>>> +:since:`Since 7.3.0`, one can set the ACPI index against network interfaces.
>>>> +With some operating systems (eg Linux with systemd), the ACPI index is used
>>>> +to provide network interface device naming, that is stable across changes
>>>> +in PCI addresses assigned to the device.
>>>
>>> Any range limits or uniqueness requirements worth mentioning?
>>
>> QEMU / ACPI spec seems to be enforcing unique indexes:
>>
>> commit 4fd7da4c0336c8fd822cd808d62f7ff8c9936aef
>> Author: Igor Mammedov <imammedo at redhat.com>
>> Date:   Mon Mar 15 14:00:59 2021 -0400
>>
>>      pci: acpi: ensure that acpi-index is unique
>>
>>      it helps to avoid device naming conflicts when guest OS is
>>      configured to use acpi-index for naming.
>>      Spec ialso says so:
>>
>>      PCI Firmware Specification Revision 3.2
>>      4.6.7.  _DSM for Naming a PCI or PCI Express Device Under Operating Systems
>>      "
>>      Instance number must be unique under \_SB scope. This instance number does not have to
>>      be sequential in a given system configuration.
>>      "
>>
>>
>> The code isn't checking whether they are declared as unique.
> 
> IMHO libvirt doesn't need to duplicate checking that is already done
> by QEMU for this.

I agree with you (I think a lot of checks like that are just adding bulk 
to libvirt for no real value, since eventually the error would be 
revealed and reported anyway), but do want to point out that libvirt's 
checking would be done at the time that the domain is defined, but QEMU 
wouldn't be checking until runtime. Of course, from the PoV of a 
management application, domain definition time is "too late" anyway - 
they should be doing their own checking for unique indexes in order to 
give feedback to the user at a time when the topic is fresh in their 
mind and they can easily do something about it (i.e. while they're 
sitting in a "create domain definition" dialog box (probably called 
something else, but you get the idea))

So after my internal conversation on the topic (see above) I agree with 
you even more - why add a *third* check for uniqueness in between the 
other two?