Re: [PATCH v3 2/8] qom/object.c: introduce object_class_property_add_alias()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Sep 18, 2026 at 11:04:31AM +0200, Markus Armbruster wrote:
> Mark Cave-Ayland <mark.caveayland@nutanix.com> writes:
> 
> > On 17/09/2026 09:52, Marc-André Lureau wrote:
> >
> >> Hi
> >> On Wed, Sep 2, 2026 at 6:42 PM Mark Cave-Ayland
> >> <mark.caveayland@nutanix.com> wrote:
> >>>
> >>> This is eventually intended to be a replacement for object_property_add_alias()
> >>> which uses an object property instead of a class property.
> >>>
> >>> With the advent of class properties, it is possible that QOM may attempt to
> >>> set or retrieve the value of an unset alias property. Update the existing
> >>> property_get_alias() and property_set_alias() functions to use the null visitor
> >>> if the alias target has not been set, and property_resolve_alias() to return
> >>> NULL for the same case.
> >>>
> >>> This ensures that unset class alias properties accessed e.g. via the monitor do
> >>> not cause QEMU to crash.
> >>>
> >>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> >>> ---
> >>>   include/qom/object.h |  25 +++++++++++
> >>>   qom/object.c         | 100 +++++++++++++++++++++++++++++++++++++------
> >>>   2 files changed, 113 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/include/qom/object.h b/include/qom/object.h
> >>> index e24f0a2b2d..96c76975ef 100644
> >>> --- a/include/qom/object.h
> >>> +++ b/include/qom/object.h
> >>> @@ -2263,6 +2263,11 @@ ObjectProperty *object_class_static_property_add_uint64_ptr(ObjectClass *klass,
> >>>                                             const uint64_t *v,
> >>>                                             ObjectPropertyFlags flags);
> >>>
> >>> +typedef enum {
> >>> +    /* private */
> >>> +    OBJ_PROP_ALIAS_CLASS = 0x1,
> >>> +} ObjectPropertyAliasFlags;
> >>> +
> >>>   /**
> >>>    * object_property_add_alias:
> >>>    * @obj: the object to add a property to
> >>> @@ -2283,6 +2288,26 @@ ObjectProperty *object_class_static_property_add_uint64_ptr(ObjectClass *klass,
> >>>   ObjectProperty *object_property_add_alias(Object *obj, const char *name,
> >>>                                  Object *target_obj, const char *target_name);
> >>>
> >>> +/**
> >>> + * object_class_property_add_alias:
> >>> + * @klass: the object class to add a property to
> >>> + * @name: the name of the property
> >>> + * @offset: the offset from the object instance where the object alias is
> >>> + *   stored
> >>> + * @target_type: QOM type we expect the alias to resolve to
> >>> + * @target_name: the name of the property on the forwarded object
> >>> + *
> >>> + * Add an alias for a property on an object.  This function will add a property
> >>> + * of the same type as the forwarded property.
> >>> + *
> >>> + * Returns: The newly added property on success, or %NULL on failure.
> >> actually, it assert() on error and never returns NULL.
> >
> > I was able to reproduce this by attempting to add a class alias property with the same name again, and then started to look at other related functions do to understand what the behaviour should be. There seems to be a number of existing issues here:
> >
> > - object_property_try_add() returns NULL in the case a property cannot
> >   be created (which seems to be only when an attempt to make add a
> >   property that already exists). However since object_property_add()
> >   always passes &error_abort, then QEMU terminates immediately without
> >   returning the NULL.
> 
> Convention: when we have a pair of functions FOO_try_BAR() and
> FOO_BAR(), the former can fail and the latter cannot.  The latter is
> useful when failure would be a programming error.
> 
> When FOO_try_BAR() sets an Error on failure, FOO_BAR() is a thin wrapper
> that passes &error_abort.
> 
> We pass &error_abort when failure would be a programming error.  It's
> more concise than "call then assert it didn't fail".  Concise is good.
> 
> The wrapper makes calls that cannot fail even more concise.  Worth
> having only when there are plenty of callers that profit from that.
> Someone decided this is the case for object_property_try_add() /
> object_property_add().
> 
> > - object_class_property_add() will currently assert() if an attempt is
> >   made to add a duplicate property: it is fairly trivial to update it to
> >   return the same error that object_property_add() does, however it is
> >   interesting to note that the kernel-doc states that
> >   error_setg(&error_abort, ...) should NOT be used. However in this case
> >   it appears to return a genuinely helpful message which I think is
> >   worth keeping.
> 
> The only difference between object_class_property_add() and
> object_property_add() should be where the property is recorded (class
> vs. object).  In particular, they should treat failure the same way.
> 
> This may make object_class_property_try_add() necessary.

IMHO such a method should not exist. A class must have a well-defined
set of properties that is invariant. Any scenario where a
object_class_property_try_add() method could fail would be 100%
repeatable and at the same time a significant programmer error.


When we had properties against objects 'try_add' was passable, as
we're dynamically adding props and different instances  can have
different props, even when the same time. This is all a terrible
idea, and ideally object_property_try_add would not exist either
but it is probably more trouble than its worth right now to attempt
removing object_property_try_add. Just don't mirror it into class
properties.


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