On Tue, Jan 5, 2021 at 11:50 AM Yan Fu <yafu@xxxxxxxxxx> wrote:
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 --active3.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.
Please provide the domain xml and the libvirtd log by the filter "1:qemu 1:node_device"
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@xxxxxxxxxx)
It should be:
Tested-by: Yan Fu <yafu@xxxxxxxxxx>
That means it works for me. If you think there are problems here, the tested-by should not be given.
On Fri, Dec 25, 2020 at 10:19 AM Han Han <hhan@xxxxxxxxxx> wrote:On Thu, Dec 24, 2020 at 10:15 PM Jonathon Jongsma <jjongsma@xxxxxxxxxx> 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=16992742. 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
