[PATCH] RFC: qdev: add legacy properties only for those print()-able

[PATCH] RFC: qdev: add legacy properties only for those print()-able

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The link properties are not printed in "info qtree", I don't know if
this was intentional. We currently register legacy properties for
link/ptr properties, but they don't have PropertyInfo getters (only
ObjectPropertyAccessor, when using non-legacy properties)

By not registering a (unusable?) legacy property, "info qtree" can now
print the link.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/core/qdev-properties.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index b7e8a89ba5..fe260a9670 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1143,14 +1143,13 @@ static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop
 {
     g_autofree char *name = NULL;
 
-    /* Register pointer properties as legacy properties */
-    if (!prop->info->print && prop->info->get) {
+    if (!prop->info->print) {
         return;
     }
 
     name = g_strdup_printf("legacy-%s", prop->name);
     object_class_property_add(OBJECT_CLASS(dc), name, "str",
-        prop->info->print ? qdev_get_legacy_property : prop->info->get,
+        qdev_get_legacy_property,
         NULL, NULL, (Property *)prop);
 }
 
-- 
2.51.0

Re: [PATCH] RFC: qdev: add legacy properties only for those print()-able

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]

Hi!

On Wed, Oct 15, 2025 at 2:54 PM <marcandre.lureau@redhat.com> wrote:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The link properties are not printed in "info qtree", I don't know if
> this was intentional. We currently register legacy properties for
> link/ptr properties, but they don't have PropertyInfo getters (only
> ObjectPropertyAccessor, when using non-legacy properties)
>
> By not registering a (unusable?) legacy property, "info qtree" can now
> print the link.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>

Any objections?


