On 2025/12/12 0:57, Peter Xu wrote:
On Fri, Dec 12, 2025 at 12:28:21AM +0900, Akihiko Odaki wrote:
I'm actually less sure if the first half makes sense than the second half.

Exporting Property from QDEV did make me think it may be polluting the
codebase with QDEV-specifics, but its motivation (creating properties
without defining accessors) is clear and it meets its requirement.

The motivation is less concrete for the first half. It says the current code
is explicit, but it is a relative term. I'm not sure if specifying
TYPE_OBJECT_COMPAT is less explicit (or more implicit) than calling
object_apply_compat_props(). Also, having a class for compat props but not
for global props makes the decision look arbitrary. It is nice to have some
clarification on these.

Thanks for the comments, Akihiko.  I can explain.

I explicitly only included object_apply_compat_props() in the current
OBJECT_COMPAT class (rather than set_globals()) is almost two-fold:

   - Inclusion of qdev_prop_set_globals() makes the 1st half start to depend
     on the 2nd half, that I don't see it strictly necessary.  E.g., one
     immediate motivation of me looking at this problem is Vladimir's series
     on virtio-net/tap [1].

     That series should only need patch 1-6 of this series as minimum to
     enable machine compat properties for TAP (after QOMify, if that makes
     sense on its own).

It is possible to have another type that inherits TYPE_OBJECT_COMPAT and does the second half.


   - qdev_prop_set_globals() itself has a trivial dependency currently
     against qdev hotpluggable or not:

     object_apply_global_props(OBJECT(dev), global_props(),
                               dev->hotplugged ? NULL : &error_fatal);

     There's way to decouple that I guess, however I just don't see it
     necessary yet.  The only real user will be migration object, which
     doesn't need to depend on this anyway, as long as Property APIs can be
     exported.

Theoretically, there may also be a call of object_apply_compat_props() that has some dependency on its state in the future, but such a future concern should be treated later; I'll focus on existing callers of object_apply_compat_props() and global_props() in the following discussion too.


Do you think we should keep open-code object_apply_compat_props() instead?

Yes, that's my conclusion so far, but please see below for the explanation:


In reality, that also works for me at least on figuring out the current
problem [1], and if Property API exported I can also open code it for
migration object.  It's just that IMHO it's cleaner we have the light class
to do this (so as to avoid providng post_init() hooks all over the places).

Avoiding providing post_init() hooks is a nice concrete explanation of the goal.

That said, the migration object provides post_init() hooks anyway, so the question will be if avoiding post_init() hooks will simplify the existing callers of object_apply_compat_props().

Looking into the codebase, there are four users of object_apply_compat_props(); hostmem, qdev, accel, and sev. Only hostmem and qdev call object_apply_compat_props() in their post_init() hooks, and qdev needs the hook anyway for the same reason the migration object does.

So there is only one post_init() hook added for object_apply_compat_props(): host_memory_backend_post_init(). And even this can be simplified; since object_apply_compat_props() and the hook has the same signature, hostmem can simply do:

.instance_post_init = object_apply_compat_props,

... and drop host_memory_backend_post_init().

This is much simpler than adding the object-compat type, so probably it's too early to add the type.

Regards,
Akihiko Odaki


[1] 
https://lore.kernel.org/all/[email protected]/



Reply via email to