1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemu: capabilities: Remove QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE

Peter Krempa posted 1 patch 2 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cefa2f0de7cf210f883cac90b4f88a9143b27fb9.1633937143.git.pkrempa@redhat.com
There is a newer version of this series
src/qemu/qemu_capabilities.c                       |  2 --
src/qemu/qemu_capabilities.h                       |  1 -
src/qemu/qemu_command.c                            |  3 +--
src/qemu/qemu_validate.c                           | 14 +-------------
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |  1 -
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1 -
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   |  1 -
.../pc-i440fx-acpi-hotplug-bridge-disable.err      |  1 -
.../q35-acpi-hotplug-bridge-disable.err            |  2 +-
tests/qemuxml2argvtest.c                           |  4 +---
tests/qemuxml2xmltest.c                            |  6 ++----
20 files changed, 6 insertions(+), 39 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
[PATCH] qemu: capabilities: Remove QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE
Posted by Peter Krempa 2 years, 6 months ago 
Commit 58ba0f6a3d7342fba29edbbf2bb9cb5497c870e5 added a capability which
is supported by all qemu versions we support. Remove it and the
associated dead code. Since the capability isn't present in any upstream
release we can delete it completely.

Specifically the commit itself states that it was introduced "around
(qemu) 2.1". The rest of the code handles properly that the feature is
used only on x86 with the i440fx machine so the capability is pointless.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_capabilities.c                       |  2 --
 src/qemu/qemu_capabilities.h                       |  1 -
 src/qemu/qemu_command.c                            |  3 +--
 src/qemu/qemu_validate.c                           | 14 +-------------
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |  1 -
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   |  1 -
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   |  1 -
 .../pc-i440fx-acpi-hotplug-bridge-disable.err      |  1 -
 .../q35-acpi-hotplug-bridge-disable.err            |  2 +-
 tests/qemuxml2argvtest.c                           |  4 +---
 tests/qemuxml2xmltest.c                            |  6 ++----
 20 files changed, 6 insertions(+), 39 deletions(-)
 delete mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c4d0e1858c..e95a44517e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -644,7 +644,6 @@ VIR_ENUM_IMPL(virQEMUCaps,
               "virtio-mem-pci", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI */
               "memory-backend-file.reserve", /* QEMU_CAPS_MEMORY_BACKEND_RESERVE */
               "piix4.acpi-root-pci-hotplug", /* QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG */
-              "piix4.acpi-hotplug-bridge", /* QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE */
               "ich9.acpi-hotplug-bridge", /* QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE */
     );

@@ -1474,7 +1473,6 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPiix4PM[] = {
     { "disable_s3", QEMU_CAPS_PIIX_DISABLE_S3, NULL },
     { "disable_s4", QEMU_CAPS_PIIX_DISABLE_S4, NULL },
     { "acpi-root-pci-hotplug", QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG, NULL },
-    { "acpi-pci-hotplug-with-bridge-support", QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE, NULL },
 };

 static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e9bd6c8885..92337d2503 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -624,7 +624,6 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
     QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI, /* -device virtio-mem-pci */
     QEMU_CAPS_MEMORY_BACKEND_RESERVE, /* -object memory-backend-*.reserve= */
     QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG, /* -M pc PIIX4_PM.acpi-root-pci-hotplug */
