Tested with v6.10.0-283-g1948d4e61e.

1.Can define/start/destroy mdev device successfully;

2.'virsh nodedev-list' has no '--active' option, which is inconsistent with the description in the patch：

# virsh nodedev-list --active
error: command 'nodedev-list' doesn't support option --active

3.virsh client hang when trying to destroy a mdev device which is using by a vm, and after that all 'virsh nodev*' cmds will hang. If restarting llibvirtd after that, libvirtd will hang.

4.Define a mdev device with the uuid specified, but the mdev device defined seems using another uuid. Maybe it make a little confusion about the use of uuid in the xml:

#cat mdev.xml

<device>
  <name>mdev_85531b6d_e5e4_41c1_aa2a_8844717f066a</name>   ****
  <path>/sys/devices/pci0000:00/0000:00:02.0/85531b6d-e5e4-41c1-aa2a-8844717f066a</path>  ***
  <parent>pci_0000_00_02_0</parent>
  <driver>
    <name>vfio_mdev</name>
  </driver>
  <capability type='mdev'>
    <type id='i915-GVTg_V5_8'/>
    <iommuGroup number='0'/>
  </capability>
</device>

# virsh nodedev-define /root/mdev.xml
Node device ***mdev_6cdbaad0_7215_4741_82b9_c51e1a4decda**** defined from /root/mdev.xml

--

Tested by Yan Fu(yafu@redhat.com)

On Fri, Dec 25, 2020 at 10:19 AM Han Han <hhan@redhat.com> wrote:

On Thu, Dec 24, 2020 at 10:15 PM 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.

Trivial suggestions:

1. Mention the bug to be resolved by this series in commit msg: https://bugzilla.redhat.com/show_bug.cgi?id=1699274

2. Add doc of man page for the new virsh commands and options

Since we rely on mdevctl for the definition of ediated 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 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 (21):
  tests: remove extra trailing semicolon
  nodedev: introduce concept of 'active' node devices
  nodedev: Add ability to filter by active state
  nodedev: expose internal helper for naming devices
  nodedev: add ability to list and parse defined mdevs
  nodedev: add STOPPED/STARTED 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: rename dataReady to udevReady
  nodedev: Refresh mdev devices when changes are detected
  api: add virNodeDeviceDefineXML()
  virsh: Add --active, --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
  libvirt-nodedev.h: remove space-padded alignment

 examples/c/misc/event-test.c                  |   4 +
 include/libvirt/libvirt-nodedev.h             |  98 ++--
 src/access/viraccessperm.c                    |   2 +-
 src/access/viraccessperm.h                    |   6 +
 src/conf/node_device_conf.h                   |   9 +
 src/conf/virnodedeviceobj.c                   | 132 ++++-
 src/conf/virnodedeviceobj.h                   |  19 +
 src/driver-nodedev.h                          |  14 +
 src/libvirt-nodedev.c                         | 115 ++++
 src/libvirt_private.syms                      |   4 +
 src/libvirt_public.syms                       |   3 +
 src/node_device/node_device_driver.c          | 538 +++++++++++++++++-
 src/node_device/node_device_driver.h          |  38 ++
 src/node_device/node_device_udev.c            | 308 ++++++++--
 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 |   1 +
 ...19_36ea_4111_8f0a_8c9a70e21366-define.json |   1 +
 ...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             |  39 ++
 tests/nodedevmdevctltest.c                    | 222 +++++++-
 tools/virsh-nodedev.c                         | 276 +++++++--
 30 files changed, 1765 insertions(+), 189 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

--=20
2.26.2