[libvirt PATCH v3 0/7] Enable autostarting mediated devices

Jonathon Jongsma posted 7 patches 2 years, 8 months ago
Failed in applying to current master (apply log)
docs/manpages/virsh.rst                       |  27 +++
include/libvirt/libvirt-nodedev.h             |  10 +
src/conf/node_device_conf.h                   |   1 +
src/conf/virnodedeviceobj.c                   |  16 ++
src/conf/virnodedeviceobj.h                   |   6 +
src/driver-nodedev.h                          |  18 ++
src/libvirt-nodedev.c                         | 141 +++++++++++++
src/libvirt_private.syms                      |   2 +
src/libvirt_public.syms                       |   4 +
src/node_device/node_device_driver.c          | 189 +++++++++++++++++-
src/node_device/node_device_driver.h          |  19 ++
src/node_device/node_device_udev.c            |  22 +-
src/remote/remote_driver.c                    |   6 +-
src/remote/remote_protocol.x                  |  60 +++++-
src/remote_protocol-structs                   |  26 +++
.../nodedevmdevctldata/mdevctl-autostart.argv |   8 +
tests/nodedevmdevctltest.c                    |  55 +++++
tools/virsh-nodedev.c                         | 139 +++++++++++++
18 files changed, 740 insertions(+), 9 deletions(-)
create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv
[libvirt PATCH v3 0/7] Enable autostarting mediated devices
Posted by Jonathon Jongsma 2 years, 8 months ago
This first version of this series was reviewed quite a while ago and all
patches were ACKed except the second one. I posted a second series with changes
noted below but it was never ACKed and I dropped the ball for a little while.

Subsequently there were questions about whether physical devices (e.g. pci,
usb, etc) should return 'true' or 'false' for the GetAutostart()/IsPersistent()
calls.  I had initially marked them as persistent=true and autostart=true
because they superficially act a bit like persistently-defined devices. But
Boris convinced me that this is not a very accurate classification since if the
physical device is unplugged, there will be no record of it left behind. So
I've switched all non-mdev devices to be persistent=false and autostart=false.
This is also imperfect, but it seems relatively harmless. Comments welcome.

A reminder of what is included in this series:
- new API consistent with existing libvirt objects:
  - virNodeDeviceGetAutostart()
  - virNodeDeviceSetAutostart()
  - virNodeDeviceIsPersistent()
  - virNodeDeviceIsActive
- new virsh commands
  - nodedev-info
  - nodedev-autostart

Changes in version 2:
 - Parse the autostart property from mdevctl output.

Changes in version 3:
 - switch physical devices to autostart=false, persistent=false
 - rebase to upstream
 - update version numbers for new API, etc
 - fix accidental copy-paste error in virsh command descriptions

Jonathon Jongsma (7):
  api: add virNodeDevice(Get|Set)Autostart()
  nodedev: implement virNodeDevice(Get|Set)Autostart()
  nodedev: Add tests for mdevctl autostart command
  virsh: add nodedev-autostart
  api: add virNodeDeviceIsPersistent()/IsActive()
  nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
  virsh: add nodedev-info

 docs/manpages/virsh.rst                       |  27 +++
 include/libvirt/libvirt-nodedev.h             |  10 +
 src/conf/node_device_conf.h                   |   1 +
 src/conf/virnodedeviceobj.c                   |  16 ++
 src/conf/virnodedeviceobj.h                   |   6 +
 src/driver-nodedev.h                          |  18 ++
 src/libvirt-nodedev.c                         | 141 +++++++++++++
 src/libvirt_private.syms                      |   2 +
 src/libvirt_public.syms                       |   4 +
 src/node_device/node_device_driver.c          | 189 +++++++++++++++++-
 src/node_device/node_device_driver.h          |  19 ++
 src/node_device/node_device_udev.c            |  22 +-
 src/remote/remote_driver.c                    |   6 +-
 src/remote/remote_protocol.x                  |  60 +++++-
 src/remote_protocol-structs                   |  26 +++
 .../nodedevmdevctldata/mdevctl-autostart.argv |   8 +
 tests/nodedevmdevctltest.c                    |  55 +++++
 tools/virsh-nodedev.c                         | 139 +++++++++++++
 18 files changed, 740 insertions(+), 9 deletions(-)
 create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv

-- 
2.31.1


