<div dir="ltr">Tested with v6.10.0-283-g1948d4e61e.<div><br></div><div>1.Can define/start/destroy mdev device successfully;</div><div><br></div><div>2.'virsh nodedev-list' has no '--active' option, which is inconsistent with the description in the patch：</div><div># virsh nodedev-list --active<br>error: command 'nodedev-list' doesn't support option --active<br></div><div><br></div><div>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.</div><div><br></div><div>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:</div><div>#cat mdev.xml</div><div><device><br> <name>mdev_85531b6d_e5e4_41c1_aa2a_8844717f066a</name> ****<br> <path>/sys/devices/pci0000:00/0000:00:02.0/85531b6d-e5e4-41c1-aa2a-8844717f066a</path> ***<br> <parent>pci_0000_00_02_0</parent><br> <driver><br> <name>vfio_mdev</name><br> </driver><br> <capability type='mdev'><br> <type id='i915-GVTg_V5_8'/><br> <iommuGroup number='0'/><br> </capability><br></device><br></div><div><br></div><div># virsh nodedev-define /root/mdev.xml <br>Node device ***mdev_6cdbaad0_7215_4741_82b9_c51e1a4decda**** defined from /root/mdev.xml<br></div><div><br></div><div><br></div><div><br></div><div>--</div><div>Tested by Yan Fu(<a href="mailto:yafu@redhat.com">yafu@redhat.com</a>)</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Dec 25, 2020 at 10:19 AM Han Han <<a href="mailto:hhan@redhat.com">hhan@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 24, 2020 at 10:15 PM Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" target="_blank">jjongsma@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This patch series follows the previously-merged series which added support for<br>
transient mediated devices. This series expands mdev support to include<br>
persistent device definitions. Again, it relies on mdevctl as the backend.<br>
<br>
It follows the common libvirt pattern of APIs by adding the following new APIs<br>
for node devices:<br>
- virNodeDeviceDefineXML() - defines a persistent device<br>
- virNodeDeviceUndefine() - undefines a persistent device<br>
- virNodeDeviceCreate() - starts a previously-defined device<br>
<br>
It also adds virsh commands mapping to these new APIs: nodedev-define,<br>
nodedev-undefine, and nodedev-start.<br></blockquote><div>Trivial suggestions:</div><div>1. Mention the bug to be resolved by this series in commit msg: <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1699274" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=1699274</a></div><div>2. Add doc of man page for the new virsh commands and options<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Since we rely on mdevctl for the definition of ediated devices, we need a way<br>
to stay up-to-date with devices that are defined by mdevctl (outside of<br>
libvirt). The method for staying up-to-date is currently a little bit crude<br>
due to the fact that mdevctl does not emit any events when new devices are<br>
added or removed. As a workaround, we create a file monitor for the mdevctl<br>
config directory and re-query mdevctl when we detect changes within that<br>
directory. In the future, mdevctl may introduce a more elegant solution.<br>
<br>
changes in v3:<br>
- streamlined tests -- removed some unnecessary duplication<br>
- split out patch to factor out node device name generation function<br>
- split nodeDeviceParseMdevctlChildDevice() into a separate function<br>
- added follow-up patch to remove space-padded alignment in header<br>
- refactored the mdevctl update handling significantly:<br>
- no longer a separate persistent thread that gets signaled by a timer<br>
- now piggybacks onto the existing udev thread and signals the thread in t=<br>
he<br>
same way that the udev event does.<br>
- Daniel suggested spawning a throw-away thread to handle mdevctl updates,<br>
but that introduces the complexity of possibly serializing multiple<br>
throw-away threads (e.g. if we get an 'created' event followed immediate=<br>
ly<br>
by a 'deleted' event, two threads may be spawned and we'd need to ensure<br>
they are properly ordered)<br>
- added virNodeDeviceObjListForEach() and virNodeDeviceObjListRemoveLocked()<br>
to simplify removing devices that are removed from mdevctl.<br>
- coding style fixes<br>
- NOTE: per Erik's request, I experimented with changing the way that mdevctl<br>
commands were generated and tested (e.g. introducing something like<br>
virMdevctlGetCommand(def, MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was<br>
too invasive and awkward and didn't seem worthwhile<br>
<br>
Changes in v2:<br>
- rebase to latest git master<br>
<br>
Jonathon Jongsma (21):<br>
tests: remove extra trailing semicolon<br>
nodedev: introduce concept of 'active' node devices<br>
nodedev: Add ability to filter by active state<br>
nodedev: expose internal helper for naming devices<br>
nodedev: add ability to list and parse defined mdevs<br>
nodedev: add STOPPED/STARTED lifecycle events<br>
nodedev: add mdevctl devices to node device list<br>
nodedev: add helper functions to remove node devices<br>
nodedev: handle mdevs that disappear from mdevctl<br>
nodedev: rename dataReady to udevReady<br>
nodedev: Refresh mdev devices when changes are detected<br>
api: add virNodeDeviceDefineXML()<br>
virsh: Add --active, --inactive, --all to nodedev-list<br>
virsh: add nodedev-define command<br>
nodedev: refactor tests to support mdev undefine<br>
api: add virNodeDeviceUndefine()<br>
virsh: Factor out function to find node device<br>
virsh: add nodedev-undefine command<br>
api: add virNodeDeviceCreate()<br>
virsh: add "nodedev-start" command<br>
libvirt-nodedev.h: remove space-padded alignment<br>
<br>
examples/c/misc/event-test.c | 4 +<br>
include/libvirt/libvirt-nodedev.h | 98 ++--<br>
src/access/viraccessperm.c | 2 +-<br>
src/access/viraccessperm.h | 6 +<br>
src/conf/node_device_conf.h | 9 +<br>
src/conf/virnodedeviceobj.c | 132 ++++-<br>
src/conf/virnodedeviceobj.h | 19 +<br>
src/driver-nodedev.h | 14 +<br>
src/libvirt-nodedev.c | 115 ++++<br>
src/libvirt_private.syms | 4 +<br>
src/libvirt_public.syms | 3 +<br>
src/node_device/node_device_driver.c | 538 +++++++++++++++++-<br>
src/node_device/node_device_driver.h | 38 ++<br>
src/node_device/node_device_udev.c | 308 ++++++++--<br>
src/remote/remote_driver.c | 3 +<br>
src/remote/remote_protocol.x | 40 +-<br>
src/remote_protocol-structs | 16 +<br>
src/rpc/<a href="http://gendispatch.pl" rel="noreferrer" target="_blank">gendispatch.pl</a> | 1 +<br>
...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 1 +<br>
...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 +<br>
...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 +<br>
...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 +<br>
...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 +<br>
...16_1ca8_49ac_b176_871d16c13076-define.json | 1 +<br>
tests/nodedevmdevctldata/mdevctl-create.argv | 1 +<br>
.../mdevctl-list-defined.argv | 1 +<br>
.../mdevctl-list-multiple.json | 59 ++<br>
.../mdevctl-list-multiple.out.xml | 39 ++<br>
tests/nodedevmdevctltest.c | 222 +++++++-<br>
tools/virsh-nodedev.c | 276 +++++++--<br>
30 files changed, 1765 insertions(+), 189 deletions(-)<br>
create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=<br>
a70e21366-define.argv<br>
create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9=<br>
a70e21366-define.json<br>
create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb=<br>
3f14c23d9-define.argv<br>
create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb=<br>
3f14c23d9-define.json<br>
create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871=<br>
d16c13076-define.argv<br>
create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871=<br>
d16c13076-define.json<br>
create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv<br>
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv<br>
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json<br>
create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml<br>
<br>
--=20<br>
2.26.2<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div>