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

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);
      ...
  }

> 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.

Conditionally registering properties on instances is an anti-pattern
IMHO. It results in objects that cannot have all their properties
statically introspected.

If a class needs different subsets of properties for different
scenarios, that is potentially a sign there ought to be a base
class and two or more specialized subclasses. Or that the
property needs a more explicit "unset" state in addition to
its other valid values.

For introspection we hack around the use of instance properties
by instantiating every object in order to trigger registration of
instance props.  This is such a gross hack that it periodically 
suffers crashes from objects not expecting to be instantiated
in this context and having undesired side-effects. Class props
are the only sane way to reliably provide introspection without
side-effects.

> 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



> 
> Thanks,
> 
> -- 
> Peter Xu
> 

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 :|