create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv
This patch series follows the previously-merged series which added support for
transient mediated devices. This series expands mdev support to include
persistent device definitions. Again, it relies on mdevctl as the backend.
It follows the common libvirt pattern of APIs by adding the following new APIs
for node devices:
- virNodeDeviceDefineXML() - defines a persistent device
- virNodeDeviceUndefine() - undefines a persistent device
- virNodeDeviceCreate() - starts a previously-defined device
It also adds virsh commands mapping to these new APIs: nodedev-define,
nodedev-undefine, and nodedev-start.
Since we rely on mdevctl for the definition of mediated devices, we need a way
to stay up-to-date with devices that are defined by mdevctl (outside of
libvirt). The method for staying up-to-date is currently a little bit crude
due to the fact that mdevctl does not emit any events when new devices are
added or removed. As a workaround, we create a file monitor for the mdevctl
config directory and re-query mdevctl when we detect changes within that
directory. In the future, mdevctl may introduce a more elegant solution.
Changes in v5:
- Rebase to git master
- updated new API version info to 7.2.0
- capture and relay stderr message from mdevctl
- changed to using GHashTable functions directly instead of deprecated
virHash functions
- protected mdevctlMonitors with a mutex
- added a couple patches to fix the 5s delay when defining a new mdev. These
are currently separate patches added to the end of the series, but could be
squashed into the earlier commits if that's preferred.
- various other minor review fixes
Changes in v4:
- rebase to git master
- switch to throwaway thread for querying mdevctl
- fixed a bug when removing devices because I was accidentally using
virHashForEach() instead of virHashForeachSafe()
- use DEFINED/UNDEFINED events instead of STARTED/STOPPED events
- changes related to merging information about mdev devices from both udev a=
nd
mdevctl:
- Re-used the same function to copy extra data from mdevctl regardless of
whether we're processing a udev event or a mdevctl event (recommended by
Erik). This results in slightly more complex handling of the object
lifetimes (see patch 9), but it consolidates some code.
- nodeDeviceDefCopyFromMdevctl() previously only copied the data that was
unique to mdevctl and didn't exist in udev. It now copies additional data
(possibly overwriting some udev). This solves a problem where a device =
is
defined but not active (i.e. we have not gotten any data from udev), and
then changed (e.g. somebody calls 'mdevctl modify' to change the mdev
type), but libvirt was not updating to the new definition.
- fix a bug where we were mistakenly emitting 'update' events for devices th=
at
had not changed
- Added the ability to specify a uuid for an mdev via device XML.
- split some commits into multiple patches
- updated new API version info to 7.1.0
- Fixed a bug reported by Yan Fu which hangs the client when attempting to
destroy a nodedev that is in use by an active vm. See
https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html for
solution suggested by Alex.
- numerous smaller fixes from review findings
changes in v3:
- streamlined tests -- removed some unnecessary duplication
- split out patch to factor out node device name generation function
- split nodeDeviceParseMdevctlChildDevice() into a separate function
- added follow-up patch to remove space-padded alignment in header
- refactored the mdevctl update handling significantly:
- no longer a separate persistent thread that gets signaled by a timer
- now piggybacks onto the existing udev thread and signals the thread in t=
he
same way that the udev event does.
- Daniel suggested spawning a throw-away thread to handle mdevctl updates,
but that introduces the complexity of possibly serializing multiple
throw-away threads (e.g. if we get an 'created' event followed immediate=
ly
by a 'deleted' event, two threads may be spawned and we'd need to ensure
they are properly ordered)
- added virNodeDeviceObjListForEach() and virNodeDeviceObjListRemoveLocked()
to simplify removing devices that are removed from mdevctl.
- coding style fixes
- NOTE: per Erik's request, I experimented with changing the way that mdevctl
commands were generated and tested (e.g. introducing something like
virMdevctlGetCommand(def, MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was
too invasive and awkward and didn't seem worthwhile
Changes in v2:
- rebase to latest git master
Jonathon Jongsma (30):
nodedev: capture and report stderror from mdevctl
tests: remove extra trailing semicolon
nodedev: introduce concept of 'active' node devices
nodedev: Add ability to filter by active state
nodedev: fix docs for virConnectListAllNodeDevices()
nodedev: expose internal helper for naming devices
nodedev: add ability to parse mdevs from mdevctl
nodedev: add ability to list defined mdevs
nodedev: add persistence to virNodeDeviceObj
nodedev: add DEFINED/UNDEFINED lifecycle events
nodedev: add mdevctl devices to node device list
nodedev: add helper functions to remove node devices
nodedev: handle mdevs that disappear from mdevctl
nodedev: Refresh mdev devices when changes are detected
nodedev: add function to generate mdevctl define command
api: add virNodeDeviceDefineXML()
virsh: Add --inactive, --all to nodedev-list
virsh: add nodedev-define command
nodedev: refactor tests to support mdev undefine
api: add virNodeDeviceUndefine()
virsh: Factor out function to find node device
virsh: add nodedev-undefine command
api: add virNodeDeviceCreate()
virsh: add "nodedev-start" command
nodedev: add <uuid> element to mdev caps
nodedev: add ability to specify UUID for new mdevs
nodedev: fix hang when destroying an mdev in use
nodedev: add docs about mdev attribute order
nodedev: factor out function to add mediated devices
nodedev: avoid delay when defining a new mdev
docs/formatnode.html.in | 5 +-
docs/schemas/nodedev.rng | 43 +-
examples/c/misc/event-test.c | 4 +
include/libvirt/libvirt-nodedev.h | 20 +-
src/access/viraccessperm.c | 2 +-
src/access/viraccessperm.h | 6 +
src/conf/node_device_conf.c | 14 +
src/conf/node_device_conf.h | 8 +
src/conf/virnodedeviceobj.c | 147 +++-
src/conf/virnodedeviceobj.h | 24 +
src/driver-nodedev.h | 14 +
src/libvirt-nodedev.c | 141 +++-
src/libvirt_private.syms | 6 +
src/libvirt_public.syms | 7 +
src/node_device/node_device_driver.c | 743 +++++++++++++++++-
src/node_device/node_device_driver.h | 50 +-
src/node_device/node_device_udev.c | 217 ++++-
src/remote/remote_driver.c | 3 +
src/remote/remote_protocol.x | 40 +-
src/remote_protocol-structs | 16 +
src/rpc/gendispatch.pl | 1 +
...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 2 +
...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 +
...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 3 +-
...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 +
...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 +
...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 +
...16_1ca8_49ac_b176_871d16c13076-define.json | 1 +
tests/nodedevmdevctldata/mdevctl-create.argv | 1 +
.../mdevctl-list-defined.argv | 1 +
.../mdevctl-list-multiple.json | 59 ++
.../mdevctl-list-multiple.out.xml | 43 +
.../nodedevmdevctldata/mdevctl-undefine.argv | 1 +
tests/nodedevmdevctltest.c | 232 +++++-
...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 1 +
tools/virsh-nodedev.c | 269 ++++++-
36 files changed, 1935 insertions(+), 193 deletions(-)
create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=
a70e21366-define.argv
create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=
a70e21366-define.json
create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb=
3f14c23d9-define.argv
create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb=
3f14c23d9-define.json
create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871=
d16c13076-define.argv
create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871=
d16c13076-define.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
create mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv
--=20
2.26.2
Just a friendly ping ;) On Tue, 2 Mar 2021 16:30:35 -0600 Jonathon Jongsma <jjongsma@redhat.com> wrote: > This patch series follows the previously-merged series which added > support for transient mediated devices. This series expands mdev > support to include persistent device definitions. Again, it relies on > mdevctl as the backend. > > It follows the common libvirt pattern of APIs by adding the following > new APIs for node devices: > - virNodeDeviceDefineXML() - defines a persistent device > - virNodeDeviceUndefine() - undefines a persistent device > - virNodeDeviceCreate() - starts a previously-defined device > > It also adds virsh commands mapping to these new APIs: nodedev-define, > nodedev-undefine, and nodedev-start. > > Since we rely on mdevctl for the definition of mediated devices, we > need a way to stay up-to-date with devices that are defined by > mdevctl (outside of libvirt). The method for staying up-to-date is > currently a little bit crude due to the fact that mdevctl does not > emit any events when new devices are added or removed. As a > workaround, we create a file monitor for the mdevctl config directory > and re-query mdevctl when we detect changes within that directory. In > the future, mdevctl may introduce a more elegant solution. > > Changes in v5: > - Rebase to git master > - updated new API version info to 7.2.0 > - capture and relay stderr message from mdevctl > - changed to using GHashTable functions directly instead of > deprecated virHash functions > - protected mdevctlMonitors with a mutex > - added a couple patches to fix the 5s delay when defining a new > mdev. These are currently separate patches added to the end of the > series, but could be squashed into the earlier commits if that's > preferred. > - various other minor review fixes > > Changes in v4: > - rebase to git master > - switch to throwaway thread for querying mdevctl > - fixed a bug when removing devices because I was accidentally using > virHashForEach() instead of virHashForeachSafe() > - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events > - changes related to merging information about mdev devices from > both udev a= nd > mdevctl: > - Re-used the same function to copy extra data from mdevctl > regardless of whether we're processing a udev event or a mdevctl > event (recommended by Erik). This results in slightly more complex > handling of the object lifetimes (see patch 9), but it consolidates > some code. > - nodeDeviceDefCopyFromMdevctl() previously only copied the data > that was unique to mdevctl and didn't exist in udev. It now copies > additional data (possibly overwriting some udev). This solves a > problem where a device = is > defined but not active (i.e. we have not gotten any data from > udev), and then changed (e.g. somebody calls 'mdevctl modify' to > change the mdev type), but libvirt was not updating to the new > definition. > - fix a bug where we were mistakenly emitting 'update' events for > devices th= at > had not changed > - Added the ability to specify a uuid for an mdev via device XML. > - split some commits into multiple patches > - updated new API version info to 7.1.0 > - Fixed a bug reported by Yan Fu which hangs the client when > attempting to destroy a nodedev that is in use by an active vm. See > https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html > for solution suggested by Alex. > - numerous smaller fixes from review findings > > changes in v3: > - streamlined tests -- removed some unnecessary duplication > - split out patch to factor out node device name generation function > - split nodeDeviceParseMdevctlChildDevice() into a separate function > - added follow-up patch to remove space-padded alignment in header > - refactored the mdevctl update handling significantly: > - no longer a separate persistent thread that gets signaled by a > timer > - now piggybacks onto the existing udev thread and signals the > thread in t= he > same way that the udev event does. > - Daniel suggested spawning a throw-away thread to handle mdevctl > updates, but that introduces the complexity of possibly serializing > multiple throw-away threads (e.g. if we get an 'created' event > followed immediate= ly > by a 'deleted' event, two threads may be spawned and we'd need > to ensure they are properly ordered) > - added virNodeDeviceObjListForEach() and > virNodeDeviceObjListRemoveLocked() to simplify removing devices that > are removed from mdevctl. > - coding style fixes > - NOTE: per Erik's request, I experimented with changing the way > that mdevctl commands were generated and tested (e.g. introducing > something like virMdevctlGetCommand(def, > MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was too invasive and > awkward and didn't seem worthwhile > > Changes in v2: > - rebase to latest git master > > Jonathon Jongsma (30): > nodedev: capture and report stderror from mdevctl > tests: remove extra trailing semicolon > nodedev: introduce concept of 'active' node devices > nodedev: Add ability to filter by active state > nodedev: fix docs for virConnectListAllNodeDevices() > nodedev: expose internal helper for naming devices > nodedev: add ability to parse mdevs from mdevctl > nodedev: add ability to list defined mdevs > nodedev: add persistence to virNodeDeviceObj > nodedev: add DEFINED/UNDEFINED lifecycle events > nodedev: add mdevctl devices to node device list > nodedev: add helper functions to remove node devices > nodedev: handle mdevs that disappear from mdevctl > nodedev: Refresh mdev devices when changes are detected > nodedev: add function to generate mdevctl define command > api: add virNodeDeviceDefineXML() > virsh: Add --inactive, --all to nodedev-list > virsh: add nodedev-define command > nodedev: refactor tests to support mdev undefine > api: add virNodeDeviceUndefine() > virsh: Factor out function to find node device > virsh: add nodedev-undefine command > api: add virNodeDeviceCreate() > virsh: add "nodedev-start" command > nodedev: add <uuid> element to mdev caps > nodedev: add ability to specify UUID for new mdevs > nodedev: fix hang when destroying an mdev in use > nodedev: add docs about mdev attribute order > nodedev: factor out function to add mediated devices > nodedev: avoid delay when defining a new mdev > > docs/formatnode.html.in | 5 +- > docs/schemas/nodedev.rng | 43 +- > examples/c/misc/event-test.c | 4 + > include/libvirt/libvirt-nodedev.h | 20 +- > src/access/viraccessperm.c | 2 +- > src/access/viraccessperm.h | 6 + > src/conf/node_device_conf.c | 14 + > src/conf/node_device_conf.h | 8 + > src/conf/virnodedeviceobj.c | 147 +++- > src/conf/virnodedeviceobj.h | 24 + > src/driver-nodedev.h | 14 + > src/libvirt-nodedev.c | 141 +++- > src/libvirt_private.syms | 6 + > src/libvirt_public.syms | 7 + > src/node_device/node_device_driver.c | 743 > +++++++++++++++++- src/node_device/node_device_driver.h | > 50 +- src/node_device/node_device_udev.c | 217 ++++- > src/remote/remote_driver.c | 3 + > src/remote/remote_protocol.x | 40 +- > src/remote_protocol-structs | 16 + > src/rpc/gendispatch.pl | 1 + > ...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 2 + > ...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 + > ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 3 +- > ...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 + > ...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 + > ...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 + > ...16_1ca8_49ac_b176_871d16c13076-define.json | 1 + > tests/nodedevmdevctldata/mdevctl-create.argv | 1 + > .../mdevctl-list-defined.argv | 1 + > .../mdevctl-list-multiple.json | 59 ++ > .../mdevctl-list-multiple.out.xml | 43 + > .../nodedevmdevctldata/mdevctl-undefine.argv | 1 + > tests/nodedevmdevctltest.c | 232 +++++- > ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 1 + > tools/virsh-nodedev.c | 269 ++++++- > 36 files changed, 1935 insertions(+), 193 deletions(-) > create mode 100644 > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > a70e21366-define.argv create mode 100644 > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > a70e21366-define.json create mode 100644 > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > 3f14c23d9-define.argv create mode 100644 > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > 3f14c23d9-define.json create mode 100644 > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > d16c13076-define.argv create mode 100644 > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > d16c13076-define.json create mode 100644 > tests/nodedevmdevctldata/mdevctl-create.argv create mode 100644 > tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode 100644 > tests/nodedevmdevctldata/mdevctl-list-multiple.json create mode > 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create > mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv > > --=20 > 2.26.2 > >
On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote: > Just a friendly ping ;) I'm sorry I've been neglecting this for the past 3 weeks or so, I'll dive right into it starting Monday next week, we're entering freeze today anyway. Would you mind resending a rebased version? There were a couple of conflicts wrt to RPC, I fixed them all locally, but it's always better to start the review with fresh content. Thanks, Erik > > On Tue, 2 Mar 2021 16:30:35 -0600 > Jonathon Jongsma <jjongsma@redhat.com> wrote: > > > This patch series follows the previously-merged series which added > > support for transient mediated devices. This series expands mdev > > support to include persistent device definitions. Again, it relies on > > mdevctl as the backend. > > > > It follows the common libvirt pattern of APIs by adding the following > > new APIs for node devices: > > - virNodeDeviceDefineXML() - defines a persistent device > > - virNodeDeviceUndefine() - undefines a persistent device > > - virNodeDeviceCreate() - starts a previously-defined device > > > > It also adds virsh commands mapping to these new APIs: nodedev-define, > > nodedev-undefine, and nodedev-start. > > > > Since we rely on mdevctl for the definition of mediated devices, we > > need a way to stay up-to-date with devices that are defined by > > mdevctl (outside of libvirt). The method for staying up-to-date is > > currently a little bit crude due to the fact that mdevctl does not > > emit any events when new devices are added or removed. As a > > workaround, we create a file monitor for the mdevctl config directory > > and re-query mdevctl when we detect changes within that directory. In > > the future, mdevctl may introduce a more elegant solution. > > > > Changes in v5: > > - Rebase to git master > > - updated new API version info to 7.2.0 > > - capture and relay stderr message from mdevctl > > - changed to using GHashTable functions directly instead of > > deprecated virHash functions > > - protected mdevctlMonitors with a mutex > > - added a couple patches to fix the 5s delay when defining a new > > mdev. These are currently separate patches added to the end of the > > series, but could be squashed into the earlier commits if that's > > preferred. > > - various other minor review fixes > > > > Changes in v4: > > - rebase to git master > > - switch to throwaway thread for querying mdevctl > > - fixed a bug when removing devices because I was accidentally using > > virHashForEach() instead of virHashForeachSafe() > > - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events > > - changes related to merging information about mdev devices from > > both udev a= nd > > mdevctl: > > - Re-used the same function to copy extra data from mdevctl > > regardless of whether we're processing a udev event or a mdevctl > > event (recommended by Erik). This results in slightly more complex > > handling of the object lifetimes (see patch 9), but it consolidates > > some code. > > - nodeDeviceDefCopyFromMdevctl() previously only copied the data > > that was unique to mdevctl and didn't exist in udev. It now copies > > additional data (possibly overwriting some udev). This solves a > > problem where a device = is > > defined but not active (i.e. we have not gotten any data from > > udev), and then changed (e.g. somebody calls 'mdevctl modify' to > > change the mdev type), but libvirt was not updating to the new > > definition. > > - fix a bug where we were mistakenly emitting 'update' events for > > devices th= at > > had not changed > > - Added the ability to specify a uuid for an mdev via device XML. > > - split some commits into multiple patches > > - updated new API version info to 7.1.0 > > - Fixed a bug reported by Yan Fu which hangs the client when > > attempting to destroy a nodedev that is in use by an active vm. See > > https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html > > for solution suggested by Alex. > > - numerous smaller fixes from review findings > > > > changes in v3: > > - streamlined tests -- removed some unnecessary duplication > > - split out patch to factor out node device name generation function > > - split nodeDeviceParseMdevctlChildDevice() into a separate function > > - added follow-up patch to remove space-padded alignment in header > > - refactored the mdevctl update handling significantly: > > - no longer a separate persistent thread that gets signaled by a > > timer > > - now piggybacks onto the existing udev thread and signals the > > thread in t= he > > same way that the udev event does. > > - Daniel suggested spawning a throw-away thread to handle mdevctl > > updates, but that introduces the complexity of possibly serializing > > multiple throw-away threads (e.g. if we get an 'created' event > > followed immediate= ly > > by a 'deleted' event, two threads may be spawned and we'd need > > to ensure they are properly ordered) > > - added virNodeDeviceObjListForEach() and > > virNodeDeviceObjListRemoveLocked() to simplify removing devices that > > are removed from mdevctl. > > - coding style fixes > > - NOTE: per Erik's request, I experimented with changing the way > > that mdevctl commands were generated and tested (e.g. introducing > > something like virMdevctlGetCommand(def, > > MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was too invasive and > > awkward and didn't seem worthwhile > > > > Changes in v2: > > - rebase to latest git master > > > > Jonathon Jongsma (30): > > nodedev: capture and report stderror from mdevctl > > tests: remove extra trailing semicolon > > nodedev: introduce concept of 'active' node devices > > nodedev: Add ability to filter by active state > > nodedev: fix docs for virConnectListAllNodeDevices() > > nodedev: expose internal helper for naming devices > > nodedev: add ability to parse mdevs from mdevctl > > nodedev: add ability to list defined mdevs > > nodedev: add persistence to virNodeDeviceObj > > nodedev: add DEFINED/UNDEFINED lifecycle events > > nodedev: add mdevctl devices to node device list > > nodedev: add helper functions to remove node devices > > nodedev: handle mdevs that disappear from mdevctl > > nodedev: Refresh mdev devices when changes are detected > > nodedev: add function to generate mdevctl define command > > api: add virNodeDeviceDefineXML() > > virsh: Add --inactive, --all to nodedev-list > > virsh: add nodedev-define command > > nodedev: refactor tests to support mdev undefine > > api: add virNodeDeviceUndefine() > > virsh: Factor out function to find node device > > virsh: add nodedev-undefine command > > api: add virNodeDeviceCreate() > > virsh: add "nodedev-start" command > > nodedev: add <uuid> element to mdev caps > > nodedev: add ability to specify UUID for new mdevs > > nodedev: fix hang when destroying an mdev in use > > nodedev: add docs about mdev attribute order > > nodedev: factor out function to add mediated devices > > nodedev: avoid delay when defining a new mdev > > > > docs/formatnode.html.in | 5 +- > > docs/schemas/nodedev.rng | 43 +- > > examples/c/misc/event-test.c | 4 + > > include/libvirt/libvirt-nodedev.h | 20 +- > > src/access/viraccessperm.c | 2 +- > > src/access/viraccessperm.h | 6 + > > src/conf/node_device_conf.c | 14 + > > src/conf/node_device_conf.h | 8 + > > src/conf/virnodedeviceobj.c | 147 +++- > > src/conf/virnodedeviceobj.h | 24 + > > src/driver-nodedev.h | 14 + > > src/libvirt-nodedev.c | 141 +++- > > src/libvirt_private.syms | 6 + > > src/libvirt_public.syms | 7 + > > src/node_device/node_device_driver.c | 743 > > +++++++++++++++++- src/node_device/node_device_driver.h | > > 50 +- src/node_device/node_device_udev.c | 217 ++++- > > src/remote/remote_driver.c | 3 + > > src/remote/remote_protocol.x | 40 +- > > src/remote_protocol-structs | 16 + > > src/rpc/gendispatch.pl | 1 + > > ...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 2 + > > ...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 + > > ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 3 +- > > ...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 + > > ...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 + > > ...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 + > > ...16_1ca8_49ac_b176_871d16c13076-define.json | 1 + > > tests/nodedevmdevctldata/mdevctl-create.argv | 1 + > > .../mdevctl-list-defined.argv | 1 + > > .../mdevctl-list-multiple.json | 59 ++ > > .../mdevctl-list-multiple.out.xml | 43 + > > .../nodedevmdevctldata/mdevctl-undefine.argv | 1 + > > tests/nodedevmdevctltest.c | 232 +++++- > > ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 1 + > > tools/virsh-nodedev.c | 269 ++++++- > > 36 files changed, 1935 insertions(+), 193 deletions(-) > > create mode 100644 > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > > a70e21366-define.argv create mode 100644 > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > > a70e21366-define.json create mode 100644 > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > > 3f14c23d9-define.argv create mode 100644 > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > > 3f14c23d9-define.json create mode 100644 > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > > d16c13076-define.argv create mode 100644 > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > > d16c13076-define.json create mode 100644 > > tests/nodedevmdevctldata/mdevctl-create.argv create mode 100644 > > tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode 100644 > > tests/nodedevmdevctldata/mdevctl-list-multiple.json create mode > > 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create > > mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv > > > > --=20 > > 2.26.2 > > > > >
On Fri, 26 Mar 2021 07:27:46 +0100 Erik Skultety <eskultet@redhat.com> wrote: > On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote: > > Just a friendly ping ;) > > I'm sorry I've been neglecting this for the past 3 weeks or so, I'll > dive right into it starting Monday next week, we're entering freeze > today anyway. Would you mind resending a rebased version? There were > a couple of conflicts wrt to RPC, I fixed them all locally, but it's > always better to start the review with fresh content. > > Thanks, > Erik Sure thing. I'll rebase and send a new series before the end of the day. Perhaps I should also omit the *Ptr typedefs since they're discouraged now? Jonathon > > > > On Tue, 2 Mar 2021 16:30:35 -0600 > > Jonathon Jongsma <jjongsma@redhat.com> wrote: > > > > > This patch series follows the previously-merged series which added > > > support for transient mediated devices. This series expands mdev > > > support to include persistent device definitions. Again, it > > > relies on mdevctl as the backend. > > > > > > It follows the common libvirt pattern of APIs by adding the > > > following new APIs for node devices: > > > - virNodeDeviceDefineXML() - defines a persistent device > > > - virNodeDeviceUndefine() - undefines a persistent device > > > - virNodeDeviceCreate() - starts a previously-defined device > > > > > > It also adds virsh commands mapping to these new APIs: > > > nodedev-define, nodedev-undefine, and nodedev-start. > > > > > > Since we rely on mdevctl for the definition of mediated devices, > > > we need a way to stay up-to-date with devices that are defined by > > > mdevctl (outside of libvirt). The method for staying up-to-date > > > is currently a little bit crude due to the fact that mdevctl does > > > not emit any events when new devices are added or removed. As a > > > workaround, we create a file monitor for the mdevctl config > > > directory and re-query mdevctl when we detect changes within that > > > directory. In the future, mdevctl may introduce a more elegant > > > solution. > > > > > > Changes in v5: > > > - Rebase to git master > > > - updated new API version info to 7.2.0 > > > - capture and relay stderr message from mdevctl > > > - changed to using GHashTable functions directly instead of > > > deprecated virHash functions > > > - protected mdevctlMonitors with a mutex > > > - added a couple patches to fix the 5s delay when defining a new > > > mdev. These are currently separate patches added to the end of the > > > series, but could be squashed into the earlier commits if that's > > > preferred. > > > - various other minor review fixes > > > > > > Changes in v4: > > > - rebase to git master > > > - switch to throwaway thread for querying mdevctl > > > - fixed a bug when removing devices because I was accidentally > > > using virHashForEach() instead of virHashForeachSafe() > > > - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events > > > - changes related to merging information about mdev devices from > > > both udev a= nd > > > mdevctl: > > > - Re-used the same function to copy extra data from mdevctl > > > regardless of whether we're processing a udev event or a mdevctl > > > event (recommended by Erik). This results in slightly more complex > > > handling of the object lifetimes (see patch 9), but it > > > consolidates some code. > > > - nodeDeviceDefCopyFromMdevctl() previously only copied the > > > data that was unique to mdevctl and didn't exist in udev. It now > > > copies additional data (possibly overwriting some udev). This > > > solves a problem where a device = is > > > defined but not active (i.e. we have not gotten any data > > > from udev), and then changed (e.g. somebody calls 'mdevctl > > > modify' to change the mdev type), but libvirt was not updating to > > > the new definition. > > > - fix a bug where we were mistakenly emitting 'update' events for > > > devices th= at > > > had not changed > > > - Added the ability to specify a uuid for an mdev via device XML. > > > - split some commits into multiple patches > > > - updated new API version info to 7.1.0 > > > - Fixed a bug reported by Yan Fu which hangs the client when > > > attempting to destroy a nodedev that is in use by an active vm. > > > See > > > https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html > > > for solution suggested by Alex. > > > - numerous smaller fixes from review findings > > > > > > changes in v3: > > > - streamlined tests -- removed some unnecessary duplication > > > - split out patch to factor out node device name generation > > > function > > > - split nodeDeviceParseMdevctlChildDevice() into a separate > > > function > > > - added follow-up patch to remove space-padded alignment in > > > header > > > - refactored the mdevctl update handling significantly: > > > - no longer a separate persistent thread that gets signaled by > > > a timer > > > - now piggybacks onto the existing udev thread and signals the > > > thread in t= he > > > same way that the udev event does. > > > - Daniel suggested spawning a throw-away thread to handle > > > mdevctl updates, but that introduces the complexity of possibly > > > serializing multiple throw-away threads (e.g. if we get an > > > 'created' event followed immediate= ly > > > by a 'deleted' event, two threads may be spawned and we'd > > > need to ensure they are properly ordered) > > > - added virNodeDeviceObjListForEach() and > > > virNodeDeviceObjListRemoveLocked() to simplify removing devices > > > that are removed from mdevctl. > > > - coding style fixes > > > - NOTE: per Erik's request, I experimented with changing the way > > > that mdevctl commands were generated and tested (e.g. introducing > > > something like virMdevctlGetCommand(def, > > > MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was too invasive and > > > awkward and didn't seem worthwhile > > > > > > Changes in v2: > > > - rebase to latest git master > > > > > > Jonathon Jongsma (30): > > > nodedev: capture and report stderror from mdevctl > > > tests: remove extra trailing semicolon > > > nodedev: introduce concept of 'active' node devices > > > nodedev: Add ability to filter by active state > > > nodedev: fix docs for virConnectListAllNodeDevices() > > > nodedev: expose internal helper for naming devices > > > nodedev: add ability to parse mdevs from mdevctl > > > nodedev: add ability to list defined mdevs > > > nodedev: add persistence to virNodeDeviceObj > > > nodedev: add DEFINED/UNDEFINED lifecycle events > > > nodedev: add mdevctl devices to node device list > > > nodedev: add helper functions to remove node devices > > > nodedev: handle mdevs that disappear from mdevctl > > > nodedev: Refresh mdev devices when changes are detected > > > nodedev: add function to generate mdevctl define command > > > api: add virNodeDeviceDefineXML() > > > virsh: Add --inactive, --all to nodedev-list > > > virsh: add nodedev-define command > > > nodedev: refactor tests to support mdev undefine > > > api: add virNodeDeviceUndefine() > > > virsh: Factor out function to find node device > > > virsh: add nodedev-undefine command > > > api: add virNodeDeviceCreate() > > > virsh: add "nodedev-start" command > > > nodedev: add <uuid> element to mdev caps > > > nodedev: add ability to specify UUID for new mdevs > > > nodedev: fix hang when destroying an mdev in use > > > nodedev: add docs about mdev attribute order > > > nodedev: factor out function to add mediated devices > > > nodedev: avoid delay when defining a new mdev > > > > > > docs/formatnode.html.in | 5 +- > > > docs/schemas/nodedev.rng | 43 +- > > > examples/c/misc/event-test.c | 4 + > > > include/libvirt/libvirt-nodedev.h | 20 +- > > > src/access/viraccessperm.c | 2 +- > > > src/access/viraccessperm.h | 6 + > > > src/conf/node_device_conf.c | 14 + > > > src/conf/node_device_conf.h | 8 + > > > src/conf/virnodedeviceobj.c | 147 +++- > > > src/conf/virnodedeviceobj.h | 24 + > > > src/driver-nodedev.h | 14 + > > > src/libvirt-nodedev.c | 141 +++- > > > src/libvirt_private.syms | 6 + > > > src/libvirt_public.syms | 7 + > > > src/node_device/node_device_driver.c | 743 > > > +++++++++++++++++- src/node_device/node_device_driver.h | > > > 50 +- src/node_device/node_device_udev.c | 217 ++++- > > > src/remote/remote_driver.c | 3 + > > > src/remote/remote_protocol.x | 40 +- > > > src/remote_protocol-structs | 16 + > > > src/rpc/gendispatch.pl | 1 + > > > ...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 2 + > > > ...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 + > > > ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 3 +- > > > ...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 + > > > ...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 + > > > ...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 + > > > ...16_1ca8_49ac_b176_871d16c13076-define.json | 1 + > > > tests/nodedevmdevctldata/mdevctl-create.argv | 1 + > > > .../mdevctl-list-defined.argv | 1 + > > > .../mdevctl-list-multiple.json | 59 ++ > > > .../mdevctl-list-multiple.out.xml | 43 + > > > .../nodedevmdevctldata/mdevctl-undefine.argv | 1 + > > > tests/nodedevmdevctltest.c | 232 +++++- > > > ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 1 + > > > tools/virsh-nodedev.c | 269 ++++++- > > > 36 files changed, 1935 insertions(+), 193 deletions(-) > > > create mode 100644 > > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > > > a70e21366-define.argv create mode 100644 > > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= > > > a70e21366-define.json create mode 100644 > > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > > > 3f14c23d9-define.argv create mode 100644 > > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= > > > 3f14c23d9-define.json create mode 100644 > > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > > > d16c13076-define.argv create mode 100644 > > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= > > > d16c13076-define.json create mode 100644 > > > tests/nodedevmdevctldata/mdevctl-create.argv create mode 100644 > > > tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode > > > 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json create > > > mode 100644 > > > tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create > > > mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv > > > > > > --=20 > > > 2.26.2 > > > > > > > >
On Fri, Mar 26, 2021 at 09:37:34AM -0500, Jonathon Jongsma wrote: > On Fri, 26 Mar 2021 07:27:46 +0100 > Erik Skultety <eskultet@redhat.com> wrote: > > > On Thu, Mar 25, 2021 at 11:49:49AM -0500, Jonathon Jongsma wrote: > > > Just a friendly ping ;) > > > > I'm sorry I've been neglecting this for the past 3 weeks or so, I'll > > dive right into it starting Monday next week, we're entering freeze > > today anyway. Would you mind resending a rebased version? There were > > a couple of conflicts wrt to RPC, I fixed them all locally, but it's > > always better to start the review with fresh content. > > > > Thanks, > > Erik > > Sure thing. I'll rebase and send a new series before the end of the > day. Perhaps I should also omit the *Ptr typedefs since they're > discouraged now? Sure, I'm more than okay with that. Erik
© 2016 - 2024 Red Hat, Inc.