-    QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE, /* -M pc PIIX4_PM.acpi-pci-hotplug-with-bridge-support */
     QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, /* -M q35 ICH9-LPC.acpi-pci-hotplug-with-bridge-support */

     QEMU_CAPS_LAST /* this must always be the last item */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 995b294736..5b7a3e5bc3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6151,8 +6151,7 @@ qemuBuildPMCommandLine(virCommand *cmd,
     if (acpihp_br != VIR_TRISTATE_SWITCH_ABSENT) {
         const char *pm_object = NULL;

-        if (!qemuDomainIsQ35(def) &&
-            virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE))
+        if (!qemuDomainIsQ35(def))
             pm_object = "PIIX4_PM";

         if (qemuDomainIsQ35(def) &&
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index be609c9d39..3e573faa4d 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -175,13 +175,9 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def,

 static int
 qemuValidateDomainDefPCIFeature(const virDomainDef *def,
-                                virQEMUCaps *qemuCaps,
                                 int feature)
 {
     size_t i;
-    bool q35Dom = qemuDomainIsQ35(def);
-    bool q35cap = q35Dom && virQEMUCapsGet(qemuCaps,
-                                           QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);

     if (def->features[feature] == VIR_TRISTATE_SWITCH_ABSENT)
         return 0;
@@ -199,14 +195,6 @@ qemuValidateDomainDefPCIFeature(const virDomainDef *def,
                                    virArchToString(def->os.arch));
                     return -1;
                 }
-                if (!q35cap &&
-                    !virQEMUCapsGet(qemuCaps,
-                                    QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE)) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                                   _("acpi-bridge-hotplug is not available "
-                                   "with this QEMU binary"));
-                    return -1;
-                }
                 break;

             case VIR_DOMAIN_PCI_LAST:
@@ -337,7 +325,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def,
             break;

         case VIR_DOMAIN_FEATURE_PCI:
-            if (qemuValidateDomainDefPCIFeature(def, qemuCaps, i) < 0)
+            if (qemuValidateDomainDefPCIFeature(def, i) < 0)
                 return -1;
             break;
         case VIR_DOMAIN_FEATURE_SMM:
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
index 65bfe911dd..d6549d6440 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
@@ -172,7 +172,6 @@
   <flag name='am53c974'/>
   <flag name='cpu-max'/>
   <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 e4d936886b..354a95cebc 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -184,7 +184,6 @@
   <flag name='cpu-max'/>
   <flag name='input-linux'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 b903fbe403..cffe482bf6 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
@@ -190,7 +190,6 @@
   <flag name='cpu-max'/>
   <flag name='input-linux'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 143edb4e52..514e5985ac 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
@@ -194,7 +194,6 @@
   <flag name='input-linux'/>
   <flag name='query-display-options'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 936726939d..5e733fec13 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
@@ -202,7 +202,6 @@
   <flag name='input-linux'/>
   <flag name='query-display-options'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 742e71e4ae..ba9ee0dd96 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -209,7 +209,6 @@
   <flag name='input-linux'/>
   <flag name='query-display-options'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 52d0acef3d..034a770b08 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
@@ -220,7 +220,6 @@
   <flag name='input-linux'/>
   <flag name='query-display-options'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 ccd7e53ea8..aae5fe018f 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
@@ -227,7 +227,6 @@
   <flag name='input-linux'/>
   <flag name='query-display-options'/>
   <flag name='virtio-blk.queue-size'/>
-  <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 267a3acd9d..e9ae3c5abb 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
@@ -230,7 +230,6 @@
   <flag name='query-display-options'/>
   <flag name='virtio-blk.queue-size'/>
   <flag name='virtio-mem-pci'/>
-  <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 2be17f0e45..98b5f34f2b 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -232,7 +232,6 @@
   <flag name='virtio-blk.queue-size'/>
   <flag name='virtio-mem-pci'/>
   <flag name='piix4.acpi-root-pci-hotplug'/>
-  <flag name='piix4.acpi-hotplug-bridge'/>
   <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 9070eb85aa..f13a909314 100644
--- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
@@ -240,7 +240,6 @@
   <flag name='virtio-blk.queue-size'/>
   <flag name='virtio-mem-pci'/>
   <flag name='piix4.acpi-root-pci-hotplug'/>
-  <flag name='piix4.acpi-hotplug-bridge'/>
   <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 01833aff4b..9c9f17a83d 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -243,7 +243,6 @@
   <flag name='virtio-mem-pci'/>
   <flag name='memory-backend-file.reserve'/>
   <flag name='piix4.acpi-root-pci-hotplug'/>
-  <flag name='piix4.acpi-hotplug-bridge'/>
   <flag name='ich9.acpi-hotplug-bridge'/>
   <version>6001000</version>
   <kvmVersion>0</kvmVersion>
diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
deleted file mode 100644
index 8c09a3cd76..0000000000
--- a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
+++ /dev/null
@@ -1 +0,0 @@
-unsupported configuration: acpi-bridge-hotplug is not available with this QEMU binary
diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
index 8c09a3cd76..03c57b805d 100644
--- a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
+++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
@@ -1 +1 @@
-unsupported configuration: acpi-bridge-hotplug is not available with this QEMU binary
+unsupported configuration: The 'i82801b11-bridge' device is not supported by this QEMU binary
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 116ba714eb..b2a5c58bae 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2570,10 +2570,8 @@ mymain(void)
     DO_TEST("pc-i440fx-acpi-hotplug-bridge-disable",
             QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_IOH3420,
-            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
-            QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE);
+            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
     DO_TEST_PARSE_ERROR_NOCAPS("q35-acpi-hotplug-bridge-disable");
-    DO_TEST_PARSE_ERROR_NOCAPS("pc-i440fx-acpi-hotplug-bridge-disable");
     /* verify that we fail when acpi-bridge-hotplug option is specified for
      * archs other than x86
      */
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2e622c002f..ba2d9652cd 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -428,10 +428,8 @@ mymain(void)
             QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG);
     DO_TEST("pc-i440fx-acpi-root-hotplug-enable",
             QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG);
