[PATCH v7 0/4] Add support to enable/disable hotplug on pci-root controller

Ani Sinha ani at anisinha.ca
Sat Oct 2 03:32:02 UTC 2021



On Fri, 1 Oct 2021, Laine Stump wrote:

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

Thanks Laine! Much appretiated!

One down, one more to go :-)




More information about the libvir-list mailing list