Re: [PATCH 03/21] qdev-properties: PropertyInfo: add realized_set_allowed field

[Vladimir Sementsov-Ogievskiy]

17.05.2021 15:40, Max Reitz wrote:
> On 17.05.21 08:44, Vladimir Sementsov-Ogievskiy wrote:
> > Add field, so property can declare support for setting the property
> > when device is realized. To be used in the following commit.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   include/hw/qdev-properties.h | 1 +
> >   hw/core/qdev-properties.c    | 6 +++---
> >   2 files changed, 4 insertions(+), 3 deletions(-)
>
> Looks OK to me, although qdev isn’t my specialty.

Neither my :) Thanks for looking anyway!

> > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> > index 0ef97d60ce..007e1f69f4 100644
> > --- a/include/hw/qdev-properties.h
> > +++ b/include/hw/qdev-properties.h
> > @@ -32,6 +32,7 @@ struct PropertyInfo {
> >       const char *name;
> >       const char *description;
> >       const QEnumLookup *enum_table;
> > +    bool realized_set_allowed;
>
> I think a comment would be nice, though.

Agree, will add.

> >       int (*print)(Object *obj, Property *prop, char *dest, size_t len);
> >       void (*set_default_value)(ObjectProperty *op, const Property *prop);
> >       ObjectProperty *(*create)(ObjectClass *oc, const char *name,
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 50f40949f5..c34aac6ebc 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -26,11 +26,11 @@ void qdev_prop_set_after_realize(DeviceState *dev, const 
> > char *name,
> >   /* returns: true if property is allowed to be set, false otherwise */
> >   static bool qdev_prop_allow_set(Object *obj, const char *name,
> > -                                Error **errp)
> > +                                const PropertyInfo *info, Error **errp)
> >   {
> >       DeviceState *dev = DEVICE(obj);
> > -    if (dev->realized) {
> > +    if (dev->realized && !info->realized_set_allowed) {
> >           qdev_prop_set_after_realize(dev, name, errp);
> >           return false;
> >       }
> > @@ -79,7 +79,7 @@ static void field_prop_set(Object *obj, Visitor *v, const 
> > char *name,
> >   {
> >       Property *prop = opaque;
> > -    if (!qdev_prop_allow_set(obj, name, errp)) {
> > +    if (!qdev_prop_allow_set(obj, name, prop->info, errp)) {
> >           return;
> >       }


--
Best regards,
Vladimir