[PATCH] qemu: Check if unpriv_sgio is already set before trying to set it
by Michal Privoznik
In case when libvirt runs inside a restricted container it may
not have enough permissions to modify unpriv_sgio. However, it
may have been set beforehand by sysadmin or an orchestration
tool. Therefore, let's check whether the currently set value is
the one we want and if it is refrain from writing to the file.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2010306
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_conf.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 916a3d36ee..0451bc70ac 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1878,9 +1878,17 @@ qemuSetUnprivSGIO(virDomainDeviceDef *dev)
* whitelist is enabled. But if requesting unfiltered access, always call
* virSetDeviceUnprivSGIO, to report an error for unsupported unpriv_sgio.
*/
- if ((virFileExists(sysfs_path) || val == 1) &&
- virSetDeviceUnprivSGIO(path, NULL, val) < 0)
- return -1;
+ if (virFileExists(sysfs_path) || val == 1) {
+ int curr_val;
+
+ if (virGetDeviceUnprivSGIO(path, NULL, &curr_val) < 0)
+ return -1;
+
+ if (curr_val != val &&
+ virSetDeviceUnprivSGIO(path, NULL, val) < 0) {
+ return -1;
+ }
+ }
return 0;
}
--
2.32.0
3 years, 1 month
[PATCH] qemu: capabilities: rename piix4-acpi-root-hotplug-en to more appropriate name
by Ani Sinha
The capability name piix4-acpi-root-hotplug-en is not conventional and
appreared to be confusing to some. "en" suffix is also incorrect as the
capability in qemu is used to both enable and disable hotplug on the pci root
bus on the i440fx. Hence, rename it to piix4.acpi-root-pci-hotplug so that it
is clearer, less confusing and more accurate.
Signed-off-by: Ani Sinha <ani(a)anisinha.ca>
---
src/qemu/qemu_capabilities.c | 2 +-
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 2 +-
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 2 +-
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9d0c96a20c..d7b79ef7c0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -641,7 +641,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"virtio-blk.queue-size", /* QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE */
/* 410 */
- "piix4-acpi-root-hotplug-en", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */
+ "piix4.acpi-root-pci-hotplug", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */
);
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index 1e5833a9f0..834fb86636 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -230,7 +230,7 @@
<flag name='input-linux'/>
<flag name='query-display-options'/>
<flag name='virtio-blk.queue-size'/>
- <flag name='piix4-acpi-root-hotplug-en'/>
+ <flag name='piix4.acpi-root-pci-hotplug'/>
<version>5002000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100243</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
index b54dd8a22e..e9d1a26400 100644
--- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
@@ -238,7 +238,7 @@
<flag name='query-display-options'/>
<flag name='set-action'/>
<flag name='virtio-blk.queue-size'/>
- <flag name='piix4-acpi-root-hotplug-en'/>
+ <flag name='piix4.acpi-root-pci-hotplug'/>
<version>6000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100242</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index 0ad493191d..971d55e0cc 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -240,7 +240,7 @@
<flag name='query-display-options'/>
<flag name='set-action'/>
<flag name='virtio-blk.queue-size'/>
- <flag name='piix4-acpi-root-hotplug-en'/>
+ <flag name='piix4.acpi-root-pci-hotplug'/>
<version>6001000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100243</microcodeVersion>
--
2.25.1
3 years, 1 month
[PATCH 0/5] ch: Fixup object leaks and extend g_auto usage
by William Douglas
Based off of feedback from Laine to Ján's series of patches increasing
g_auto usage for the ch_driver.
This fixes leaks in virCHMonitorBuildKernelRelatedJson and virCHMonitorNew
while tidying up and increasing the automatic cleanup for those functions and
virCHMonitorBuildMemoryJson.
William Douglas (5):
ch_monitor: Stop leaking json value objects
ch_monitor: Correctly close and ref the virCHMonitor
ch: use g_auto in virCHMonitorBuildMemoryJson
ch: use g_auto in virCHMonitorBuildKernelRelatedJson
ch: use g_auto in virCHMonitorNew
src/ch/ch_monitor.c | 59 +++++++++++++++------------------------------
src/ch/ch_monitor.h | 1 +
2 files changed, 20 insertions(+), 40 deletions(-)
--
2.33.0
3 years, 1 month
[PATCH v7 0/4] Add support to enable/disable hotplug on pci-root controller
by Ani Sinha
changelog:
v7: rebased the patches post libvirt release 7.8. Adjusted NEWS.rst to reflect 7.9 (unreleased)
version. Changed version info for new config option in formatdomain.rst as well.
Added reviewed-by tags. Also in formatdomain.rst added information on what type of hotplug
(ACPI/native etc) are affected by the setting.
v6: incorporated Jan's suggestions.
v5: incorporated suggestions from Laine for patches #2 and #3. The qemu command line
is now added in a new function and called from qemuBuildControllersByTypeCommandLine().
Output xmls are now symlinked to input xmls for unit tests.
v4: split the original patchset into a pci-root controller specific patch series.
also the libvirt conf is now a sub-element of the pci-root controller as was
suggested by Dan Berrange. Please see discussion here:
https://listman.redhat.com/archives/libvir-list/2021-September/msg00839.html
v3: reorganized the patches as per Laine's suggestion. Added more
details in commit messages. Added conf description in formatdomain.rst.
Added changelog for next release.
v2: fixed bugs and added additional missing unit tests.
v1: initial implementation. Had some bugs and missed some unit tests
This patchset introduces libvirt xml support to enable/disable hotplug on the
pci-root controller. It adds a 'target' subelement for the pci-root controller
with a 'hotplug' property. This property can be used to enable or disable
hotplug for the pci-root controller. For example, in order to disable hotplug
on the pci-root controller, one has to use set '<target hotplug='off'>' as
shown below:
<controller type='pci' model='pci-root'>
<target hotplug='off'/>
</controller>
'<target hotplug='on'>' option would enable hotplug for pci-root controller.
This is also the default value. This option is only available for pc machine
types and is applicable for qemu only/kvm accelerator onlt.This feature was
introduced from qemu version 5.2 with the following change in qemu repository:
3d7e78aa7777f ("Introduce a new flag for i440fx to disable PCI hotplug on the root bus")
The above qemu commit describes some reasons why users might to disable hotplug
on PCI root buses [1].
The corresponding commandline option to qemu for x86 guests is:
-global PIIX4_PM.acpi-root-pci-hotplug=<off/on>
Notes:
1. The use case scenario described by Laine in
https://listman.redhat.com/archives/libvir-list/2020-February/msg00110.html
intentionally does not discuss i440fx and focusses solely on q35. I do realize
that redhat has moved on from i440fx and currently efforts for new features
are concentrated on q35 machines only. We have had some hard debates on this
on the qemu mailing list before. The fact of the matter is that i440fx is
not at 1-1 parity with q35. There are many users who are currenly using i440fx
and are simply not ready to move to q35 without sacrificing some
existing features they support today. For example
https://wiki.qemu.org/images/4/4e/Q35.pdf lists some of q35 limitations.
https://www.linux-kvm.org/images/0/06/2012-forum-Q35.pdf provides more
information on the differences. Hence we need to solve the issue Laine has
described in the above email for i440fx without adding additional bridges.
Further, in Daniel Berrange's words from :
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg03012.html
"From the upstream POV, there's been no decision / agreement to phase
out PIIX, this is purely a RHEL downstream decision & plan. If other
distros / users have a different POV, and find the feature useful, we
should accept the patch if it meets the normal QEMU patch requirements.
"
Also to be noted that I have already experimented this qemu commandline option
using libvirt passthrough feature as has been documented in
http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html
This was only meant to be a short term solution until libvirt started
supporting this natively. Supporting this option through libvirt would simplify
their use case as well as add capability validations
and graceful failure scenarios in case qemu did not support the option.
Ani Sinha (4):
qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx
machines
conf: introduce option to enable/disable pci hotplug on pci-root
controller
qemu: command: add support to enable/disable hotplug on pci-root
controller
NEWS: release note the new hotplug enable/disable option on pci-root
controller
NEWS.rst | 6 ++++
docs/formatdomain.rst | 14 ++++++---
src/qemu/qemu_capabilities.c | 4 +++
src/qemu/qemu_capabilities.h | 3 ++
src/qemu/qemu_command.c | 17 ++++++++++
src/qemu/qemu_validate.c | 9 +++++-
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 1 +
.../pc-i440fx-acpi-root-hotplug-disable.args | 31 +++++++++++++++++++
.../pc-i440fx-acpi-root-hotplug-disable.err | 1 +
.../pc-i440fx-acpi-root-hotplug-disable.xml | 30 ++++++++++++++++++
.../pc-i440fx-acpi-root-hotplug-enable.xml | 30 ++++++++++++++++++
tests/qemuxml2argvtest.c | 3 ++
.../pc-i440fx-acpi-root-hotplug-disable.xml | 1 +
.../pc-i440fx-acpi-root-hotplug-enable.xml | 1 +
tests/qemuxml2xmltest.c | 4 +++
17 files changed, 151 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.args
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.err
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-enable.xml
--
2.25.1
3 years, 1 month
[PATCH] NEWS: cosmetic - fix indentation
by Ani Sinha
The indentation of the first item under the categoty "new features" for the
future release v7.9.0 is not right. Fix it.
Signed-off-by: Ani Sinha <ani(a)anisinha.ca>
---
NEWS.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/NEWS.rst b/NEWS.rst
index 76d2375e97..caa61f0646 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -17,7 +17,7 @@ v7.9.0 (unreleased)
* **New features**
- * Introduce virtio-mem ``<memory/>`` model
+ * Introduce virtio-mem ``<memory/>`` model
New virtio-mem model is introduced for ``<memory/>`` device which is a
paravirtualized mechanism of adding/removing memory to/from a VM. Use
--
2.25.1
3 years, 1 month
[PATCH] virsh: Fix --nvram and --keep-nvram help strings
by Michal Privoznik
The --nvram and --keep-nvram options of the undefine command can
be used regardless of the domain status (the only consumer so far
- qemuDomainUndefineFlags() doesn't care about the domain
status). Yet, their corresponding help strings say something
about inactive domains while manpage says nothing. Remove the
reference to domain state.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2007659
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0b78fbf728..f73d9a057e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3595,11 +3595,11 @@ static const vshCmdOptDef opts_undefine[] = {
},
{.name = "nvram",
.type = VSH_OT_BOOL,
- .help = N_("remove nvram file, if inactive")
+ .help = N_("remove nvram file")
},
{.name = "keep-nvram",
.type = VSH_OT_BOOL,
- .help = N_("keep nvram file, if inactive")
+ .help = N_("keep nvram file")
},
{.name = NULL}
};
--
2.32.0
3 years, 1 month
[PATCH 0/1] vmx: Fix <genid/> mapping
by Michal Privoznik
Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it
was brought up in a private thread that libvirt doesn't report correct
UUIDs. For instance for the following input:
vm.genid = "-8536691797830587195"
vm.genidX = "-1723453263670062919"
Libvirt would report:
<genid>8987940a-0951-2cc5-e815-10634ff550b9</genid>
while virt-v2v would report:
<genid>09512cc5-940a-8987-b950-f54f631015e8</genid>
Another example:
vm.genid = "-6284418052148993891"
vm.genidX = "-649955058627554545"
Libvirt:
<genid>a8c94313-ed9b-609d-f6fa-e5515a89530f</genid>
virt-v2v:
<genid>ed9b609d-4313-a8c9-0f53-895a51e5faf6</genid>
To test my patch I modified tests/vmx2xmldata/esx-in-the-wild-10.vmx
(because it already contains vmx.genid, but any file can be modified
really), and then ran:
libvirt.git/_build/tests $ VIR_TEST_DEBUG=1 VIR_TEST_RANGE=58 ./vmx2xmltest
Mind you, the fix is almost direct rewrite of virt-v2v's algorithm,
except it's optimized for C and not OCaml :-) You can find various
attempts/versions of that on my gitlab:
https://gitlab.com/MichalPrivoznik/libvirt/-/commits/vmx_genid/
I'm sending only the last one because that looks the best IMO.
Michal Prívozník (1):
vmx: Fix <genid/> mapping
src/vmx/vmx.c | 16 ++++++++++++----
tests/vmx2xmldata/esx-in-the-wild-10.xml | 2 +-
2 files changed, 13 insertions(+), 5 deletions(-)
--
2.32.0
3 years, 1 month
[PATCH] docs: describe flag VIR_STORAGE_POOL_CREATE_NORMAL to correct the HTML doc
by Erik Skultety
From: Robin Lee <cheeselee(a)fedoraproject.org>
This patch makes the descriptions of virStoragePoolCreateFlags annotate to the
correct flag in the generated HTML file.
---
Resending a merged change from GitLab.
include/libvirt/libvirt-storage.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
index b459fe3e8e..f89856b93e 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -70,6 +70,7 @@ typedef enum {
} virStoragePoolDeleteFlags;
typedef enum {
+ /* Create the pool but do not perform pool build */
VIR_STORAGE_POOL_CREATE_NORMAL = 0,
/* Create the pool and perform pool build without any flags */
--
2.31.1
3 years, 1 month
[PATCH v5 0/4] introduce support for acpi-bridge-hotplug feature
by Ani Sinha
changelog:
v5: rebased the patchset with the latest master.
v4: split the original series into two - pci-root controller specific one
(https://www.mail-archive.com/libvir-list@redhat.com/msg221645.html)
and this one specific to pci bridges.
The conf xml has been introduced as per suggestion by Berrange here:
https://patchew.org/Libvirt/20210912032631.2853520-1-ani@anisinha.ca
Changes has been introduced to parse and validate the xml accordingly
as well as to add backend qemu commandline option.
v3: reorganized the patches as per Laine's suggestion. Added more
details in commit messages. Added conf description in formatdomain.rst.
Added changelog for next release.
v2: fixed bugs and added additional missing unit tests.
v1: initial implementation. Had some bugs and missed some unit tests
This change introduces a new libvirt sub-element <pci> under <features> that
can be used to configure all pci related features.
Currently the only sub-sub element supported by this sub-element is
'acpi-bridge-hotplug' as shown below:
<features>
<pci>
<acpi-bridge-hotplug state='on|off'/>
</pci>
</features>
The above option is only available for qemu driver and that too for x86 guests
only. It is a global option.
'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
(pci-bridge controller) or PCIe-PCI bridges for pc machines and
pcie-root-port controller for q35 machines. Being global option, no other
bridge specific option are required. For pc machine type in x86, this option
is available in qemu for a long time, from version 2.1.
Please see the following changes in qemu repo:
9e047b982452c6 ("piix4: add acpi pci hotplug support")
133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled")
For q35 machine type, this was introduced in qemu 6.1 with the following
changes in qemu repo:
(a) c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
(b) 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
The reasons for enabling ACPI based hotplug for PCIe (q35) based machines (as
opposed to native hotplug) are outlined in (b). There are use cases where users
would still want to use native hotplug (see notes). Therefore, this config option
enables users to choose either ACPI based hotplug or native hotplug for bridges
(for example for pcie root port controller in q35 machines).
Notes:
One concrete example of why one might still want to use native hotplug with
pcie-root-port controller is the fact that we are still discovering issues with
acpi hotplug on PCIE. One such issue is:
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02146.html
Another reason is that users have been using native hotplug on pcie root ports
up until now. They have built and tested their systems based on native hotplug.
They may not want to suddenly move to acpi based hotplug just because it is now
the default in qemu. Supporting the option to chose one or the other through
libvirt makes things simpler for end users.
Ani Sinha (4):
qemu: capablities: detect presence of
acpi-pci-hotplug-with-bridge-support
conf: introduce support for acpi-bridge-hotplug feature
qemu: command: add support for acpi-bridge-hotplug feature
NEWS: add new acpi pci hotplug config option in the release note for
next release
NEWS.rst | 7 ++
docs/formatdomain.rst | 11 +++
docs/schemas/domaincommon.rng | 15 ++++
src/conf/domain_conf.c | 89 ++++++++++++++++++-
src/conf/domain_conf.h | 9 ++
src/qemu/qemu_capabilities.c | 4 +
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c | 14 +++
src/qemu/qemu_validate.c | 46 ++++++++++
.../caps_2.11.0.x86_64.xml | 1 +
.../caps_2.12.0.x86_64.xml | 1 +
.../caps_3.0.0.x86_64.xml | 1 +
.../caps_3.1.0.x86_64.xml | 1 +
.../caps_4.0.0.x86_64.xml | 1 +
.../caps_4.1.0.x86_64.xml | 1 +
.../caps_4.2.0.x86_64.xml | 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 2 +
.../aarch64-acpi-hotplug-bridge-disable.err | 1 +
.../aarch64-acpi-hotplug-bridge-disable.xml | 33 +++++++
...pc-i440fx-acpi-hotplug-bridge-disable.args | 31 +++++++
.../pc-i440fx-acpi-hotplug-bridge-disable.err | 1 +
.../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++++++
.../pc-i440fx-acpi-hotplug-bridge-enable.xml | 33 +++++++
.../q35-acpi-hotplug-bridge-disable.args | 33 +++++++
.../q35-acpi-hotplug-bridge-disable.err | 1 +
.../q35-acpi-hotplug-bridge-disable.xml | 47 ++++++++++
.../q35-acpi-hotplug-bridge-enable.xml | 47 ++++++++++
tests/qemuxml2argvtest.c | 16 ++++
.../pc-i440fx-acpi-hotplug-bridge-disable.xml | 1 +
.../pc-i440fx-acpi-hotplug-bridge-enable.xml | 1 +
.../q35-acpi-hotplug-bridge-disable.xml | 1 +
.../q35-acpi-hotplug-bridge-enable.xml | 1 +
tests/qemuxml2xmltest.c | 14 +++
37 files changed, 503 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml
--
2.25.1
3 years, 1 month
[libvirt PATCH v5 0/7] Add a PCI/PCIe device VPD Capability
by Dmitrii Shcherbakov
Add support for deserializing the binary PCI/PCIe VPD format and
exposing VPD resources as XML elements in a new nested capability
of PCI/PCIe devices called 'vpd'.
The series contains the following incremental changes:
* The PCI VPD parser module, in-memory resource representation
and tests;
* VPD-related helpers added to the virpci module;
* VPD capability support: XML serialization/deserialization from/into
VPD resource data structures;
* Documentation.
The VPD format is specified in "I.3. VPD Definitions" in PCI specs
(2.2+) and "6.28.1 VPD Format" PCIe 4.0. As section 6.28 in PCIe 4.0
notes, the PCI Local Bus and PCIe VPD formats are binary compatible
and PCIe 4.0 merely started incorporating what was already present in
PCI specs.
Linux kernel exposes a binary blob in the VPD format via sysfs since
v2.6.26 (commit 94e6108803469a37ee1e3c92dafdd1d59298602f) which requires
a parser to interpret.
There are usage scenarios where information such as the board serial
number needs to be retrieved from PCI(e) VPD. Projects like Nova can
utilize this information for cases which involve virtual interface
plugging on SmartNIC DPUs but there may be other scenarios and types of
information useful to retrieve from VPD. The fact that the format is
binary requires proper parsing instead of substring searching hence the
full parser is proposed. Likewise, checksum validation requires proper
parsing as well.
A usage example is present here:
https://review.opendev.org/c/openstack/nova/+/808199
The patch follows a prior discussion on the mailing list which has
additional context about the use-case but a narrower proposal:
https://listman.redhat.com/archives/libvir-list/2021-May/msg00873.html
https://www.mail-archive.com/libvir-list@redhat.com/msg218165.html
The new functionality is mostly contained in virpcivpd with a
couple of new functions added to virpci. Additionally, the necessary XML
serialization/deserialization and glue code is added to expose the VPD
capability to external clients as XML.
A new capability flag is added along with a new capability in order to
allow for filtering of PCI devices with the VPD capability using virsh:
virsh nodedev-list --cap vpd
sudo virsh nodedev-dumpxml --device pci_dddd_bb_ss_f
In this example having the root uid is required in order to access the
vpd sysfs entry, therefore, the nodedev XML output will only contain
the VPD capability if virsh is run as root.
The capability is treated as dynamic due to the presence of read-write
sections in the VPD format per PCI/PCIe specs (the idea being that
read-write resource fields may potentially be altered by the DPU OS
over time independently from the host OS).
Unit tests cover the parser functionality (including many possible
invalid cases), in-memory representation as well as XML serialization
and deserialization.
Manual functional testing was performed with 2 DPUs and several other
NIC models which expose PCI(e) VPD. Testing have also been performed
for devices that do not have VPD or those that expose a VPD capability
but exhibit invalid behavior (I/O errors while reading a sysfs entry).
Per the existing guidelines, the implementation relies heavily on glib
for various purposes.
https://libvirt.org/glib-adoption.html
* v5 fixes a couple of minor typos and adds NEWS entries.
* The v4 of the patch includes a number of fixes compared to v3:
* Fixed the patch to correctly build against older glib (2.56.0);
* Notably, glib commit 86c073dba9d82ef3f1bc3d3116b058b9b5c3b1eb (in
2.59.0) fixes g_autolist support for derivable Glib types. To make
things work in 2.56.0 a workaround is conditionally applied;
* virCreateAnonymousFile now uses a temporary file which is
unlinked after creation instead of memfd because OpenSUSE 15.2 does
not have support memfd;
* Keyword resources now use GTree instead of GHashTable as the
underlying data structure:
* This allows for stable ordering which is important for XML2XML tests
as they were failing with when GLib versions were different,
resulting in a different ordering of elements;
* The keyword resource iteration function was complex and got replaced
by a simpler g_tree_foreach-based approach;
* Added more testing: functions added to virpci are now assessed by
creating a mocked vpd file under a mocked sysfs structure while the
parser is still tested in virpcivpdtest file;
* Refactoring:
* Applied changes based on the indent tool operation with some
post-processing;
* Renamed functions which had the Glib naming style to use camel case
where possible. Auto-generated declarations are an exception:
gobject/gtype.h defines type_name##_init, type_name##_class_init,
module_obj_name##_get_type functions which were left unchanged;
* camelCase is now used for local variables and function parameters;
* Replaced //-style comments with multi-line ones;
* Split out one patch into 4 based on distinct features:
* PCI VPD parser functionality and the respective in-memory types;
* VPD helpers in virpci;
* XML serialization/deserialization and VPD capability support;
* Documentation.
Build & test results for targets in ci/manifest.yaml:
ci/helper test --meson-args='-Dexpensive_tests=enabled' <target>
https://gist.github.com/dshcherb/225b5da9478275f08c220487814ffd1c
Dmitrii Shcherbakov (7):
Add a PCI/PCIe device VPD Parser
News: Add a PCI VPD parser
Add PCI VPD-related helper functions to virpci
News: Add PCI VPD-related helper functions to virpci
Add PCI VPD Capability Support
News: Add PCI VPD capability support
Add PCI VPD Capability Documentation
NEWS.rst | 22 +
build-aux/syntax-check.mk | 4 +-
docs/drvnodedev.html.in | 46 ++
docs/formatnode.html.in | 24 +-
docs/schemas/nodedev.rng | 40 +
include/libvirt/libvirt-nodedev.h | 1 +
po/POTFILES.in | 1 +
src/conf/node_device_conf.c | 271 +++++++
src/conf/node_device_conf.h | 6 +-
src/conf/virnodedeviceobj.c | 7 +-
src/libvirt_private.syms | 17 +
src/node_device/node_device_driver.c | 2 +
src/node_device/node_device_udev.c | 2 +
src/util/meson.build | 1 +
src/util/virpci.c | 62 ++
src/util/virpci.h | 3 +
src/util/virpcivpd.c | 755 ++++++++++++++++++
src/util/virpcivpd.h | 117 +++
src/util/virpcivpdpriv.h | 45 ++
tests/meson.build | 1 +
.../pci_0000_42_00_0_vpd.xml | 33 +
.../pci_0000_42_00_0_vpd.xml | 1 +
tests/nodedevxml2xmltest.c | 1 +
tests/testutils.c | 40 +
tests/testutils.h | 4 +
tests/virpcimock.c | 30 +
tests/virpcitest.c | 64 ++
tests/virpcivpdtest.c | 705 ++++++++++++++++
tools/virsh-nodedev.c | 3 +
29 files changed, 2303 insertions(+), 5 deletions(-)
create mode 100644 src/util/virpcivpd.c
create mode 100644 src/util/virpcivpd.h
create mode 100644 src/util/virpcivpdpriv.h
create mode 100644 tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
create mode 120000 tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml
create mode 100644 tests/virpcivpdtest.c
--
2.30.2
3 years, 1 month
