On Mon, 12 Dec 2016 18:49:05 -0200
Eduardo Habkost <ehabk...@redhat.com> wrote:
> Original problem description by Greg Kurz:
>
> > Since commit "9a4c0e220d8a hw/virtio-pci: fix virtio
> > behaviour", passing -device virtio-blk-pci.disable-modern=off
> > has no effect on 2.6 machine types because the internal
> > virtio-pci.disable-modern=on compat property always prevail.
>
> The same bug also affects other abstract type names mentioned on
> compat_props by machine-types: apic-common, i386-cpu, pci-device,
> powerpc64-cpu, s390-skeys, spapr-pci-host-bridge, usb-device,
> virtio-pci, x86_64-cpu.
>
> The right fix for this problem is to make sure compat_props and
> -global options are always applied in the order they are
> registered, instead of reordering them based on the type
> hierarchy. But changing the ordering rules of -global is risky
> and might break existing configurations, so we shouldn't do that
> on a stable branch.
>
> This is a temporary hack that will work around the bug when
> registering compat_props properties: if we find an abstract class
> on compat_props, register properties for all its non-abstract
> subtypes instead. This will make sure -global won't be overridden
> by compat_props, while keeping the existing ordering rules on
> -global options.
>
> Note that there's one case that won't be fixed by this hack:
> "-global spapr-pci-vfio-host-bridge.<option>=<value>" won't be
> able to override compat_props, because spapr-pci-host-bridge is
> not an abstract class.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
This patch had 3 r-b. Is there any good reason to hold
it up to 2.8.1 ?
> Changes v1 (RFC) -> v2:
> * New commit message
> * Clarified comments
> Suggested-by: Halil Pasic <pa...@linux.vnet.ibm.com>
> * Moved variable declaration outside of loop
> Suggested-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> ---
> hw/core/machine.c | 39 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index b0fd91f..213e8e0 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -554,11 +554,31 @@ static void machine_class_finalize(ObjectClass *klass,
> void *data)
> g_free(mc->name);
> }
>
> +static void register_compat_prop(const char *driver,
> + const char *property,
> + const char *value)
> +{
> + GlobalProperty *p = g_new0(GlobalProperty, 1);
> + /* Machine compat_props must never cause errors: */
> + p->errp = &error_abort;
> + p->driver = driver;
> + p->property = property;
> + p->value = value;
> + qdev_prop_register_global(p);
> +}
> +
> +static void machine_register_compat_for_subclass(ObjectClass *oc, void
> *opaque)
> +{
> + GlobalProperty *p = opaque;
> + register_compat_prop(object_class_get_name(oc), p->property, p->value);
> +}
> +
> void machine_register_compat_props(MachineState *machine)
> {
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> int i;
> GlobalProperty *p;
> + ObjectClass *oc;
>
> if (!mc->compat_props) {
> return;
> @@ -566,9 +586,22 @@ void machine_register_compat_props(MachineState *machine)
>
> for (i = 0; i < mc->compat_props->len; i++) {
> p = g_array_index(mc->compat_props, GlobalProperty *, i);
> - /* Machine compat_props must never cause errors: */
> - p->errp = &error_abort;
> - qdev_prop_register_global(p);
> + oc = object_class_by_name(p->driver);
> + if (oc && object_class_is_abstract(oc)) {
> + /* temporary hack to make sure we do not override
> + * globals set explicitly on -global: if an abstract class
> + * is on compat_props, register globals for all its
> + * non-abstract subtypes instead.
> + *
> + * This doesn't solve the problem for cases where
> + * a non-abstract typename mentioned on compat_props
> + * has subclasses, like spapr-pci-host-bridge.
> + */
> + object_class_foreach(machine_register_compat_for_subclass,
> + p->driver, false, p);
> + } else {
> + register_compat_prop(p->driver, p->property, p->value);
> + }
> }
> }
>
