On Fri, Apr 26, 2019 at 04:55:17PM +0800, Like Xu wrote:
> On 2019/4/26 4:00, Eduardo Habkost wrote:
> > This series moves some qdev code outside qdev.o, so it can be
> > compiled only in CONFIG_SOFTMMU.
> >
> > The code being moved includes two qdev_get_machine() calls, so
> > this will make it easier to move qdev_get_machine() to
> > CONFIG_SOFTMMU later.
> >
> > After this series, there's one remaining qdev_get_machine() call
> > that seems more difficult to remove:
> >
> > static void device_set_realized(Object *obj, bool value, Error **errp)
> > {
> > /* [...] */
> > if (!obj->parent) {
> > gchar *name = g_strdup_printf("device[%d]",
> > unattached_count++);
> >
> > object_property_add_child(container_get(qdev_get_machine(),
> > "/unattached"),
> > name, obj, &error_abort);
> > unattached_parent = true;
> > g_free(name);
> > }
> > /* [...] */
> > }
> >
>
> I may have an experimental patch to fix device_set_realized issue:
>
> 1. in qdev_get_machine():
> replace
> dev = container_get(object_get_root(), "/machine");
> with
> dev = object_resolve_path("/machine", NULL);
>
> 2. in device_set_realized():
>
> Using
> Object *container = qdev_get_machine() ?
> qdev_get_machine() : object_get_root();
> and pass it to
> object_property_add_child(
> container_get(container, "/unattached"),
> name, obj, &error_abort);
I wouldn't like to call qdev_get_machine() here (see below[1]),
but trying "/machine" first and falling back to object_get_root()
sounds good, for two reasons:
* It won't require "/machine" to exist at all, on *-user;
* It will allow machines to create unattached devices on
instance_init without crashing (which is not possible today).
>
> With this fix, we could say the qdev_get_machine() does
> return the "/machine" object (or null) not a confused "/container".
>
> We could continue to use qdev_get_machine() in system emulation mode,
> getting rid of its surprising side effect as Markus said.
>
> The return value of qdev_get_machine() in user-only mode
> is the same object returned by object_get_root(),
> so no semantic changes.
[1] I wouldn't like to have a different qdev_get_machine()
function in user-only mode. I would like to simply not call
qdev_get_machine() at all unless we're in system emulation mode.
--
Eduardo
