On Wed, 2020-06-10 at 20:01 +0200, Michal Privoznik wrote:
On 6/9/20 11:43 PM, Jonathon Jongsma wrote:
> Apologies for the delay in posting the second version of this
> series.
>
> This is the first portion of an effort to support persistent
> mediated devices
> with libvirt. This first series simply enables creating and
> destroying
> non-persistent mediated devices via the virNodeDeviceCreateXML()
> and
> virNodeDeviceDestroy() functions. The 'mdevctl' utility[1] provides
> the backend
> implementation.
>
> Changes in v2:
> - review findings fixed
> - Added Unit testing
> - passed JSON config via stdin instead of a temporary file (see
> portability
> note in patch 5)
>
> [1]
https://github.com/mdevctl/mdevctl
>
> Jonathon Jongsma (10):
> nodedev: factor out nodeDeviceHasCapability()
> nodedev: add support for mdev attributes
> nodedev: refactor nodeDeviceFindNewDevice()
> nodedev: store mdev UUID in mdev caps
> nodedev: add mdev support to virNodeDeviceCreateXML()
> nodedev: Build a non-loadable driver lib
> nodedev: Add testing for 'mdevctl start'
> nodedev: add mdev support to virNodeDeviceDestroy()
> nodedev: Add testing for 'mdevctl stop'
> docs: note node device fields that are read-only
>
> build-aux/syntax-check.mk | 2 +-
> docs/formatnode.html.in | 16 +-
> docs/schemas/nodedev.rng | 6 +
> libvirt.spec.in | 2 +
> m4/virt-external-programs.m4 | 3 +
> src/conf/node_device_conf.c | 60 ++-
> src/conf/node_device_conf.h | 3 +
> src/conf/virnodedeviceobj.c | 34 ++
> src/conf/virnodedeviceobj.h | 3 +
> src/libvirt_private.syms | 3 +
> src/node_device/Makefile.inc.am | 23 +-
> src/node_device/node_device_driver.c | 377
> ++++++++++++++++--
> src/node_device/node_device_driver.h | 9 +
> src/node_device/node_device_udev.c | 5 +-
> src/util/virmdev.c | 12 +
> src/util/virmdev.h | 11 +
> tests/Makefile.am | 14 +
> ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 +
> ...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 +
> ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 +
> ...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 +
> ...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 +
> ...916_1ca8_49ac_b176_871d16c13076-start.json | 1 +
> tests/nodedevmdevctldata/mdevctl-stop.argv | 1 +
> tests/nodedevmdevctltest.c | 300
> ++++++++++++++
> ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 8 +
> ...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 10 +
> ...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 9 +
> 28 files changed, 862 insertions(+), 55 deletions(-)
> create mode 100644
> tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-
> start.argv
> create mode 100644
> tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-
> start.json
> create mode 100644
> tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-
> start.argv
> create mode 100644
> tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-
> start.json
> create mode 100644
> tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-
> start.argv
> create mode 100644
> tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-
> start.json
> create mode 100644 tests/nodedevmdevctldata/mdevctl-stop.argv
> create mode 100644 tests/nodedevmdevctltest.c
> create mode 100644
> tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.x
> ml
> create mode 100644
> tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.x
> ml
> create mode 100644
> tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.x
> ml
>
Patches look good to me. I've raised couple of small issues (mostly
mem
leaks), but suggested what needs to be squashed in. I'm keeping it
all
in my local branch, ready to push if you agree with my comments (no
need
to send v3 then).
I have no objections to your squashed patches. Thanks for that.
However, as in v1, I'd like either Erik or Dan to skim through
patches,
because I know next to nothing about mdevs.
Conditional:
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal