[PATCH] qemu: capabilities: Remove QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE

Peter Krempa pkrempa at redhat.com
Mon Oct 11 11:34:41 UTC 2021


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 at 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.




More information about the libvir-list mailing list