On 10/1/21 5:29 AM, Ani Sinha wrote:
changelog:

v7: rebased the patches post libvirt release 7.8. Adjusted NEWS.rst to reflect 
7.9 (unreleased)
     version. Changed version info for new config option in formatdomain.rst as 
well.
     Added reviewed-by tags. Also in formatdomain.rst added information on what 
type of hotplug
     (ACPI/native etc) are affected by the setting.
v6: incorporated Jan's suggestions.
v5: incorporated suggestions from Laine for patches #2 and #3. The qemu command 
line
     is now added in a new function and called from 
qemuBuildControllersByTypeCommandLine().
     Output xmls are now symlinked to input xmls for unit tests.
v4: split the original patchset into a pci-root controller specific patch 
series.
     also the libvirt conf is now a sub-element of the pci-root controller as 
was
     suggested by Dan Berrange. Please see discussion here:
     
https://listman.redhat.com/archives/libvir-list/2021-September/msg00839.html
v3: reorganized the patches as per Laine's suggestion. Added more
     details in commit messages. Added conf description in formatdomain.rst.
     Added changelog for next release.
v2: fixed bugs and added additional missing unit tests.
v1: initial implementation. Had some bugs and missed some unit tests

This patchset introduces libvirt xml support to enable/disable hotplug on the
pci-root controller. It adds a 'target' subelement for the pci-root controller
with a 'hotplug' property. This property can be used to enable or disable
hotplug for the pci-root controller. For example, in order to disable hotplug
on the pci-root controller, one has to use set '<target hotplug='off'>' as
shown below:

<controller type='pci' model='pci-root'>
   <target hotplug='off'/>
</controller>

'<target hotplug='on'>' option would enable hotplug for pci-root controller.
This is also the default value. This option is only available for pc machine
types and is applicable for qemu only/kvm accelerator onlt.This feature was
introduced from qemu version 5.2 with the following change in qemu repository:

3d7e78aa7777f ("Introduce a new flag for i440fx to disable PCI hotplug on the root 
bus")

The above qemu commit describes some reasons why users might to disable hotplug
on PCI root buses [1].

The corresponding commandline option to qemu for x86 guests is:

-global PIIX4_PM.acpi-root-pci-hotplug=<off/on>

Notes:

1. The use case scenario described by Laine in
https://listman.redhat.com/archives/libvir-list/2020-February/msg00110.html
intentionally does not discuss i440fx and focusses solely on q35. I do realize
that redhat has moved on from i440fx and currently efforts for new features
are concentrated on q35 machines only. We have had some hard debates on this
on the qemu mailing list before. The fact of the matter is that i440fx is
not at 1-1 parity with q35. There are many users who are currenly using i440fx
and are simply not ready to move to q35 without sacrificing some
existing features they support today. For example
https://wiki.qemu.org/images/4/4e/Q35.pdf lists some of q35 limitations.
https://www.linux-kvm.org/images/0/06/2012-forum-Q35.pdf provides more
information on the differences. Hence we need to solve the issue Laine has
described in the above email for i440fx without adding additional bridges.

Further, in  Daniel Berrange's words from :
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg03012.html

"From the upstream POV, there's been no decision / agreement to phase
out PIIX, this is purely a RHEL downstream decision & plan. If other
distros / users have a different POV, and find the feature useful, we
should accept the patch if it meets the normal QEMU patch requirements.
"

Also to be noted that I have already experimented this qemu commandline option
using libvirt passthrough feature as has been documented in
http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html
This was only meant to be a short term solution until libvirt started
supporting this natively. Supporting this option through libvirt would simplify
their use case as well as add capability validations
and graceful failure scenarios in case qemu did not support the option.


Ani Sinha (4):
   qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx
     machines
   conf: introduce option to enable/disable pci hotplug on pci-root
     controller
   qemu: command: add support to enable/disable hotplug on pci-root
     controller
   NEWS: release note the new hotplug enable/disable option on pci-root
     controller

  NEWS.rst                                      |  6 ++++
  docs/formatdomain.rst                         | 14 ++++++---
  src/qemu/qemu_capabilities.c                  |  4 +++
  src/qemu/qemu_capabilities.h                  |  3 ++
  src/qemu/qemu_command.c                       | 17 ++++++++++
  src/qemu/qemu_validate.c                      |  9 +++++-
  .../caps_5.2.0.x86_64.xml                     |  1 +
  .../caps_6.0.0.x86_64.xml                     |  1 +
  .../caps_6.1.0.x86_64.xml                     |  1 +
  .../pc-i440fx-acpi-root-hotplug-disable.args  | 31 +++++++++++++++++++
  .../pc-i440fx-acpi-root-hotplug-disable.err   |  1 +
  .../pc-i440fx-acpi-root-hotplug-disable.xml   | 30 ++++++++++++++++++
  .../pc-i440fx-acpi-root-hotplug-enable.xml    | 30 ++++++++++++++++++
  tests/qemuxml2argvtest.c                      |  3 ++
  .../pc-i440fx-acpi-root-hotplug-disable.xml   |  1 +
  .../pc-i440fx-acpi-root-hotplug-enable.xml    |  1 +
  tests/qemuxml2xmltest.c                       |  4 +++
  17 files changed, 151 insertions(+), 6 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.args
  create mode 100644 
tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.err
  create mode 100644 
tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml
  create mode 100644 
tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.xml
  create mode 120000 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml
  create mode 120000 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-enable.xml


Okay, I made slight changes to the wording in a couple of the commit messages and documentation to emphasize that it only applies to i440fx machinetypes, and pushed the series.

Thanks for the contribution!

Reply via email to