6:50 p.m.
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
6:50 p.m.
New subject: [PATCH v2 1/4] pm/i386: add support for two options that controls acpi hotplug on q35/i440fx
'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(a)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
6:50 p.m.
New subject: [PATCH v2 2/4] tests: add positive xml2argv tests to exercize pm acpi hotplug options for i386
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(a)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
6:50 p.m.
New subject: [PATCH v2 3/4] tests: add negative xml2argv tests to exercize pm acpi hotplug options for i386
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(a)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
6:50 p.m.
New subject: [PATCH v2 4/4] tests: add xml2xml tests to exercize pm acpi hotplug options for i386
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(a)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
11:33 a.m.
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.
10:02 p.m.
Ping again …
On Wed, Sep 1, 2021 at 09:03 Ani Sinha <ani(a)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.
9:26 a.m.
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.
11:36 a.m.
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.
1091
days inactive
1115
days old
8 comments
2 participants
participants (2)
-
Ani Sinha -
Laine Stump