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

On Thu, Jun 11, 2026 at 11:36:46AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <[email protected]> writes:
> 
> > On Wed, Jun 10, 2026 at 02:39:09PM -0400, Peter Xu wrote:
> >> 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.
> >
> > There shouldn't be any need to play with field offsets either. Just
> > define the setters & getters to directly access the fields.
> >
> > Take some samples from the "machine_class_init" in hw/core/machine.c
> > as an example of the normal design pattern:
> >
> >     object_class_property_add_str(oc, "dumpdtb",
> >         machine_get_dumpdtb, machine_set_dumpdtb);
> >     object_class_property_set_description(oc, "dumpdtb",
> >         "Dump current dtb to a file and quit");
> >
> >    ..snip..
> >
> >     object_class_property_add_bool(oc, "dump-guest-core",
> >         machine_get_dump_guest_core, machine_set_dump_guest_core);
> >     object_class_property_set_description(oc, "dump-guest-core",
> >         "Include guest memory in a core dump");
> >
> > These are paired with:
> >
> >   static char *machine_get_dumpdtb(Object *obj, Error **errp)
> >   {
> >       MachineState *ms = MACHINE(obj);
> >
> >       return g_strdup(ms->dumpdtb);
> >   }
> >
> >   static void machine_set_dumpdtb(Object *obj, const char *value, Error **errp)
> >   {
> >       MachineState *ms = MACHINE(obj);
> >
> >       g_free(ms->dumpdtb);
> >       ms->dumpdtb = g_strdup(value);
> >   }
> >
> > and
> >
> >   static bool machine_get_dump_guest_core(Object *obj, Error **errp)
> >   {
> >       MachineState *ms = MACHINE(obj);
> >
> >       return ms->dump_guest_core;
> >   }
> >
> >   static void machine_set_dump_guest_core(Object *obj, bool value, Error **errp)
> >   {
> >       MachineState *ms = MACHINE(obj);
> >
> >       if (!value && QEMU_MADV_DONTDUMP == QEMU_MADV_INVALID) {
> >           error_setg(errp, "Dumping guest memory cannot be disabled on this host");
> >           return;
> >       }
> >       ms->dump_guest_core = value;
> >   }
> >
> >
> > and defaults (if needed) are set in  the instance init method:
> >
> >   static void machine_initfn(Object *obj)
> >   {
> >       MachineState *ms = MACHINE(obj);
> >       MachineClass *mc = MACHINE_GET_CLASS(obj);
> >
> >       ms->dump_guest_core = true;
> >       ms->mem_merge = (QEMU_MADV_MERGEABLE != QEMU_MADV_INVALID);
> >       ...
> >   }
> >
> 
> Thanks for the practical example. In this usage, do you think there
> would be a way to avoid having 53 getters and setters? We often run into
> this issue of the rest of QEMU having just a handful of options and
> migration having a ton. Anything we can do to streamline this would be
> good.

There's nothing standard in QOM, but in patch 10 in this series
Peter has a DEFINE_TLS_PROP_HELPERS() which macro-ized the
repetitive getters/setters for the TLS string properties. The
duplicate code still exists in the binary of course, just hidden
from the source.


> >> Is there any pointer I can read about the discussion previously on this?
> >
> > I don't know that there's a particular thread in recent times
> > that I'd point to, just my ancient series from when I introduced
> > class properties for QOM back in 2015 and started the effort to
> > convert away from instances properties.
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg03112.html
> >
> 
> Anything we can do to avoid stumbling into this again? Should we add
> some words to object_property_add() discouraging instance properties?

Yeah, it is probably worth adding some guidance to the docs there.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|