[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/9] qom/globals: generalize object_property_set
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH 5/9] qom/globals: generalize object_property_set_globals() |
Date: |
Tue, 30 Oct 2018 16:16:43 +0400 |
Hi
On Mon, Oct 29, 2018 at 5:11 PM Igor Mammedov <address@hidden> wrote:
>
> On Wed, 12 Sep 2018 16:55:27 +0400
> Marc-André Lureau <address@hidden> wrote:
>
> > Handle calls of object_property_set_globals() with any object type,
> > but only apply globals to TYPE_DEVICE & TYPE_USER_CREATABLE.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > qom/globals.c | 22 ++++++++++++++--------
> > 1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/qom/globals.c b/qom/globals.c
> > index 587f4a1b5c..8664baebe0 100644
> > --- a/qom/globals.c
> > +++ b/qom/globals.c
> > @@ -15,22 +15,28 @@ void object_property_register_global(GlobalProperty
> > *prop)
> >
> > void object_property_set_globals(Object *obj)
> > {
> > - DeviceState *dev = DEVICE(obj);
> > GList *l;
> > + DeviceState *dev = (DeviceState *)object_dynamic_cast(obj,
> > TYPE_DEVICE);
> > +
> > + if (!dev && !IS_USER_CREATABLE(obj)) {
> > + /* only TYPE_DEVICE and TYPE_USER_CREATABLE support globals */
> > + return;
> > + }
> more dynamic casts but now imposed on every create object :(
>
> Maybe we should add ObjectClass::check/set_globals hook?
> It would be cheap to check and only objects we intend to work with
> it would be affected. On top of that hooks could be different so
> that device/user_creatable specifics won't be in generic code
> (like it's implemented here).
I don't think adding a few casts during object creation will impact
negatively in a measurable way.
However I'll add an additional patch to the series to implement what I
think you suggested.
thanks
>
> >
> > for (l = global_props; l; l = l->next) {
> > GlobalProperty *prop = l->data;
> > Error *err = NULL;
> >
> > - if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> > + if (object_dynamic_cast(obj, prop->driver) == NULL) {
> > continue;
> > }
> > prop->used = true;
> > - object_property_parse(OBJECT(dev), prop->value, prop->property,
> > &err);
> > + object_property_parse(obj, prop->value, prop->property, &err);
> > if (err != NULL) {
> > error_prepend(&err, "can't apply global %s.%s=%s: ",
> > prop->driver, prop->property, prop->value);
> > - if (!dev->hotplugged && prop->errp) {
> > +
> > + if (dev && !dev->hotplugged && prop->errp) {
> > error_propagate(prop->errp, err);
> > } else {
> > assert(prop->user_provided);
> > @@ -56,15 +62,15 @@ int object_property_check_globals(void)
> > continue;
> > }
> > oc = object_class_by_name(prop->driver);
> > - oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> > - if (!oc) {
> > + dc = (DeviceClass *)object_class_dynamic_cast(oc, TYPE_DEVICE);
> > + if (!IS_USER_CREATABLE_CLASS(oc) && !dc) {
> > warn_report("global %s.%s has invalid class name",
> > prop->driver, prop->property);
> > ret = 1;
> > continue;
> > }
> > - dc = DEVICE_CLASS(oc);
> > - if (!dc->hotpluggable && !prop->used) {
> > +
> > + if (dc && !dc->hotpluggable) {
> > warn_report("global %s.%s=%s not used",
> > prop->driver, prop->property, prop->value);
> > ret = 1;
>