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@redhat.com>
>
>
> Michal