Hi

On Tue, Oct 21, 2025 at 4:55 PM Markus Armbruster <[email protected]> wrote:
>
> [email protected] writes:
>
> > From: Marc-André Lureau <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> >  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

Reply via email to