[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 23/32] qdev: Move dev->realized check to qdev_property_set
From: |
Igor Mammedov |
Subject: |
Re: [PATCH v4 23/32] qdev: Move dev->realized check to qdev_property_set() |
Date: |
Tue, 15 Dec 2020 15:24:18 +0100 |
On Mon, 14 Dec 2020 12:24:18 -0500
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Dec 14, 2020 at 03:55:30PM +0100, Igor Mammedov wrote:
> > On Fri, 11 Dec 2020 17:05:20 -0500
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > Every single qdev property setter function manually checks
> > > dev->realized. We can just check dev->realized inside
> > > qdev_property_set() instead.
> > >
> > > The check is being added as a separate function
> > > (qdev_prop_allow_set()) because it will become a callback later.
> >
> > is callback added within this series?
> > and I'd add here what's the purpose of it.
>
> It will be added in part 2 of the series. See v3:
> https://lore.kernel.org/qemu-devel/20201112214350.872250-35-ehabkost@redhat.com/
>
> I don't know what else I could say about its purpose, in addition
> to what I wrote above, and the comment below[1].
>
> If you are just curious about the callback and confused because
> it is not anywhere in this series, I can just remove the
> paragraph above from the commit message. Would that be enough?
lets remove it for now to avoid confusion
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> >
> [...]
> > > +/* returns: true if property is allowed to be set, false otherwise */
>
> [1] ^^^
>
> > > +static bool qdev_prop_allow_set(Object *obj, const char *name,
> > > + Error **errp)
> > > +{
> > > + DeviceState *dev = DEVICE(obj);
> > > +
> > > + if (dev->realized) {
> > > + qdev_prop_set_after_realize(dev, name, errp);
> > > + return false;
> > > + }
> > > + return true;
> > > +}
> > > +
>