Re: [PATCH v2] qdev: Clarify instantiation and realization

[Markus Armbruster <armbru@redhat.com>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 13 Jul 2026 at 12:21, Akihiko Odaki
> <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> On 2026/07/13 18:13, Peter Maydell wrote:
>> > On Mon, 29 Jun 2026 at 09:18, Akihiko Odaki
>> > <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>> >>
>> >> The distinction of instantiation and realization was vague in the old
>> >> documentation so this change clarifies it.
>> >>
>> >> The old documentation said:
>> >>> The former may not fail (and must not abort or exit, since it is
>> >>> called during device introspection already), and the latter may return
>> >>> error information to the caller and must be re-entrant.
>> >>> Trivial field initializations should go into #TypeInfo.instance_init.
>> >>> Operations depending on @props static properties should go into
>> >>> @realize.
>> >>
>> >> The first problem with the old documentation is that it is unclear what
>> >> "trivial field initializations" means and why triviality makes
>> >> initialization appropriate for #TypeInfo.instance_init. Another problem
>> >> is that the documentation is not comprehensive enough; for example, it
>> >> mentions @props static properties, but it does not say anything about
>> >> the other properties.
>> >>
>> >> The keys to distinguish instantiation and realization are property
>> >> setting and device introspection. The fact that property setting happens
>> >> after #TypeInfo.instance_init and before realization implies that
>> >> operations depending on properties should go into @realize.
>> >>
>> >> The fact that instantiation happens during device introspection but
>> >> realization does not implies:
>> >> - Instance properties should be added in #TypeInfo.instance_init.
>> >> - Instantiation must not have any side effect not contained in the
>> >>    instance.
>> >> - Any operations without special requirements should go into @realize so
>> >>    that they can be skipped during device introspection.
>> >>
>> >> Note these two facts to guide appropriate instantiation and realization.
>> >
>> > Thanks for sending this. I think we definitely need to try to be
>> > clearer about what to do in instance_init and what to do in realize.
>> >
>> >> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> >
>> >>    * Realization
>> >>    * -----------
>> >>    *
>> >> - * Devices are constructed in two stages:
>> >> - *
>> >> - * 1) object instantiation via object_initialize() and
>> >> - * 2) device realization via the #DeviceState.realized property
>> >> - *
>> >> - * The former may not fail (and must not abort or exit, since it is called
>> >> - * during device introspection already), and the latter may return error
>> >> - * information to the caller and must be re-entrant.
>> >> - * Trivial field initializations should go into #TypeInfo.instance_init.
>> >> - * Operations depending on @props static properties should go into @realize.
>> >> + * Devices are constructed in the following order:
>> >> + *
>> >> + * 1) #TypeInfo.instance_init
>> >> + * 2) pre-realize property value setting

I find "pre-realize" confusing.  Suggest

          2) Set property values

>> >> + * 3) device realization via the #DeviceState.realized property

"via the #DeviceState.realized property" feels like detail.

>> >> + *
>> >> + * #TypeInfo.instance_init may not fail, and realization may return
>> >> + * error information to the caller and must be re-entrant.
>> >
>> > I know this is in the existing text, but what do we mean by
>> > "must be re-entrant" here ? Does this mean "even if you return
>> > a failure from your realize method you must be able to handle the
>> > caller retrying the realize operation" ?  (I'm not sure why that's
>> > a useful thing for the caller to be able to do...)>
>> > I assume we don't mean "even if you successfully realize you must
>> > handle (and treat as a no-op) a second call to realize", because
>> > none of our devices do that :-)
>>
>> If you take the code literally, device_set_realized() indeed avoids
>> calling realize after successful realization, but it doesn't report an
>> error when retrying, so I think this means to allow that.

Note that device_set_realized() is the setter of device property
"realized".

Setting a property to the value it already has is commonly a no-op, not
an error.

>> I don't think it's that useful, and it is likely to cause bunch of bugs.
>> I can also imagine that careful developers have written device code that
>> preserves this contract, though the added complexity is useless in practice.
>>
>> In my opinion, the property should be tristate: initialized, realized,
>> or retired. Then we would have only the following state transitions:
>>
>> Successful realization: initialized -> realized
>> Failed realization: initialized -> retired
>> Unrealization: realized -> retired

Maybe, but this is not the state machine we have.

>> This is another opportunity for refactoring, but for this particular
>> patch I'm planning to keep it out of scope.
>
> OK. We could make the text clearer then, I think. Maybe
>
> "#TypeInfo.instance_init may not fail. #DeviceClass.realize can
> fail, returning error information to the caller. A device realize
> method should handle being called again after it has failed once."
>
> ?

In general, a function should either do its job, or fail cleanly,
i.e. without side effects.

For .realize() "without side effects" includes:

* The failed call is invisible to the guest.

* The device behaves as if the failed call never happened.  This means
  you can try .realize() again.

Does the comment need to spell this out?

[...]