> ---
>  hw/core/qdev-properties.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index b7e8a89ba5..fe260a9670 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1143,14 +1143,13 @@ static void
> qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop
>  {
>      g_autofree char *name = NULL;
>
> -    /* Register pointer properties as legacy properties */
> -    if (!prop->info->print && prop->info->get) {
> +    if (!prop->info->print) {
>          return;
>      }
>
>      name = g_strdup_printf("legacy-%s", prop->name);
>      object_class_property_add(OBJECT_CLASS(dc), name, "str",
> -        prop->info->print ? qdev_get_legacy_property : prop->info->get,
> +        qdev_get_legacy_property,
>          NULL, NULL, (Property *)prop);
>  }
>
> --
> 2.51.0
>
>

[-- Attachment #2: Type: text/html, Size: 2379 bytes --]

Re: [PATCH] RFC: qdev: add legacy properties only for those print()-able

[Philippe Mathieu-Daudé]

+Markus

On 15/10/25 12:54, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The link properties are not printed in "info qtree", I don't know if
> this was intentional. We currently register legacy properties for
> link/ptr properties, but they don't have PropertyInfo getters (only
> ObjectPropertyAccessor, when using non-legacy properties)
> 
> By not registering a (unusable?) legacy property, "info qtree" can now
> print the link.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/core/qdev-properties.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index b7e8a89ba5..fe260a9670 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1143,14 +1143,13 @@ static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop
>   {
>       g_autofree char *name = NULL;
>   
> -    /* Register pointer properties as legacy properties */
> -    if (!prop->info->print && prop->info->get) {
> +    if (!prop->info->print) {
>           return;
>       }
>   
>       name = g_strdup_printf("legacy-%s", prop->name);
>       object_class_property_add(OBJECT_CLASS(dc), name, "str",
> -        prop->info->print ? qdev_get_legacy_property : prop->info->get,
> +        qdev_get_legacy_property,
>           NULL, NULL, (Property *)prop);
>   }
>   

Re: [PATCH] RFC: qdev: add legacy properties only for those print()-able

[Markus Armbruster]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The link properties are not printed in "info qtree", I don't know if
> this was intentional. We currently register legacy properties for
> link/ptr properties, but they don't have PropertyInfo getters (only
> ObjectPropertyAccessor, when using non-legacy properties)
>
> By not registering a (unusable?) legacy property, "info qtree" can now
> print the link.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/core/qdev-properties.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index b7e8a89ba5..fe260a9670 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1143,14 +1143,13 @@ static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop
>  {
>      g_autofree char *name = NULL;
>  
> -    /* Register pointer properties as legacy properties */
> -    if (!prop->info->print && prop->info->get) {
> +    if (!prop->info->print) {
>          return;
>      }
>  
>      name = g_strdup_printf("legacy-%s", prop->name);
>      object_class_property_add(OBJECT_CLASS(dc), name, "str",
> -        prop->info->print ? qdev_get_legacy_property : prop->info->get,
> +        qdev_get_legacy_property,
>          NULL, NULL, (Property *)prop);
>  }

The old code confuses me.  Let's go through it real slow.

    /**
     * qdev_class_add_legacy_property:
     * @dev: Device to add the property to.
     * @prop: The qdev property definition.
     *
     * Add a legacy QOM property to @dev for qdev property @prop.
     *
     * Legacy properties are string versions of QOM properties.  The format of
     * the string depends on the property type.  Legacy properties are only
     * needed for "info qtree".
     *
     * Do not use this in new code!  QOM Properties added through this interface
     * will be given names in the "legacy" namespace.
     */
    static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop)
    {
        g_autofree char *name = NULL;

        /* Register pointer properties as legacy properties */

The comment talks about "pointer properties".  We used to call
properties defined with DEFINE_PROP_PTR() that way, but these were
deleted years ago.  The comment is even older.  I'm going to ignore it.

        if (!prop->info->print && prop->info->get) {
            return;
        }

To get here, prop->info->print || !prop->info->get.

        name = g_strdup_printf("legacy-%s", prop->name);
        object_class_property_add(OBJECT_CLASS(dc), name, "str",
            prop->info->print ? qdev_get_legacy_property : prop->info->get,
            NULL, NULL, (Property *)prop);

If qdev property @prop has a .print() method, we create a QOM property
"legacy-FOO" of type "str" with qdev_get_legacy_property() as .get(),
and no .set() or .release().

qdev_get_legacy_property() is a QOM .get() wrapping around qdev
.print(): it calls .print() to format the property value as a string
(arbitrarily limited to 1023 characters), then visits it with
visit_type_str().

Aside: there seems to be just one property that implements .print():
DEFINE_PROP_PCI_DEVFN(), in qdev_prop_pci_devfn.  Quite a lot of
infrastructure just for that.

Else, prop->info->get is null, because prop->info->print || !prop->info->get.
So we create a QOM property "legacy-FOO" with no .get(), .set(),
.release().  Why?

Your patch gets rid of these.  How does this make "info qtree" show link
properties?  Hmm...  qdev_print_props() uses "legacy-FOO" instead of
"FOO" when it exists.  But a "legacy-FOO" without a .get() will fail.
When it does, the property is skipped.

Is this what makes the patch work?

    }

Re: [PATCH] RFC: qdev: add legacy properties only for those print()-able

[Marc-André Lureau]

Hi

On Tue, Oct 21, 2025 at 4:55 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The link properties are not printed in "info qtree", I don't know if
> > this was intentional. We currently register legacy properties for
> > link/ptr properties, but they don't have PropertyInfo getters (only
> > ObjectPropertyAccessor, when using non-legacy properties)
> >
> > By not registering a (unusable?) legacy property, "info qtree" can now
> > print the link.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  hw/core/qdev-properties.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index b7e8a89ba5..fe260a9670 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1143,14 +1143,13 @@ static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop
> >  {
> >      g_autofree char *name = NULL;
> >
> > -    /* Register pointer properties as legacy properties */
> > -    if (!prop->info->print && prop->info->get) {
> > +    if (!prop->info->print) {
> >          return;
> >      }
> >
> >      name = g_strdup_printf("legacy-%s", prop->name);
> >      object_class_property_add(OBJECT_CLASS(dc), name, "str",
> > -        prop->info->print ? qdev_get_legacy_property : prop->info->get,
> > +        qdev_get_legacy_property,
> >          NULL, NULL, (Property *)prop);
> >  }
>
> The old code confuses me.  Let's go through it real slow.
>
>     /**
>      * qdev_class_add_legacy_property:
>      * @dev: Device to add the property to.
>      * @prop: The qdev property definition.
>      *
>      * Add a legacy QOM property to @dev for qdev property @prop.
>      *
>      * Legacy properties are string versions of QOM properties.  The format of
>      * the string depends on the property type.  Legacy properties are only
>      * needed for "info qtree".
>      *
>      * Do not use this in new code!  QOM Properties added through this interface
>      * will be given names in the "legacy" namespace.
>      */
>     static void qdev_class_add_legacy_property(DeviceClass *dc, const Property *prop)
>     {
>         g_autofree char *name = NULL;
>
>         /* Register pointer properties as legacy properties */
>
> The comment talks about "pointer properties".  We used to call
> properties defined with DEFINE_PROP_PTR() that way, but these were
> deleted years ago.  The comment is even older.  I'm going to ignore it.
>
>         if (!prop->info->print && prop->info->get) {
>             return;
>         }
>
> To get here, prop->info->print || !prop->info->get.
>
>         name = g_strdup_printf("legacy-%s", prop->name);
>         object_class_property_add(OBJECT_CLASS(dc), name, "str",
>             prop->info->print ? qdev_get_legacy_property : prop->info->get,
>             NULL, NULL, (Property *)prop);
>
> If qdev property @prop has a .print() method, we create a QOM property
> "legacy-FOO" of type "str" with qdev_get_legacy_property() as .get(),
> and no .set() or .release().
>
> qdev_get_legacy_property() is a QOM .get() wrapping around qdev
> .print(): it calls .print() to format the property value as a string
> (arbitrarily limited to 1023 characters), then visits it with
> visit_type_str().
>
> Aside: there seems to be just one property that implements .print():
> DEFINE_PROP_PCI_DEVFN(), in qdev_prop_pci_devfn.  Quite a lot of
> infrastructure just for that.
>
> Else, prop->info->get is null, because prop->info->print || !prop->info->get.
> So we create a QOM property "legacy-FOO" with no .get(), .set(),
> .release().  Why?
>
> Your patch gets rid of these.  How does this make "info qtree" show link
> properties?  Hmm...  qdev_print_props() uses "legacy-FOO" instead of
> "FOO" when it exists.  But a "legacy-FOO" without a .get() will fail.
> When it does, the property is skipped.
>
> Is this what makes the patch work?
>


Yes, that matches my understanding and I agree about your feeling
about the infrastructure just for DEVFN. But at the same time, it's
nice to have and I don't have a proposal to change that :)

-- 
Marc-André Lureau

Re: [PATCH] RFC: qdev: add legacy properties only for those print()-able

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

[...]

> Yes, that matches my understanding and I agree about your feeling
> about the infrastructure just for DEVFN. But at the same time, it's
> nice to have and I don't have a proposal to change that :)

I have an idea on how to simplify things.  If it works, I'll send a
patch.