6:34 a.m.
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(a)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.