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@xxxxxxxxxx>
> > ---
> > 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.