Re: [libvirt PATCH v3 0/7] Enable autostarting mediated devices
Posted by Michal Prívozník 2 years, 7 months ago
On 8/21/21 12:29 AM, Jonathon Jongsma wrote:
> This first version of this series was reviewed quite a while ago and all
> patches were ACKed except the second one. I posted a second series with changes
> noted below but it was never ACKed and I dropped the ball for a little while.
> 
> Subsequently there were questions about whether physical devices (e.g. pci,
> usb, etc) should return 'true' or 'false' for the GetAutostart()/IsPersistent()
> calls.  I had initially marked them as persistent=true and autostart=true
> because they superficially act a bit like persistently-defined devices. But
> Boris convinced me that this is not a very accurate classification since if the
> physical device is unplugged, there will be no record of it left behind. So
> I've switched all non-mdev devices to be persistent=false and autostart=false.
> This is also imperfect, but it seems relatively harmless. Comments welcome.
> 
> A reminder of what is included in this series:
> - new API consistent with existing libvirt objects:
>   - virNodeDeviceGetAutostart()
>   - virNodeDeviceSetAutostart()
>   - virNodeDeviceIsPersistent()
>   - virNodeDeviceIsActive
> - new virsh commands
>   - nodedev-info
>   - nodedev-autostart
> 
> Changes in version 2:
>  - Parse the autostart property from mdevctl output.
> 
> Changes in version 3:
>  - switch physical devices to autostart=false, persistent=false
>  - rebase to upstream
>  - update version numbers for new API, etc
>  - fix accidental copy-paste error in virsh command descriptions
> 
> Jonathon Jongsma (7):
>   api: add virNodeDevice(Get|Set)Autostart()
>   nodedev: implement virNodeDevice(Get|Set)Autostart()
>   nodedev: Add tests for mdevctl autostart command
>   virsh: add nodedev-autostart
>   api: add virNodeDeviceIsPersistent()/IsActive()
>   nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
>   virsh: add nodedev-info
> 
>  docs/manpages/virsh.rst                       |  27 +++
>  include/libvirt/libvirt-nodedev.h             |  10 +
>  src/conf/node_device_conf.h                   |   1 +
>  src/conf/virnodedeviceobj.c                   |  16 ++
>  src/conf/virnodedeviceobj.h                   |   6 +
>  src/driver-nodedev.h                          |  18 ++
>  src/libvirt-nodedev.c                         | 141 +++++++++++++
>  src/libvirt_private.syms                      |   2 +
>  src/libvirt_public.syms                       |   4 +
>  src/node_device/node_device_driver.c          | 189 +++++++++++++++++-
>  src/node_device/node_device_driver.h          |  19 ++
>  src/node_device/node_device_udev.c            |  22 +-
>  src/remote/remote_driver.c                    |   6 +-
>  src/remote/remote_protocol.x                  |  60 +++++-
>  src/remote_protocol-structs                   |  26 +++
>  .../nodedevmdevctldata/mdevctl-autostart.argv |   8 +
>  tests/nodedevmdevctltest.c                    |  55 +++++
>  tools/virsh-nodedev.c                         | 139 +++++++++++++
>  18 files changed, 740 insertions(+), 9 deletions(-)
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv
> 

Sorry for the delay.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Re: [libvirt PATCH v3 0/7] Enable autostarting mediated devices
Posted by Jonathon Jongsma 2 years, 7 months ago
Ping

Can I get a review on this? As I mentioned in the cover letter, the
only patch that was not initially ACKed in the last version was the
second one. However, the non-mdev devices were also switched to to
persistent=false and autostart=false.


On Fri, Aug 20, 2021 at 5:36 PM Jonathon Jongsma <jjongsma@redhat.com> wrote:
>
> This first version of this series was reviewed quite a while ago and all
> patches were ACKed except the second one. I posted a second series with changes
> noted below but it was never ACKed and I dropped the ball for a little while.
>
> Subsequently there were questions about whether physical devices (e.g. pci,
> usb, etc) should return 'true' or 'false' for the GetAutostart()/IsPersistent()
> calls.  I had initially marked them as persistent=true and autostart=true
> because they superficially act a bit like persistently-defined devices. But
> Boris convinced me that this is not a very accurate classification since if the
> physical device is unplugged, there will be no record of it left behind. So
> I've switched all non-mdev devices to be persistent=false and autostart=false.
> This is also imperfect, but it seems relatively harmless. Comments welcome.
>
> A reminder of what is included in this series:
> - new API consistent with existing libvirt objects:
>   - virNodeDeviceGetAutostart()
>   - virNodeDeviceSetAutostart()
>   - virNodeDeviceIsPersistent()
>   - virNodeDeviceIsActive
> - new virsh commands
>   - nodedev-info
>   - nodedev-autostart
>
> Changes in version 2:
>  - Parse the autostart property from mdevctl output.
>
> Changes in version 3:
>  - switch physical devices to autostart=false, persistent=false
>  - rebase to upstream
>  - update version numbers for new API, etc
>  - fix accidental copy-paste error in virsh command descriptions
>
> Jonathon Jongsma (7):
>   api: add virNodeDevice(Get|Set)Autostart()
>   nodedev: implement virNodeDevice(Get|Set)Autostart()
>   nodedev: Add tests for mdevctl autostart command
>   virsh: add nodedev-autostart
>   api: add virNodeDeviceIsPersistent()/IsActive()
>   nodedev: Implement virNodeDeviceIsPersistent()/IsActive()
>   virsh: add nodedev-info
>
>  docs/manpages/virsh.rst                       |  27 +++
>  include/libvirt/libvirt-nodedev.h             |  10 +
>  src/conf/node_device_conf.h                   |   1 +
>  src/conf/virnodedeviceobj.c                   |  16 ++
>  src/conf/virnodedeviceobj.h                   |   6 +
>  src/driver-nodedev.h                          |  18 ++
>  src/libvirt-nodedev.c                         | 141 +++++++++++++
>  src/libvirt_private.syms                      |   2 +
>  src/libvirt_public.syms                       |   4 +
>  src/node_device/node_device_driver.c          | 189 +++++++++++++++++-
>  src/node_device/node_device_driver.h          |  19 ++
>  src/node_device/node_device_udev.c            |  22 +-
>  src/remote/remote_driver.c                    |   6 +-
>  src/remote/remote_protocol.x                  |  60 +++++-
>  src/remote_protocol-structs                   |  26 +++
>  .../nodedevmdevctldata/mdevctl-autostart.argv |   8 +
>  tests/nodedevmdevctltest.c                    |  55 +++++
>  tools/virsh-nodedev.c                         | 139 +++++++++++++
>  18 files changed, 740 insertions(+), 9 deletions(-)
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-autostart.argv
>
> --
> 2.31.1
>
>