On Tue, 30 Oct 2018 16:16:43 +0400
Marc-André Lureau <marcandre.lur...@redhat.com> wrote:
> Hi
>
> On Mon, Oct 29, 2018 at 5:11 PM Igor Mammedov <imamm...@redhat.com> wrote:
> >
> > On Wed, 12 Sep 2018 16:55:27 +0400
> > Marc-André Lureau <marcandre.lur...@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.lur...@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;
> >
