Re: [Qemu-devel] [PATCH v2] qom: Introduce object_realize_nofail()

[Andreas Färber]

Am 12.04.2012 21:59, schrieb Anthony Liguori:
> On 04/12/2012 11:52 AM, Andreas Färber wrote:
>> Am 12.04.2012 17:41, schrieb Anthony Liguori:
>>> On 04/12/2012 09:04 AM, Andreas Färber wrote:
>>>> Am 12.04.2012 16:59, schrieb Paolo Bonzini:
>>>>> Il 12/04/2012 16:47, Anthony Liguori ha scritto:
>>>>>>
>>>>>>> Wrap setting of Object::realized property, error reporting and
>>>>>>> exit(1)
>>>>>>> into a helper function. It is the equivalent of qdev_init_nofail().
>>>>>>
>>>>>> I don't like this.
>>>>>>
>>>>>> If for no reason other than, a much more specific justification is
>>>>>> needed for this.  I absolutely don't want to repeat the error
>>>>>> handling
>>>>>> mistakes of qdev.  I would rather we refactor all of the users of
>>>>>> qdev_init_nofail() to propagate errors.
>>>>>
>>>>> I agree about this in general, but for a different reason.  There
>>>>> should be
>>>>> only one call to object_realize_nofail, in vl.c, which might as
>>>>> well be
>>>>> inlined---I'll include it in my series.  All calls to qdev_init_nofail
>>>>> and qdev_init should disappear from boards that are properly converted
>>>>> to QOM.
>>>>
>>>> We had talked about this on IRC and found that it won't work for
>>>> selecting the type of an object.
>>>
>>> Can you be more specific?
>>>
>>> Selecting the type of an object should be done by having a link<>
>>> property and letting the user create an object and setup the link.
>>
>> No, that's not what a link does. A link<>  property lets the user
>> associate one instance, not a type that can be used for multiple
>> instances. We'd need a template<>  or clone<>  mechanism for that!
> 
> No, you misunderstood.  A link allows a user to create an object and
> make an associate.  If you want to let a user choose a type, just have
> them make an object.  Since you need no parameters to create an object,
> there's no advantage of telling something about a type name vs. giving
> it an object of that type.

You're missing the point: You are advocating putting all configuration
into a config file, having the user call --readconfig
/path/to/board.conf and either fiddling with the config file or fiddling
with (looping) QMP scripts to apply machine-wide changes.

By contrast I am talking about backwards-compatible convenience options
that influence more than one object in such a machine, wherever that
hierarchy is being built up.

Reality with SysBus is multi-stage constructors:
A = qdev_create()
A.a = x
qdev_init_nofail(A) -> A_initfn() -> B = qdev_create()
                                     B.b = y
                                     qdev_init_nofail(B) -> B_initfn()
...

while you are dreaming of a flat hierarchy:

A = qdev_create()
B = qdev_create()
...
A.a = x
B.b = y
...
object_property_set_bool(qdev_get_machine(), "realized", true)

But getting there requires more than denying me object_realize(), it
requires rewriting all qdev devices *first*, which I do not see Paolo
doing. You started for i440fx but did not finish either.

Calling realize recursively shouldn't hurt us if realizing an already
realized object is no-op and if the child iteration can cope with a
growing child list for the current parent and any node in /machine.
Meaning we can't just call one inlined object_realize_nofail() but we'd
need to loop doing that until no node in the subgraph is left that is
not realized.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg