Re: [PATCH v2 05/10] qom: Create object-property-ptr.[ch]

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> On Wed, Jun 10, 2026 at 05:15:59PM +0100, Daniel P. Berrangé wrote:
>> On Tue, Jun 09, 2026 at 01:25:09PM -0400, Peter Xu wrote:
>> > Create object-property-ptr.[ch] files to include all the helpers for
>> > object_property_add*_ptr().
>> > 
>> > These set of helpers are handy because they look extremely familiar with
>> > qdev-properties, allowing the caller to provide a pointer and it will
>> > manage all the setters and getters.
>> > 
>> > The follow up patches may introduce more of such helpers.  Since object.c
>> > has been already too big, split that part out.
>> 
>> The "ptr" helpers are all instance level properties which is a concept
>> we discourage from new usage, in favour of class level properties.
>> 
>> I don't think we should be adding more "ptr" helpers, but rather
>> planning to eliminiate the (surprisingly little) usage of the
>> existing ones.
>
> The other way to do similar thing is qdev's offset way, but IMHO that's
> more awkward to remember an offset of a pointer then do math everytime.
> Essentially, from technical pov we need at least one uintptr_t to store
> either (1) offset, or (2) field pointer when there's a field that is bound
> to a prop.  IMHO (2) can be better otherwise we'll need to do all the maths
> to calculate offsets then when access we add the offset back and do a force
> cast.  It seems not necessary.
>

The more limiting aspect of using the offset is that MigrationParameters
needs to be embedded into MigrationState because the Object is
MigrationState and not MigrationParameters. We could try to solve that
issue specifically by making MigrationParameters a standalone object.

The main issue is that we would need to figure out how to set the compat
properties that are not part of MigrationParameters.

The other main issue =) is that we'd need to wrap this in another
structure because MigrationParameters is generated by QAPI.

A minor issue is that -object migration and -global migration use the
"migration" word from TYPE_MIGRATION, so the MigrationState would have
to be renamed.

We could actually solve the first two by doing similarly to what I did
in qom.json in my reply to the cover-letter:

struct MigrationOptions {
   MigrationParameters params;
   <compat props>
}

... then access it by offset. This is fine because we can then do
whatever we want with the MigrationOptions pointer without having to
follow the lifetime of MigrationState.

> OTOH, I still see value on non-class instance properties (that sometimes we
> don't even want to have some props avail for the class, but conditional to
> some instances when created dynamically).  If that is needed, IMHO it's
> fine we still provide per-instance properties.
>
> Is there any pointer I can read about the discussion previously on this?
>
> Thanks,