[PATCH v2 0/4] Add support for two i386 pm options which control acpi hotplug
Hi: I added some negative xml2argv tests as well as new xml2xml tests. In the process, I also fixed a bug where I had not added appropriate code to generate the output xml correctly. The patch series is sent again as v2. Kindly, please provide inputs and review them. [PATCH v2 1/4] pm/i386: add support for two options that controls [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug
'acpi-pci-hotplug-with-bridge-support' and 'acpi-root-pci-hotplug' are two pm options for i386 that governs acpi pci hotplug support for qemu. This patch tries to implement support for both these two global i386 pm options within libvirt. 'acpi-pci-hotplug-with-bridge-support' applies both for q35 as well as i440fx machine types. The 'acpi-root-pci-hotplug' only applies for i440fx machine type. 'acpi-pci-hotplug-with-bridge-support' can be turned off by providing the following libvirt xml snippet: <pm> <acpi-hotplug-bridge enabled='no'> </pm> Similarly, 'acpi-root-pci-hotplug' can be turned off by providing the following libvirt xml snippet: <pm> <acpi-root-hotplug enabled='no'> </pm> Similarly for turning them on explicitly, they can pass " enabled='yes' " in the above xml snippet. Unit tests for this change will be added in subsequent patches. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- docs/schemas/domaincommon.rng | 17 +++++++++++ src/conf/domain_conf.c | 21 +++++++++++++- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_capabilities.c | 8 ++++++ src/qemu/qemu_capabilities.h | 5 ++++ src/qemu/qemu_command.c | 28 +++++++++++++++++++ src/qemu/qemu_validate.c | 21 ++++++++++++++ .../caps_2.11.0.x86_64.xml | 1 + .../caps_2.12.0.x86_64.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.x86_64.xml | 2 ++ .../caps_6.0.0.x86_64.xml | 2 ++ .../caps_6.1.0.x86_64.xml | 3 ++ 19 files changed, 117 insertions(+), 1 deletion(-) changelog: v1: initial version. v2: added code to generate output xml correctly. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2442078969..e2d9bb336c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4317,6 +4317,16 @@ <ref name="suspendChoices"/> </element> </optional> + <optional> + <element name="acpi-hotplug-bridge"> + <ref name="acpiHotplugChoices"/> + </element> + </optional> + <optional> + <element name="acpi-root-hotplug"> + <ref name="acpiHotplugChoices"/> + </element> + </optional> </interleave> <empty/> </element> @@ -4328,6 +4338,13 @@ </attribute> </optional> </define> + <define name="acpiHotplugChoices"> + <optional> + <attribute name="enabled"> + <ref name="virYesNo"/> + </attribute> + </optional> + </define> <!-- Specific setup for a qemu emulated character device. Note: this definition doesn't fully specify the constraints on this node. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06c1fcf5e5..778a44e08e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19351,6 +19351,16 @@ virDomainDefLifecycleParse(virDomainDef *def, &def->pm.s4) < 0) goto error; + if (virDomainPMStateParseXML(ctxt, + "string(./pm/acpi-hotplug-bridge/@enabled)", + &def->pm.acpi_hp_bridge) < 0) + goto error; + + if (virDomainPMStateParseXML(ctxt, + "string(./pm/acpi-root-hotplug/@enabled)", + &def->pm.acpi_root_hp) < 0) + goto error; + return 0; error: @@ -28179,7 +28189,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, virDomainLockFailureTypeToString) < 0) return -1; - if (def->pm.s3 || def->pm.s4) { + if (def->pm.s3 || def->pm.s4 || def->pm.acpi_hp_bridge || + def->pm.acpi_root_hp) { virBufferAddLit(buf, "<pm>\n"); virBufferAdjustIndent(buf, 2); if (def->pm.s3) { @@ -28190,6 +28201,14 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, virBufferAsprintf(buf, "<suspend-to-disk enabled='%s'/>\n", virTristateBoolTypeToString(def->pm.s4)); } + if (def->pm.acpi_hp_bridge) { + virBufferAsprintf(buf, "<acpi-hotplug-bridge enabled='%s'/>\n", + virTristateBoolTypeToString(def->pm.acpi_hp_bridge)); + } + if (def->pm.acpi_root_hp) { + virBufferAsprintf(buf, "<acpi-root-hotplug enabled='%s'/>\n", + virTristateBoolTypeToString(def->pm.acpi_root_hp)); + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</pm>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ca21082624..94283cbb04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2643,6 +2643,8 @@ struct _virDomainPowerManagement { /* These options are of type enum virTristateBool */ int s3; int s4; + int acpi_hp_bridge; + int acpi_root_hp; }; struct _virDomainPerfDef { diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9558938866..ba0665474c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -637,6 +637,11 @@ VIR_ENUM_IMPL(virQEMUCaps, "confidential-guest-support", "query-display-options", "s390-pv-guest", + "piix4-acpi-hotplug-bridge", + "piix4-acpi-root-hotplug-en", + + /* 410 */ + "ich9-acpi-hotplug-bridge", ); @@ -1467,6 +1472,8 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsIDEDrive[] = { static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPiix4PM[] = { { "disable_s3", QEMU_CAPS_PIIX_DISABLE_S3, NULL }, { "disable_s4", QEMU_CAPS_PIIX_DISABLE_S4, NULL }, + { "acpi-pci-hotplug-with-bridge-support", QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, NULL }, + { "acpi-root-pci-hotplug", QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = { @@ -1519,6 +1526,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioGpu[] = { static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsICH9[] = { { "disable_s3", QEMU_CAPS_ICH9_DISABLE_S3, NULL }, { "disable_s4", QEMU_CAPS_ICH9_DISABLE_S4, NULL }, + { "acpi-pci-hotplug-with-bridge-support", QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBNECXHCI[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2b1bb57a49..e988527f39 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -617,6 +617,11 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine confidential-guest-support */ QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */ QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */ + QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, /* -M pc PIIX4_PM.acpi-pci-hotplug-with-bridge-support */ + QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, /* -M pc PIIX4_PM.acpi-root-pci-hotplug */ + + /* 410 */ + QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, /* -M q35 PIIX4_PM.acpi-pci-hotplug-with-bridge-support */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 156af4caee..16c609202c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6171,6 +6171,34 @@ qemuBuildPMCommandLine(virCommand *cmd, pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO); } + if (def->pm.acpi_hp_bridge) { + const char *pm_object = "PIIX4_PM"; + const char *switch_str = "on"; + + if (def->pm.acpi_hp_bridge == VIR_TRISTATE_BOOL_NO) + switch_str = "off"; + + if (qemuDomainIsQ35(def) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE)) + pm_object = "ICH9-LPC"; + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.acpi-pci-hotplug-with-bridge-support=%s", + pm_object, switch_str); + } + + if (def->pm.acpi_root_hp) { + const char *pm_object = "PIIX4_PM"; + const char *switch_str = "on"; + + if (def->pm.acpi_root_hp == VIR_TRISTATE_BOOL_NO) + switch_str = "off"; + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.acpi-root-pci-hotplug=%s", + pm_object, switch_str); + } + return 0; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index a964c8593d..22403e6d01 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -560,6 +560,27 @@ qemuValidateDomainDefPM(const virDomainDef *def, } } + if (def->pm.acpi_hp_bridge) { + bool q35ICH9_pcihpbr = q35Dom && + virQEMUCapsGet(qemuCaps, + QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE); + + if (!q35ICH9_pcihpbr && !virQEMUCapsGet(qemuCaps, + QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI hotplug bridge not supported")); + return -1; + } + } + + if (def->pm.acpi_root_hp) { + if (!q35Dom && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI root pci hotplug not supported")); + return -1; + } + } + return 0; } diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml index 37fb33e8e3..75ebb88dd8 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml @@ -196,6 +196,7 @@ <flag name='cpu-max'/> <flag name='vnc-opts'/> <flag name='input-linux'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100288</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 37aaabe0d3..dbf77b04a1 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -207,6 +207,7 @@ <flag name='cpu-max'/> <flag name='vnc-opts'/> <flag name='input-linux'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100289</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml index f45fd84aaa..9cde4a95a3 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml @@ -213,6 +213,7 @@ <flag name='cpu-max'/> <flag name='vnc-opts'/> <flag name='input-linux'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100239</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml index dcc5f6f137..5dcf3b5a08 100644 --- a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml @@ -217,6 +217,7 @@ <flag name='vnc-opts'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>3000092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 7eff08235e..7ab188072d 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -225,6 +225,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index 736b120547..bacb5f8dd2 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -232,6 +232,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>4001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index 8352fd0200..e89655d209 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -243,6 +243,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index 6a99f4e343..058c943716 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -250,6 +250,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index 674d984432..85e9ba95d0 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -252,6 +252,7 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index ec3384cab8..190e36719a 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -253,6 +253,8 @@ <flag name='rotation-rate'/> <flag name='input-linux'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> + <flag name='piix4-acpi-root-hotplug-en'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index d6198c2479..cf9c06470d 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -260,6 +260,8 @@ <flag name='input-linux'/> <flag name='confidential-guest-support'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> + <flag name='piix4-acpi-root-hotplug-en'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index 933b8eb2b5..e3a7d231ce 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -262,6 +262,9 @@ <flag name='virtio-vga-gl'/> <flag name='confidential-guest-support'/> <flag name='query-display-options'/> + <flag name='piix4-acpi-hotplug-bridge'/> + <flag name='piix4-acpi-root-hotplug-en'/> + <flag name='ich9-acpi-hotplug-bridge'/> <version>6000090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.25.1
This change adds positive xml2argv unit tests for the following change: 78ced83e3bb354 ("pm/i386: add support for two options that controls acpi hotplug on q35/i440fx") These tests ensure that libvirt generates the correct qemu commandline for a given input xml. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- ...pc-i440fx-acpi-hotplug-bridge-disable.args | 28 +++++++++++++++++ .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 17 +++++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.args | 28 +++++++++++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 17 +++++++++++ .../pc-i440fx-acpi-root-hotplug-disable.args | 28 +++++++++++++++++ .../pc-i440fx-acpi-root-hotplug-disable.xml | 17 +++++++++++ .../pc-i440fx-acpi-root-hotplug-enable.args | 28 +++++++++++++++++ .../pc-i440fx-acpi-root-hotplug-enable.xml | 17 +++++++++++ .../q35-acpi-hotplug-bridge-disable.args | 30 +++++++++++++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 17 +++++++++++ .../q35-acpi-hotplug-bridge-enable.args | 30 +++++++++++++++++++ .../q35-acpi-hotplug-bridge-enable.xml | 17 +++++++++++ tests/qemuxml2argvtest.c | 30 +++++++++++++++++++ 13 files changed, 304 insertions(+) create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.args create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.args create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.args create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.args create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml changelog: v1: initial patch. v2: removed some capabilities from tests that were not needed. 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..0bf95a023a --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args @@ -0,0 +1,28 @@ +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 i440fx \ +-S \ +-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 \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..b96caa9b2d --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <pm> + <acpi-hotplug-bridge enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.args b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.args new file mode 100644 index 0000000000..75c99b49d3 --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.args @@ -0,0 +1,28 @@ +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 i440fx \ +-S \ +-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=on \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml new file mode 100644 index 0000000000..40bda970ad --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <pm> + <acpi-hotplug-bridge enabled='yes'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.args b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.args new file mode 100644 index 0000000000..1fc19aa868 --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.args @@ -0,0 +1,28 @@ +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 i440fx \ +-S \ +-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-root-pci-hotplug=off \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml new file mode 100644 index 0000000000..ef563200bf --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <pm> + <acpi-root-hotplug enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.args b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.args new file mode 100644 index 0000000000..f9abb953e3 --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.args @@ -0,0 +1,28 @@ +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 i440fx \ +-S \ +-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-root-pci-hotplug=on \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.xml new file mode 100644 index 0000000000..7767025c88 --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <pm> + <acpi-root-hotplug enabled='yes'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> 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..250a479938 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args @@ -0,0 +1,30 @@ +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 q35 \ +-S \ +-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 \ +-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 diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..bb9e8d4ee2 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>q35</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.5'>hvm</type> + <boot dev='network'/> + </os> + <pm> + <acpi-hotplug-bridge enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.args b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.args new file mode 100644 index 0000000000..24a5d40b47 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.args @@ -0,0 +1,30 @@ +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 q35 \ +-S \ +-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=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 diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml new file mode 100644 index 0000000000..e9c4b09da2 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>q35</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.5'>hvm</type> + <boot dev='network'/> + </os> + <pm> + <acpi-hotplug-bridge enabled='yes'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 67c056b887..75a7acdfe8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2622,6 +2622,36 @@ mymain(void) QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4); + 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("q35-acpi-hotplug-bridge-enable", + 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_PIIX_ACPI_HOTPLUG_BRIDGE); + DO_TEST("pc-i440fx-acpi-hotplug-bridge-enable", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE); + DO_TEST("pc-i440fx-acpi-root-hotplug-disable", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG); + DO_TEST("pc-i440fx-acpi-root-hotplug-enable", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG); DO_TEST("q35-usb2", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, -- 2.25.1
This change adds negative argv2xml unit tests for the following change: 78ced83e3bb354 ("pm/i386: add support for two options that controls acpi hotplug on q35/i440fx") These tests ensure that libvirt produces the correct error when the qemu capabilities required from an input xml are absent. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- .../qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.err | 1 + tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.err | 1 + tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.err | 1 + tests/qemuxml2argvtest.c | 3 +++ 4 files changed, 6 insertions(+) create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.err create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.err create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.err changelog: v1: not present. v2: added this new test. diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.err b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.err new file mode 100644 index 0000000000..98211b726f --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.err @@ -0,0 +1 @@ +unsupported configuration: setting ACPI hotplug bridge not supported diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.err b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.err new file mode 100644 index 0000000000..c5c9de8389 --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.err @@ -0,0 +1 @@ +unsupported configuration: setting ACPI root pci hotplug not supported diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.err b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.err new file mode 100644 index 0000000000..98211b726f --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.err @@ -0,0 +1 @@ +unsupported configuration: setting ACPI hotplug bridge not supported diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 75a7acdfe8..7d83937f48 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2652,6 +2652,9 @@ mymain(void) QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG); + DO_TEST_PARSE_ERROR("q35-acpi-hotplug-bridge-enable", NONE); + DO_TEST_PARSE_ERROR("pc-i440fx-acpi-hotplug-bridge-enable", NONE); + DO_TEST_PARSE_ERROR("pc-i440fx-acpi-root-hotplug-enable", NONE); DO_TEST("q35-usb2", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, -- 2.25.1
This change adds xml2xml unit tests for the following change: 78ced83e3bb354 ("pm/i386: add support for two options that controls acpi hotplug on q35/i440fx") This unit test ensures that libvirt generates the correct output xml with proper pm acpi hotplug options for a given input xml. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 31 +++++++++++++ .../pc-i440fx-acpi-root-hotplug-disable.xml | 31 +++++++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 45 +++++++++++++++++++ tests/qemuxml2xmltest.c | 9 ++++ 4 files changed, 116 insertions(+) create mode 100644 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml create mode 100644 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml changelog: v1: not present. v2: added this new test. diff --git a/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..2534dc7e35 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <pm> + <acpi-hotplug-bridge enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml new file mode 100644 index 0000000000..0ee404ee46 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <pm> + <acpi-root-hotplug enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..e8c6f82145 --- /dev/null +++ b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>q35</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.5'>hvm</type> + <boot dev='network'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <pm> + <acpi-hotplug-bridge enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 983489af6c..31086214fc 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -427,6 +427,15 @@ mymain(void) DO_TEST("input-usbtablet", NONE); DO_TEST("misc-acpi", NONE); DO_TEST("misc-disable-s3", QEMU_CAPS_PIIX_DISABLE_S3); + DO_TEST("pc-i440fx-acpi-hotplug-bridge-disable", + QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE); + 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_ACPI_HOTPLUG_BRIDGE); + DO_TEST("pc-i440fx-acpi-root-hotplug-disable", + QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG); DO_TEST("misc-disable-suspends", QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4); -- 2.25.1
On Thu, 19 Aug 2021, Ani Sinha wrote:
Hi:
I added some negative xml2argv tests as well as new xml2xml tests. In the process, I also fixed a bug where I had not added appropriate code to generate the output xml correctly. The patch series is sent again as v2. Kindly, please provide inputs and review them.
[PATCH v2 1/4] pm/i386: add support for two options that controls [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug
Ping ... please provide some review on this patchset.
Ping again … On Wed, Sep 1, 2021 at 09:03 Ani Sinha <ani@anisinha.ca> wrote:
On Thu, 19 Aug 2021, Ani Sinha wrote:
Hi:
I added some negative xml2argv tests as well as new xml2xml tests. In the process, I also fixed a bug where I had not added appropriate code to generate the output xml correctly. The patch series is sent again as v2. Kindly, please provide inputs and review them.
[PATCH v2 1/4] pm/i386: add support for two options that controls [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug
Ping ... please provide some review on this patchset.
On 8/19/21 6:50 AM, Ani Sinha wrote:
Hi:
I added some negative xml2argv tests as well as new xml2xml tests. In the process, I also fixed a bug where I had not added appropriate code to generate the output xml correctly. The patch series is sent again as v2. Kindly, please provide inputs and review them.
[PATCH v2 1/4] pm/i386: add support for two options that controls [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug
Hi, Sorry for not sending email about this earlier. We went over a few things in an IRC chat a couple weeks ago. I'll reiterate the questions I asked there, and fill in more info on what we are looking for in patches. * Usually when a new feature like this is supported, there will be several patches, divided something like this: 1) A patch that just adds the new QEMU capabilities flag(s) that will be used later to detect whether or not the selected QEMU binary supports the feature. (This might require updates to the sample QEMU capabilities test files, if the flag/option didn't happen to coincidentally already be in them. Instructions for regenerating the capabilities .replies files are in the comments of tests/qemucapabilitiestest.c.) 2) A patch adding the new XML to the RNG and to the XML parser in conf. This patch will also contain 2b) an addition to XML documentation in docs/formatdomain.rst that names & describes the purpose of the new elements/attributes and provides at least one example of their use (the commit log message should include an example too, to make searching for the commit easier), and 2c) parser/formatter tests added to qemuxml2xmltest.c, with the original XML in tests/qemuxml2argvdata, and output XML in qemuxml2xmlout (if the input and output files are identical, then the file in qemuxml2xmlout should be a symlink to the file in qemuxml2argvdata) 3) A patch that implements the backend of this new feature in the qemu driver by checking for its availability using the capabilities in patch 1, and using the data in the domain object now being parsed by patch 2 to add something to the qemu commandline. This patch should also include 3b) all additional test cases for qemuxml2argvtest. By careful planning, these new test cases will use the same source XML that was added in (2c). Note that *all* tests for new code are in the same patch as the new code itself. I like doing it this way because it ensures that the tests won't be forgotten/omitted on purpose when backporting to a different branch. 4) Oh, and just for abologna, a separate patch to add an entry to the NEWS.rst file for the next release :-) 0) (backing up a bit) In addition, the cover-letter (patch 0/n) should contain a *thorough* description of what each new XML element/attribute does, why it is desirable, and a link to the QEMU documentation (and/or patches as pushed into qemu) describing what they do in QEMU terms. I *think* that the 440fx-only option you added in these patches disables/enables hotplug of devices with a single systemwide flag (right?); I'm still uncertain what the other option does - apparently something about enabling the hotplug of a conventional PCI bridge? Or does it enable/disable hotplug of endpoint devices *into* conventional PCI bridges?). This information could be included directly in the cover letter, or the cover letter can point to the commit log message for patch 2 or 3 which would then have all the information. ========= About the "Why?" question above - many years ago someone decided that every feature added to QEMU *must* be supported directly in libvirt. This led to a large bloat of XML to support QEMU features that "seemed like a good idea at the time", but nobody ever uses (partly because it's unclear exactly how/when they should be used). In the more recent past, we've started asking "Is the maintenance burden of supporting this feature in libvirt really worthwhile? Is it usable? Who will actually use it, and for what?" (for a few years both QEMU and libvirt have been trying to get away from conventional PCI + 440fx, and concentrate our efforts on PCIe + Q35, so adding new functionality for conventional PCI + 440fx feels kind of like adding a new option to the IDE disk controller :-) I had questions on this topic for these new options, but realized that they all depended on my proper understanding of their function, and since I don't think I properly understand them yet, all my questions are potentially pointless, so I've removed them for now. Maybe it will all be clear once I've been properly informed. ==== In the meantime, although not officially "supported" (I believe its use will taint the libvirt config) it's possible to add any random option not supported by libvirt to the QEMU commandline with libvirt's <qemu:commandline> element, documented here: http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html If your reason for posting these patches was just so that you could try out these QEMU options, <qemu:commandline> is a much simpler/quicker way to do it than going through all the trouble of adding specific support to libvirt. ==== If you're beyond the experimenting stage with these new options, and really do have a real-world use case that requires specific support in libvirt, then I'd be happy to review (hopefully in a more timely fashion!) a resubmission of the patches organized as I laid out above, with the additional requested documentation added to 1) the cover letter, 2) the patch that adds the new XML to the parser/formatter, and 3) formatdomain.rst.
On Thu, 9 Sep 2021, Laine Stump wrote:
On 8/19/21 6:50 AM, Ani Sinha wrote:
Hi:
I added some negative xml2argv tests as well as new xml2xml tests. In the process, I also fixed a bug where I had not added appropriate code to generate the output xml correctly. The patch series is sent again as v2. Kindly, please provide inputs and review them.
[PATCH v2 1/4] pm/i386: add support for two options that controls [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug
Hi,
Sorry for not sending email about this earlier. We went over a few things in an IRC chat a couple weeks ago. I'll reiterate the questions I asked there, and fill in more info on what we are looking for in patches.
Thanks laine for such a detailed description of the preferred patch organization. I have sent a v3 with the same subject line CC-ing you and Julia. I have tried to add all the relevant details to the cover letter as well as the commit messages. The patches has been organized as per the suggestion in this email. I have tried to use my best judgement in terms of description/documentation etc and look forward to your further comments. I have also tried to answer the question as to why I think the support is needed in libvirt. In particular, the libvirt passthrough to qemu option has already been tried and tested to work. Please let me know if you have further questions or doubts. Ani
* Usually when a new feature like this is supported, there will be several patches, divided something like this:
1) A patch that just adds the new QEMU capabilities flag(s) that will be used later to detect whether or not the selected QEMU binary supports the feature. (This might require updates to the sample QEMU capabilities test files, if the flag/option didn't happen to coincidentally already be in them. Instructions for regenerating the capabilities .replies files are in the comments of tests/qemucapabilitiestest.c.)
2) A patch adding the new XML to the RNG and to the XML parser in conf. This patch will also contain
2b) an addition to XML documentation in docs/formatdomain.rst that names & describes the purpose of the new elements/attributes and provides at least one example of their use (the commit log message should include an example too, to make searching for the commit easier), and
2c) parser/formatter tests added to qemuxml2xmltest.c, with the original XML in tests/qemuxml2argvdata, and output XML in qemuxml2xmlout (if the input and output files are identical, then the file in qemuxml2xmlout should be a symlink to the file in qemuxml2argvdata)
3) A patch that implements the backend of this new feature in the qemu driver by checking for its availability using the capabilities in patch 1, and using the data in the domain object now being parsed by patch 2 to add something to the qemu commandline. This patch should also include
3b) all additional test cases for qemuxml2argvtest. By careful planning, these new test cases will use the same source XML that was added in (2c).
Note that *all* tests for new code are in the same patch as the new code itself. I like doing it this way because it ensures that the tests won't be forgotten/omitted on purpose when backporting to a different branch.
4) Oh, and just for abologna, a separate patch to add an entry to the NEWS.rst file for the next release :-)
0) (backing up a bit) In addition, the cover-letter (patch 0/n) should contain a *thorough* description of what each new XML element/attribute does, why it is desirable, and a link to the QEMU documentation (and/or patches as pushed into qemu) describing what they do in QEMU terms. I *think* that the 440fx-only option you added in these patches disables/enables hotplug of devices with a single systemwide flag (right?); I'm still uncertain what the other option does - apparently something about enabling the hotplug of a conventional PCI bridge? Or does it enable/disable hotplug of endpoint devices *into* conventional PCI bridges?).
This information could be included directly in the cover letter, or the cover letter can point to the commit log message for patch 2 or 3 which would then have all the information.
=========
About the "Why?" question above - many years ago someone decided that every feature added to QEMU *must* be supported directly in libvirt. This led to a large bloat of XML to support QEMU features that "seemed like a good idea at the time", but nobody ever uses (partly because it's unclear exactly how/when they should be used). In the more recent past, we've started asking "Is the maintenance burden of supporting this feature in libvirt really worthwhile? Is it usable? Who will actually use it, and for what?" (for a few years both QEMU and libvirt have been trying to get away from conventional PCI + 440fx, and concentrate our efforts on PCIe + Q35, so adding new functionality for conventional PCI + 440fx feels kind of like adding a new option to the IDE disk controller :-)
I had questions on this topic for these new options, but realized that they all depended on my proper understanding of their function, and since I don't think I properly understand them yet, all my questions are potentially pointless, so I've removed them for now. Maybe it will all be clear once I've been properly informed.
====
In the meantime, although not officially "supported" (I believe its use will taint the libvirt config) it's possible to add any random option not supported by libvirt to the QEMU commandline with libvirt's <qemu:commandline> element, documented here:
http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html
If your reason for posting these patches was just so that you could try out these QEMU options, <qemu:commandline> is a much simpler/quicker way to do it than going through all the trouble of adding specific support to libvirt.
====
If you're beyond the experimenting stage with these new options, and really do have a real-world use case that requires specific support in libvirt, then I'd be happy to review (hopefully in a more timely fashion!) a resubmission of the patches organized as I laid out above, with the additional requested documentation added to 1) the cover letter, 2) the patch that adds the new XML to the parser/formatter, and 3) formatdomain.rst.
participants (2)
-
Ani Sinha -
Laine Stump