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.