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 <marcandre.lureau@redhat.com>
---
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;
+ }
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;
--
2.19.0.rc1
On Wed, 12 Sep 2018 16:55:27 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> 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 <marcandre.lureau@redhat.com>
> ---
> 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).
>
> 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;
Hi
On Mon, Oct 29, 2018 at 5:11 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 12 Sep 2018 16:55:27 +0400
> Marc-André Lureau <marcandre.lureau@redhat.com> 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 <marcandre.lureau@redhat.com>
> > ---
> > 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;
>
On Tue, 30 Oct 2018 16:16:43 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> Hi
>
> On Mon, Oct 29, 2018 at 5:11 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Wed, 12 Sep 2018 16:55:27 +0400
> > Marc-André Lureau <marcandre.lureau@redhat.com> 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 <marcandre.lureau@redhat.com>
> > > ---
> > > 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.
When it's applied to every object created as opposed to -object CLI option,
it will add up. For example 512 x 2 (cpus + apic) ~ 1000 x cast_cost,
for pc-dimm 255 x 3 (backend + memory_region + pc-dimm) ~ 800 x cast_cost,
We can have more backends of other types, basically everything QOM will
incur tiny extra cost.
I'm just thinking about worst case scenario.
> However I'll add an additional patch to the series to implement what I
> think you suggested.
thanks,
for this patch also important to keep Device vs UserCreatable
parts separate instead of dumping evrything into one pit.
>
> 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;
> >
© 2016 - 2025 Red Hat, Inc.