qdev: Fix latent bug with compat_props and onboard devices

Compatibility properties started life as a qdev property thing: we
supported them only for qdev properties, and implemented them with the
machinery backing command line option -global.

Recent commit fa0cb34d22 put them to use (tacitly) with memory
backend objects (subtypes of TYPE_MEMORY_BACKEND).  To make that
possible, we first moved the work of applying them from the -global
machinery into TYPE_DEVICE's .instance_post_init() method
device_post_init(), in commits ea9ce8934c and b66bbee39f, then made
it available to TYPE_MEMORY_BACKEND's .instance_post_init() method
host_memory_backend_post_init() as object_apply_compat_props(), in
commit 1c3994f6d2.

Note the code smell: we now have function name starting with object_
in hw/core/qdev.c.  It has to be there rather than in qom/, because it
calls qdev_get_machine() to find the current accelerator's and
machine's compat_props.

Turns out calling qdev_get_machine() there is problematic.  If we
qdev_create() from a machine's .instance_init() method, we call
device_post_init() and thus qdev_get_machine() before main() can
create "/machine" in QOM.  qdev_get_machine() tries to get it with
container_get(), which "helpfully" creates it as "container" object,
and returns that.  object_apply_compat_props() tries to paper over the
problem by doing nothing when the value of qdev_get_machine() isn't a
TYPE_MACHINE.  But the damage is done already: when main() later
attempts to create the real "/machine", it fails with "attempt to add
duplicate property 'machine' to object (type 'container')", and
aborts.

Since no machine .instance_init() calls qdev_create() so far, the bug
is latent.  But since I want to do that, I get to fix the bug first.

Observe that object_apply_compat_props() doesn't actually need the
MachineState, only its the compat_props member of its MachineClass and
AccelClass.  This permits a simple fix: register MachineClass and
AccelClass compat_props with the object_apply_compat_props() machinery
right after these classes get selected.

This is actually similar to how things worked before commits
ea9ce8934c and b66bbee39f, except we now register much earlier.  The
old code registered them only after the machine's .instance_init()
ran, which would've broken compatibility properties for any devices
created there.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190308131445.17502-2-armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
Markus Armbruster 2019-03-08 14:14:34 +01:00
parent ce14710f4f
commit 1a3ec8c156
4 changed files with 41 additions and 11 deletions

View File

@ -66,6 +66,7 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
*(acc->allowed) = false;
object_unref(OBJECT(accel));
}
object_set_accelerator_compat_props(acc->compat_props);
return ret;
}

View File

@ -978,25 +978,51 @@ static void device_initfn(Object *obj)
QLIST_INIT(&dev->gpios);
}
/*
* Global property defaults
* Slot 0: accelerator's global property defaults
* Slot 1: machine's global property defaults
* Each is a GPtrArray of of GlobalProperty.
* Applied in order, later entries override earlier ones.
*/
static GPtrArray *object_compat_props[2];
/*
* Set machine's global property defaults to @compat_props.
* May be called at most once.
*/
void object_set_machine_compat_props(GPtrArray *compat_props)
{
assert(!object_compat_props[1]);
object_compat_props[1] = compat_props;
}
/*
* Set accelerator's global property defaults to @compat_props.
* May be called at most once.
*/
void object_set_accelerator_compat_props(GPtrArray *compat_props)
{
assert(!object_compat_props[0]);
object_compat_props[0] = compat_props;
}
void object_apply_compat_props(Object *obj)
{
if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
MachineState *m = MACHINE(qdev_get_machine());
MachineClass *mc = MACHINE_GET_CLASS(m);
int i;
if (m->accelerator) {
AccelClass *ac = ACCEL_GET_CLASS(m->accelerator);
if (ac->compat_props) {
object_apply_global_props(obj, ac->compat_props, &error_abort);
}
}
object_apply_global_props(obj, mc->compat_props, &error_abort);
for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
object_apply_global_props(obj, object_compat_props[i],
&error_abort);
}
}
static void device_post_init(Object *obj)
{
/*
* Note: ordered so that the user's global properties take
* precedence.
*/
object_apply_compat_props(obj);
qdev_prop_set_globals(DEVICE(obj));
}

View File

@ -431,6 +431,8 @@ const char *qdev_fw_name(DeviceState *dev);
Object *qdev_get_machine(void);
void object_set_machine_compat_props(GPtrArray *compat_props);
void object_set_accelerator_compat_props(GPtrArray *compat_props);
void object_apply_compat_props(Object *obj);
/* FIXME: make this a link<> */

1
vl.c
View File

@ -3953,6 +3953,7 @@ int main(int argc, char **argv, char **envp)
configure_rtc(qemu_find_opts_singleton("rtc"));
machine_class = select_machine();
object_set_machine_compat_props(machine_class->compat_props);
set_memory_options(&ram_slots, &maxram_size, machine_class);