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

[Paolo Bonzini]

On 18/03/20 16:06, Markus Armbruster wrote:
>> - 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?

At least object_initialize_child() adds the property to the object
itself, so it's fine.

sysbus_init_child_obj() adds a link to the object to the sysbus object
(via qdev_set_parent_bus/bus_add_child).  This does violate the rule.
However, currently we have:

- create link on qdev_set_parent_bus, before realizing

- remove link on device_unparent, after unrealizing

which makes the device setup even more complicated than necessary.  In
other words I don't think the bug is in the function, instead it
probably makes sense to do something else:

- create link in device_set_realized, before calling ->pre_plug (undo on
failure)

- remove link in device_set_realized, after calling ->unrealize (if it
succeeds)

and leave sysbus_init_child_obj() as is.

>> - 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?

Yes, guest-visible state is off limits until realization.

>> - 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?

That's the lower bound of descriptivity.  The upper bound is anything
that is visible from the guest.  The truth could be in the middle.

Paolo