On Tue, Jan 5, 2021 at 11:50 AM Yan Fu <y...@redhat.com> 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 --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. > 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(y...@redhat.com) > It should be: Tested-by: Yan Fu <y...@redhat.com> 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 <h...@redhat.com> wrote: > >> >> >> On Thu, Dec 24, 2020 at 10:15 PM Jonathon Jongsma <jjong...@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 >>> >>> >>>