-    DO_TEST("pc-i440fx-acpi-hotplug-bridge-disable",
-            QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE);
-    DO_TEST("pc-i440fx-acpi-hotplug-bridge-enable",
-            QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE);
+    DO_TEST_NOCAPS("pc-i440fx-acpi-hotplug-bridge-disable");
+    DO_TEST_NOCAPS("pc-i440fx-acpi-hotplug-bridge-enable");
     DO_TEST("q35-acpi-hotplug-bridge-disable",
             QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_IOH3420,
-- 
2.31.1
Re: [PATCH] qemu: capabilities: Remove QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE
Posted by Ján Tomko 2 years, 6 months ago 
On a Monday in 2021, Peter Krempa wrote:
>Commit 58ba0f6a3d7342fba29edbbf2bb9cb5497c870e5 added a capability which
>is supported by all qemu versions we support. Remove it and the
>associated dead code. Since the capability isn't present in any upstream
>release we can delete it completely.
>
>Specifically the commit itself states that it was introduced "around
>(qemu) 2.1". The rest of the code handles properly that the feature is
>used only on x86 with the i440fx machine so the capability is pointless.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_capabilities.c                       |  2 --
> src/qemu/qemu_capabilities.h                       |  1 -
> src/qemu/qemu_command.c                            |  3 +--
> src/qemu/qemu_validate.c                           | 14 +-------------
> tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |  1 -
> tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1 -
> tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   |  1 -
> tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   |  1 -
> .../pc-i440fx-acpi-hotplug-bridge-disable.err      |  1 -
> .../q35-acpi-hotplug-bridge-disable.err            |  2 +-
> tests/qemuxml2argvtest.c                           |  4 +---
> tests/qemuxml2xmltest.c                            |  6 ++----
> 20 files changed, 6 insertions(+), 39 deletions(-)
> delete mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
>
>diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>index be609c9d39..3e573faa4d 100644
>--- a/src/qemu/qemu_validate.c
>+++ b/src/qemu/qemu_validate.c
>@@ -175,13 +175,9 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def,
>
> static int
> qemuValidateDomainDefPCIFeature(const virDomainDef *def,
>-                                virQEMUCaps *qemuCaps,
>                                 int feature)
> {
>     size_t i;
>-    bool q35Dom = qemuDomainIsQ35(def);
>-    bool q35cap = q35Dom && virQEMUCapsGet(qemuCaps,
>-                                           QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);

Here you removed a use of the cap for Q35's ICH9, not PIIX4 as the
commit message claims...

>
>     if (def->features[feature] == VIR_TRISTATE_SWITCH_ABSENT)
>         return 0;
>diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
>index 8c09a3cd76..03c57b805d 100644
>--- a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
>+++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
>@@ -1 +1 @@
>-unsupported configuration: acpi-bridge-hotplug is not available with this QEMU binary
>+unsupported configuration: The 'i82801b11-bridge' device is not supported by this QEMU binary

... as shown by the change in this test.

With that fixed:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [PATCH] qemu: capabilities: Remove QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE
Posted by Peter Krempa 2 years, 6 months ago 
On Mon, Oct 11, 2021 at 13:26:14 +0200, Ján Tomko wrote:
> On a Monday in 2021, Peter Krempa wrote:
> > Commit 58ba0f6a3d7342fba29edbbf2bb9cb5497c870e5 added a capability which
> > is supported by all qemu versions we support. Remove it and the
> > associated dead code. Since the capability isn't present in any upstream
> > release we can delete it completely.
> > 
> > Specifically the commit itself states that it was introduced "around
> > (qemu) 2.1". The rest of the code handles properly that the feature is
> > used only on x86 with the i440fx machine so the capability is pointless.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > src/qemu/qemu_capabilities.c                       |  2 --
> > src/qemu/qemu_capabilities.h                       |  1 -
> > src/qemu/qemu_command.c                            |  3 +--
> > src/qemu/qemu_validate.c                           | 14 +-------------
> > tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |  1 -
> > tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1 -
> > tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml   |  1 -
> > tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml   |  1 -
> > .../pc-i440fx-acpi-hotplug-bridge-disable.err      |  1 -
> > .../q35-acpi-hotplug-bridge-disable.err            |  2 +-
> > tests/qemuxml2argvtest.c                           |  4 +---
> > tests/qemuxml2xmltest.c                            |  6 ++----
> > 20 files changed, 6 insertions(+), 39 deletions(-)
> > delete mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
> > 
> > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> > index be609c9d39..3e573faa4d 100644
> > --- a/src/qemu/qemu_validate.c
> > +++ b/src/qemu/qemu_validate.c
> > @@ -175,13 +175,9 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def,
> > 
> > static int
> > qemuValidateDomainDefPCIFeature(const virDomainDef *def,
> > -                                virQEMUCaps *qemuCaps,
> >                                 int feature)
> > {
> >     size_t i;
> > -    bool q35Dom = qemuDomainIsQ35(def);
> > -    bool q35cap = q35Dom && virQEMUCapsGet(qemuCaps,
> > -                                           QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);
> 
> Here you removed a use of the cap for Q35's ICH9, not PIIX4 as the
> commit message claims...

You've trimmed a bit too much. The 'q35cap' variable was used only once
in this function ...

> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index be609c9d39..3e573faa4d 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -175,13 +175,9 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def,
> 
>  static int
>  qemuValidateDomainDefPCIFeature(const virDomainDef *def,
> -                                virQEMUCaps *qemuCaps,
>                                  int feature)
>  {
>      size_t i;
> -    bool q35Dom = qemuDomainIsQ35(def);
> -    bool q35cap = q35Dom && virQEMUCapsGet(qemuCaps,
> -                                           QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);
> 
>      if (def->features[feature] == VIR_TRISTATE_SWITCH_ABSENT)
>          return 0;
> @@ -199,14 +195,6 @@ qemuValidateDomainDefPCIFeature(const virDomainDef *def,
>                                     virArchToString(def->os.arch));
>                      return -1;
>                  }
> -                if (!q35cap &&
> -                    !virQEMUCapsGet(qemuCaps,
> -                                    QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE)) {
> -                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                                   _("acpi-bridge-hotplug is not available "
> -                                   "with this QEMU binary"));

... here. Since QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE is always present,
the whole condition is a contradiction since && is used. Thus this whole
thing can be removed and 'q35cap' becomes unused. So I had to remove it.

> -                    return -1;
> -                }
>                  break;
> 
>              case VIR_DOMAIN_PCI_LAST:


The error message changes but it's because the new tests use fake
capabilities, otherwise it would just succeed.

I plan to address the issue of testing of the new series later on, I
just wanted to get rid of this extra capability as I needed to rebase my
branches.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.