1. Patchew
  2. QEMU
  3. View series

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

marcandre.lureau@redhat.com posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251015105419.2975542-1-marcandre.lureau@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
hw/core/qdev-properties.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[PATCH] RFC: qdev: add legacy properties only for those print()-able
Posted by marcandre.lureau@redhat.com 1 month ago 
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
Posted by Markus Armbruster 3 weeks, 3 days ago 
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
Posted by Marc-André Lureau 3 weeks, 3 days ago 
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
Posted by Markus Armbruster 3 weeks, 2 days ago 
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.
Re: [PATCH] RFC: qdev: add legacy properties only for those print()-able
Posted by Philippe Mathieu-Daudé 3 weeks, 3 days ago 
+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
Posted by Marc-André Lureau 3 weeks, 3 days ago 
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
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.