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