Re: [Qemu-ppc] [PATCH for-3.2 v4 16/28] hw: apply machine compat properties without touching globals

[Eduardo Habkost]

On Tue, Nov 27, 2018 at 05:10:05PM +0400, Marc-André Lureau wrote:
> On Tue, Nov 27, 2018 at 4:57 PM Eduardo Habkost <address@hidden> wrote:
> >
> > On Tue, Nov 27, 2018 at 01:27:49PM +0400, Marc-André Lureau wrote:
> > > Similarly to accel properties, move compat properties out of globals
> > > registration, and apply the machine compat properties during
> > > device_post_init().
> > >
> > > Signed-off-by: Marc-André Lureau <address@hidden>
> > [...]
> > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > > index 7066d28271..3b31b2c025 100644
> > > --- a/hw/core/qdev.c
> > > +++ b/hw/core/qdev.c
> > > @@ -971,17 +971,26 @@ static void device_initfn(Object *obj)
> > >  }
> > >
> > >  static const GPtrArray *ac_compat_props;
> > > +static const GPtrArray *mc_compat_props;
> > >
> > >  void accel_register_compat_props(const GPtrArray *props)
> > >  {
> > >      ac_compat_props = props;
> > >  }
> > >
> > > +void machine_register_compat_props(const GPtrArray *props)
> > > +{
> > > +    mc_compat_props = props;
> > > +}
> > > +
> > >  static void device_post_init(Object *obj)
> > >  {
> > >      if (ac_compat_props) {
> > >          object_apply_global_props(obj, ac_compat_props, &error_abort);
> > >      }
> >
> > Why not just use MACHINE(qdev_get_machine())->accel->compat_props
> > directly?
> >
> > > +    if (mc_compat_props) {
> > > +        object_apply_global_props(obj, mc_compat_props, &error_abort);
> > > +    }
> >
> > Why not just use MACHINE(qdev_get_machine())->compat_props
> > directly?
> 
> This was the approach in v3, but Igor didn't quite like referencing
> machine in qdev:
> https://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg04774.html

I disagree with Igor, here.  Core qdev code already have multiple
references to machine, I don't see any problem with that.

The previous code was clearer and easier to follow, and wasn't
sensitive to subtle changes in initialization ordering (e.g. what
happens if we create a device before *_register_compat_props() is
called?).


> 
> >
> > >
> > >      qdev_prop_set_globals(DEVICE(obj));
> > >  }
> > [...]
> >
> > --
> > Eduardo

-- 
Eduardo