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
>>>
>>>
>>>

Reply via email to