Re: [libvirt PATCH v3 1/1] Add a PCI/PCIe device VPD Capability
On Fri, Sep 17, 2021 at 13:54:49 +0300, Dmitrii Shcherbakov wrote:
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 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.
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov(a)canonical.com>
---
This is a quick note, not a full review:
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 | 258 ++++++
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 | 60 ++
src/util/virpci.h | 3 +
src/util/virpcivpd.c | 771 +++++++++++++++++
src/util/virpcivpd.h | 106 +++
src/util/virpcivpdpriv.h | 42 +
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 | 51 ++
tests/testutils.h | 6 +
tests/virpcitest.c | 3 +
tests/virpcivpdtest.c | 777 ++++++++++++++++++
tools/virsh-nodedev.c | 3 +
27 files changed, 2262 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
Firstly your patch is massive and it looks to me as it can be properly
split into multiple smaller chunks as our contributor guidelines ask
for. [1]
Additionally few of the functions I had a look at don't conform to our
coding standard at least in term of comment usage, function naming and
indentation.
Lastly it doesn't pass the test suite (ninja test). Note that once
you'll split the patch, the testsuite must pass after every commit.
[1] https://www.libvirt.org/hacking.html
https://www.libvirt.org/best-practices.html
https://www.libvirt.org/coding-style.html