Re: [PATCH v2 05/10] qom: Create object-property-ptr.[ch]
Fabiano Rosas <[email protected]> Wed, 10 Jun 2026 17:37:56 -0300
| Newsgroups | org.nongnu.qemu-rust,org.nongnu.qemu-arm,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
Peter Xu <[email protected]> writes: > On Wed, Jun 10, 2026 at 05:15:59PM +0100, Daniel P. Berrang=C3=A9 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(). >> >=20 >> > These set of helpers are handy because they look extremely familiar wi= th >> > qdev-properties, allowing the caller to provide a pointer and it will >> > manage all the setters and getters. >> >=20 >> > The follow up patches may introduce more of such helpers. Since objec= t.c >> > has been already too big, split that part out. >>=20 >> The "ptr" helpers are all instance level properties which is a concept >> we discourage from new usage, in favour of class level properties. >>=20 >> 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 mat= hs > to calculate offsets then when access we add the offset back and do a for= ce > 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 =3D) 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,