Re: [libvirt PATCH 1/6] conf: add support for <acpi index='NNN'/> for PCI devices
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(a)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(a)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?