Re: [RFC 02/10] qom: Add basic object property deprecation hint support

[Igor Mammedov <imammedo@redhat.com>]

On Wed,  3 Dec 2025 01:04:54 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:

> Now the common (but a bit fragmented) way to mark a property deprecated
> is to add the warning in its accssors.
> 
> But this is pretty inconvenient for such qdev properties, which are
> defined via DEFINE_PROP_* macros in the Property array. For qdev
> properties, their accessors are provided by pre-defined PropertyInfo, so
> that it's possible to modify PropertyInfo for a single "deprecated"
> property.
> 
> Then it's necessary to introduce property flags to mark some properties
> as deprecated, and to check the property flags when set the property,
> thereby to print a deprecation warning.
> 
> This not only benefits traditional qdev properties but also helps the
> deprecation of generic objects.
> 
> Note, internal attempt (except the compat case) should not trigger the
> deprecation warning but external user should see the deprecation
> information. Whether to perform deprecation checks based on property
> flags is controlled by the newly added "check" argument in
> object_property_try_add_full().

I'd split deprecation warning out for this patch,
i.e. make this one "add per object instance flags",
and take care of deprecation stuff on top,

Also, API likely would need set/get/clear calls to operate on object flags.

> 
> In subsequent work, the "check" option will be enabled for specific
> external property setting paths.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  include/qom/object.h | 72 ++++++++++++++++++++++++++++++++++++++++++++
>  qom/object.c         | 72 +++++++++++++++++++++++++++++++++++---------
>  2 files changed, 130 insertions(+), 14 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 3f807a03f5aa..8f4c2f44d835 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -86,6 +86,12 @@ typedef void (ObjectPropertyRelease)(Object *obj,
>   */
>  typedef void (ObjectPropertyInit)(Object *obj, ObjectProperty *prop);
>  
> +typedef enum ObjectPropertyFlags {
> +    OBJECT_PROPERTY_NO_FLAGS     =                             0,
> +    OBJECT_PROPERTY_DEPRECATED   =                        1 << 0,

maybe use BIT() instead of manual shift?

addidtionally given you are going to distinguish external vs internal,
perhaps add flags 'default' and 'user set',
I think the both could be used to cleanup cpu flags handling where we rely on
setting/checking  magic numbers to figure out where value comes from.


> +    OBJECT_PROPERTY_FULL_FLAGS   =    OBJECT_PROPERTY_DEPRECATED,
> +} ObjectPropertyFlags;
> +
>  struct ObjectProperty
>  {
>      char *name;
> @@ -98,6 +104,7 @@ struct ObjectProperty
>      ObjectPropertyInit *init;
>      void *opaque;
>      QObject *defval;
> +    uint8_t flags;
>  };
>  
>  /**
> @@ -1090,6 +1097,41 @@ ObjectProperty *object_property_try_add(Object *obj, const char *name,
>                                          ObjectPropertyRelease *release,
>                                          void *opaque, Error **errp);
>  
> +/**
> + * object_property_try_add_full:

what's the reason for adding _full flavour over just modifying existing API?

> + *
> + * Same as object_property_try_add() with more arguments (@flags).
> + *
> + * @obj: the object to add a property to
> + * @name: the name of the property.  This can contain any character except for
> + *  a forward slash.  In general, you should use hyphens '-' instead of
> + *  underscores '_' when naming properties.
> + * @type: the type name of the property.  This namespace is pretty loosely
> + *   defined.  Sub namespaces are constructed by using a prefix and then
> + *   to angle brackets.  For instance, the type 'virtio-net-pci' in the
> + *   'link' namespace would be 'link<virtio-net-pci>'.
> + * @get: The getter to be called to read a property.  If this is NULL, then
> + *   the property cannot be read.
> + * @set: the setter to be called to write a property.  If this is NULL,
> + *   then the property cannot be written.
> + * @release: called when the property is removed from the object.  This is
> + *   meant to allow a property to free its opaque upon object
> + *   destruction.  This may be NULL.
> + * @flags: the flags to control property uses.
> + * @opaque: an opaque pointer to pass to the callbacks for the property
> + * @errp: pointer to error object
> + *
> + * Returns: The #ObjectProperty; this can be used to set the @resolve
> + * callback for child and link properties.
> + */
> +ObjectProperty *object_property_try_add_full(Object *obj, const char *name,
> +                                             const char *type,
> +                                             ObjectPropertyAccessor *get,
> +                                             ObjectPropertyAccessor *set,
> +                                             ObjectPropertyRelease *release,
> +                                             ObjectPropertyFlags flags,
> +                                             void *opaque, Error **errp);
> +
>  /**
>   * object_property_add:
>   * Same as object_property_try_add() with @errp hardcoded to
> @@ -1128,6 +1170,15 @@ ObjectProperty *object_class_property_add(ObjectClass *klass, const char *name,
>                                            ObjectPropertyRelease *release,
>                                            void *opaque);
>  
> +ObjectProperty *object_class_property_add_full(ObjectClass *klass,
> +                                               const char *name,
> +                                               const char *type,
> +                                               ObjectPropertyAccessor *get,
> +                                               ObjectPropertyAccessor *set,
> +                                               ObjectPropertyRelease *release,
> +                                               ObjectPropertyFlags flags,
> +                                               void *opaque);
> +
>  /**
>   * object_property_set_default_bool:
>   * @prop: the property to set
> @@ -1469,6 +1520,27 @@ int object_property_get_enum(Object *obj, const char *name,
>  bool object_property_set(Object *obj, const char *name, Visitor *v,
>                           Error **errp);
>  
> +/**
> + * object_property_set_full:
> + *
> + * Same as object_property_set() with extra check over property flags
> + * (ObjectPropertyFlags) if @check is true.
> + *
> + * @obj: the object
> + * @name: the name of the property
> + * @v: the visitor that will be used to write the property value.  This should
> + *   be an Input visitor and the data will be first read with @name as the
> + *   name and then written as the property value.
> + * @check: whether to check if the property can be set over property flags.
> + * @errp: returns an error if this function fails
> + *
> + * Writes a property to a object.
> + *
> + * Returns: %true on success, %false on failure.
> + */
> +bool object_property_set_full(Object *obj, const char *name,
> +                              Visitor *v, bool check, Error **errp);
> +
>  /**
>   * object_property_parse:
>   * @obj: the object
> diff --git a/qom/object.c b/qom/object.c
> index 85d31bb64b36..184afc6730dd 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1234,15 +1234,17 @@ void object_unref(void *objptr)
>  }
>  
>  ObjectProperty *
> -object_property_try_add(Object *obj, const char *name, const char *type,
> -                        ObjectPropertyAccessor *get,
> -                        ObjectPropertyAccessor *set,
> -                        ObjectPropertyRelease *release,
> -                        void *opaque, Error **errp)
> +object_property_try_add_full(Object *obj, const char *name, const char *type,
> +                             ObjectPropertyAccessor *get,
> +                             ObjectPropertyAccessor *set,
> +                             ObjectPropertyRelease *release,
> +                             ObjectPropertyFlags flags, void *opaque, Error **errp)
>  {
>      ObjectProperty *prop;
>      size_t name_len = strlen(name);
>  
> +    assert(!flags || !(flags & ~(OBJECT_PROPERTY_FULL_FLAGS)));
> +
>      if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
>          int i;
>          ObjectProperty *ret = NULL;
> @@ -1279,11 +1281,24 @@ object_property_try_add(Object *obj, const char *name, const char *type,
>      prop->set = set;
>      prop->release = release;
>      prop->opaque = opaque;
> +    prop->flags = flags;
>  
>      g_hash_table_insert(obj->properties, prop->name, prop);
>      return prop;
>  }
>  
> +ObjectProperty *
> +object_property_try_add(Object *obj, const char *name, const char *type,
> +                        ObjectPropertyAccessor *get,
> +                        ObjectPropertyAccessor *set,
> +                        ObjectPropertyRelease *release,
> +                        void *opaque, Error **errp)
> +{
> +    return object_property_try_add_full(obj, name, type, get, set, release,
> +                                        OBJECT_PROPERTY_NO_FLAGS, opaque,
> +                                        errp);
> +}
> +
>  ObjectProperty *
>  object_property_add(Object *obj, const char *name, const char *type,
>                      ObjectPropertyAccessor *get,
> @@ -1296,17 +1311,19 @@ object_property_add(Object *obj, const char *name, const char *type,
>  }
>  
>  ObjectProperty *
> -object_class_property_add(ObjectClass *klass,
> -                          const char *name,
> -                          const char *type,
> -                          ObjectPropertyAccessor *get,
> -                          ObjectPropertyAccessor *set,
> -                          ObjectPropertyRelease *release,
> -                          void *opaque)
> +object_class_property_add_full(ObjectClass *klass,
> +                               const char *name,
> +                               const char *type,
> +                               ObjectPropertyAccessor *get,
> +                               ObjectPropertyAccessor *set,
> +                               ObjectPropertyRelease *release,
> +                               ObjectPropertyFlags flags,
> +                               void *opaque)
>  {
>      ObjectProperty *prop;
>  
>      assert(!object_class_property_find(klass, name));
> +    assert(!flags || !(flags & ~(OBJECT_PROPERTY_FULL_FLAGS)));
>  
>      prop = g_malloc0(sizeof(*prop));
>  
> @@ -1317,12 +1334,27 @@ object_class_property_add(ObjectClass *klass,
>      prop->set = set;
>      prop->release = release;
>      prop->opaque = opaque;
> +    prop->flags = flags;
>  
>      g_hash_table_insert(klass->properties, prop->name, prop);
>  
>      return prop;
>  }
>  
> +ObjectProperty *
> +object_class_property_add(ObjectClass *klass,
> +                          const char *name,
> +                          const char *type,
> +                          ObjectPropertyAccessor *get,
> +                          ObjectPropertyAccessor *set,
> +                          ObjectPropertyRelease *release,
> +                          void *opaque)
> +{
> +    return object_class_property_add_full(klass, name, type, get, set,
> +                                          release, OBJECT_PROPERTY_NO_FLAGS,
> +                                          opaque);
> +}
> +
>  ObjectProperty *object_property_find(Object *obj, const char *name)
>  {
>      ObjectProperty *prop;
> @@ -1432,8 +1464,8 @@ bool object_property_get(Object *obj, const char *name, Visitor *v,
>      return !err;
>  }
>  
> -bool object_property_set(Object *obj, const char *name, Visitor *v,
> -                         Error **errp)
> +bool object_property_set_full(Object *obj, const char *name,
> +                              Visitor *v, bool check, Error **errp)
>  {
>      ERRP_GUARD();
>      ObjectProperty *prop = object_property_find_err(obj, name, errp);
> @@ -1447,10 +1479,22 @@ bool object_property_set(Object *obj, const char *name, Visitor *v,
>                     object_get_typename(obj), name);
>          return false;
>      }
> +
> +    if (check && prop->flags & OBJECT_PROPERTY_DEPRECATED) {
> +        warn_report("Property '%s.%s' has been deprecated. "
> +                    "Please do not use it.", object_get_typename(obj), name);
> +    }
> +
>      prop->set(obj, v, name, prop->opaque, errp);
>      return !*errp;
>  }
>  
> +bool object_property_set(Object *obj, const char *name, Visitor *v,
> +                         Error **errp)
> +{
> +    return object_property_set_full(obj, name, v, false, errp);
> +}
> +
>  bool object_property_set_str(Object *obj, const char *name,
>                               const char *value, Error **errp)
>  {