Daniel P. Berrangé <[email protected]> writes: > On Fri, Sep 18, 2026 at 11:04:31AM +0200, Markus Armbruster wrote: >> Mark Cave-Ayland <[email protected]> writes: >> >> > On 17/09/2026 09:52, Marc-André Lureau wrote: >> > >> >> Hi >> >> On Wed, Sep 2, 2026 at 6:42 PM Mark Cave-Ayland >> >> <[email protected]> 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 <[email protected]> >> >>> --- >> >>> 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.
Good points. Work into object.h comments?
