[PATCH v6 3/4] qemu: command: add support for acpi-bridge-hotplug feature
Laine Stump
laine at redhat.com
Fri Oct 8 04:07:58 UTC 2021
On 10/5/21 1:51 AM, Ani Sinha wrote:
> This change adds backend qemu command line support for new libvirt global
> feature 'acpi-bridge-hotplug'. This option can be used as following:
>
> <feature>
> <pci>
> <acpi-bridge-hotplug state='off|on'/>
> </pci>
> </feature>
>
> The '<pci>' sub-element under '<feature>' is also newly introduced.
>
> 'acpi-bridge-hotplug' turns on the following command line option to qemu for
> x86 guests:
>
> (pc machines): -global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=<off|on>
> (q35 machines): -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=<off|on>
>
> This change also adds the required qemuxml2argv unit tests in order to test
> correct qemu arguments. Unit tests have also been added to test qemu capability
> validation checks as well as checks for using this option with the right
> architecture.
>
> Signed-off-by: Ani Sinha <ani at anisinha.ca>
> ---
> src/qemu/qemu_command.c | 14 ++++++++
> .../aarch64-acpi-hotplug-bridge-disable.err | 1 +
> ...pc-i440fx-acpi-hotplug-bridge-disable.args | 31 +++++++++++++++++
> .../pc-i440fx-acpi-hotplug-bridge-disable.err | 1 +
> .../q35-acpi-hotplug-bridge-disable.args | 33 +++++++++++++++++++
> .../q35-acpi-hotplug-bridge-disable.err | 1 +
> tests/qemuxml2argvtest.c | 16 +++++++++
> 7 files changed, 97 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
> create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
> create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
> create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
> create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 08f6d735f8..6c8c99e595 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6069,6 +6069,7 @@ qemuBuildPMCommandLine(virCommand *cmd,
> qemuDomainObjPrivate *priv)
> {
> virQEMUCaps *qemuCaps = priv->qemuCaps;
> + int acpihp_br = def->pci_features[(virDomainPCI)VIR_DOMAIN_PCI_ACPI_BRIDGE_HP];
I'm not sure why you put the typecast on the array subscript, but it's
unnecessary. I removed it.
>
> if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) {
> /* with new qemu we always want '-no-shutdown' on startup and we set
> @@ -6114,6 +6115,19 @@ qemuBuildPMCommandLine(virCommand *cmd,
> pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO);
> }
>
> + if (acpihp_br) {
> + const char *pm_object = "PIIX4_PM";
> +
> + if (qemuDomainIsQ35(def) &&
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE))
> + pm_object = "ICH9-LPC";
So on Q35 systems that don't support setting
"ICH9-LPC.acpi-pci-hotplug....." it will also work to set
"PIIX4_PM.acpi-pci-...."? (apparently this is the case with the s3 and
s4 acpi suspend options, which have a similar hack a few lines below
this one).
Assuming that is the case, then this patch also seems fine to me
Reviewed-by: Laine Stump <laine at redhat.com>
I may want to make the documentation in formatdomain.rst a bit more
clear, but I can do that as a separate patch after these (since I know
you're leaving on a vacation so won't be around to do it).
As long as nobody complains about the naming of the feature, I think I
can push this tomorrow evening (U.S. time) (that will give time for any
discussion about naming in the morning, then I'll have to be afk for a
few hours in the afternoon.
Thanks for your contribution, and for your perseverance in the face of
my procrastination, and enjoy your vacation!!
> +
> + virCommandAddArg(cmd, "-global");
> + virCommandAddArgFormat(cmd, "%s.acpi-pci-hotplug-with-bridge-support=%s",
> + pm_object,
> + virTristateSwitchTypeToString(acpihp_br));
> + }
> +
> return 0;
> }
>
> diff --git a/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
> new file mode 100644
> index 0000000000..9f0a88b826
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
> @@ -0,0 +1 @@
> +unsupported configuration: acpi-bridge-hotplug is not available for architecture 'aarch64'
> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
> new file mode 100644
> index 0000000000..a1e5dc85c7
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
> @@ -0,0 +1,31 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-i440fx \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-i440fx/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-i440fx/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-i440fx/.config \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 \
> +-name guest=i440fx,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-i440fx/master-key.aes \
> +-machine pc-i440fx-2.5,accel=tcg,usb=off,dump-guest-core=off \
> +-m 1024 \
> +-realtime mlock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid 56f5055c-1b8d-490c-844a-ad646a1caaaa \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-i440fx/monitor.sock,server=on,wait=off \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off \
> +-boot strict=on \
> +-usb \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
> new file mode 100644
> index 0000000000..8c09a3cd76
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
> @@ -0,0 +1 @@
> +unsupported configuration: acpi-bridge-hotplug is not available with this QEMU binary
> diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
> new file mode 100644
> index 0000000000..007c22c646
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
> @@ -0,0 +1,33 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-q35 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-q35/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-q35/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-q35/.config \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 \
> +-name guest=q35,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-q35/master-key.aes \
> +-machine pc-q35-2.5,accel=tcg,usb=off,dump-guest-core=off \
> +-m 1024 \
> +-realtime mlock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid 56f5055c-1b8d-490c-844a-ad646a1caaaa \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-q35/monitor.sock,server=on,wait=off \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off \
> +-boot strict=on \
> +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
> +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
> +-device ioh3420,port=0x8,chassis=3,id=pci.3,bus=pcie.0,addr=0x1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
> new file mode 100644
> index 0000000000..8c09a3cd76
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
> @@ -0,0 +1 @@
> +unsupported configuration: acpi-bridge-hotplug is not available with this QEMU binary
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 94aaa2f53e..507aa32009 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2574,6 +2574,22 @@ mymain(void)
> DO_TEST("pc-i440fx-acpi-root-hotplug-disable",
> QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG);
> DO_TEST_PARSE_ERROR_NOCAPS("pc-i440fx-acpi-root-hotplug-disable");
> + DO_TEST("q35-acpi-hotplug-bridge-disable",
> + QEMU_CAPS_DEVICE_PCI_BRIDGE,
> + QEMU_CAPS_DEVICE_IOH3420,
> + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_ICH9_AHCI,
> + QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);
> + DO_TEST("pc-i440fx-acpi-hotplug-bridge-disable",
> + QEMU_CAPS_DEVICE_PCI_BRIDGE,
> + QEMU_CAPS_DEVICE_IOH3420,
> + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
> + QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE);
> + DO_TEST_PARSE_ERROR_NOCAPS("q35-acpi-hotplug-bridge-disable");
> + DO_TEST_PARSE_ERROR_NOCAPS("pc-i440fx-acpi-hotplug-bridge-disable");
> + /* verify that we fail when acpi-bridge-hotplug option is specified for
> + * archs other than x86
> + */
> + DO_TEST_PARSE_ERROR_NOCAPS("aarch64-acpi-hotplug-bridge-disable");
> DO_TEST("q35-usb2",
> QEMU_CAPS_DEVICE_PCI_BRIDGE,
> QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
>
More information about the libvir-list
mailing list