Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/03/20 14:02, Markus Armbruster wrote:
>> Object instantiation must be completely reverted by finalization.
>> 
>> device-introspect-test guards against a particularly egregious violation
>> of this rule, namely output of "info qtree" after initialization +
>> finalization differing from output before.  Surprisingly common issue.
>> It's what made Peter invite me to this thread.
>> 
>> Device realization must be completely reverted by unrealization.
>
> So far so good.
>
>> We don't always implement unrealize.  Fine when the device cannot be
>> unrealized.  But the code doesn't seem to guard against omitting
>> unrealize when the device actually can be unrealized.
>> 
>> Properties for use with device_add must exist after object
>> instantiation.
>> 
>> But is that true?  Setting a property can run arbitrary code.  What if
>> setting property P to value V runs code that adds property Q?  Then
>> device_add P=V,Q=W works provided P gets set before Q, which is
>> anybody's guess.
>> 
>> So "must exist" is actually "should exist", and we' already moved from
>> the self-evident to the unclear.
>
> Right, and there is already one case where the properties don't exist,
> namely the "fake array" properties.

I tried to strangle them in their crib, but failed.

>> Even less clear: what side effects may be visible between object
>> initialization and realization / finalization?
>> 
>> A safe but constricting answer would be "only host resource
>> reservation".
>> 
>> What's your answer?
>
> Here are some random thoughts about it:
>
> - object initialization should cause no side effects outside the subtree
> of the object

object_initialize_child() and its users like sysbus_init_child_obj()
violate this rule: they add a child property to the subtree's parent.
Correct?

> - setting properties can cause side effects outside the subtree of the
> object (e.g. marking an object as "in use"), but they must be undone by
> finalization.

Textbook example is the 1:1 connection between device frontend and
backend: block frontend's property "drive", char frontend's property
"chardev", NIC frontend property "netdev", ...

Can we come up with some guardrails for what property setting may do?
Affecting guest-visible state is off limits.  What else is?

> - realization can also cause side effects outside the subtree of the
> object, but if unrealization is possible, they must be undone by
> unrealization. if an object is realized and unrealization is not
> possible, finalization will never run (and in that case, side effects of
> realization need not be undone at all).

One possible answer the question what should be done in realize() is
whatever must not be done earlier.  Is that the best we can do?