Re: [Qemu-arm] [PATCH v3 2/2] machine: Move nvdimms state into struct MachineState

[Eduardo Habkost]

On Fri, Mar 08, 2019 at 02:42:14PM -0300, Eduardo Habkost wrote:
> On Fri, Mar 08, 2019 at 04:25:11PM +0100, Eric Auger wrote:
> [...]
> > @@ -791,6 +833,28 @@ static void machine_initfn(Object *obj)
> >      ms->mem_merge = true;
> >      ms->enable_graphics = true;
> >  
> > +    if (mc->nvdimm_supported) {
> > +        ObjectClass *oc = OBJECT_CLASS(mc);
> > +
> > +        ms->nvdimms_state = g_new0(NVDIMMState, 1);
> 
> Initializing ms->nvdimms_state inside machine_initfn()
> only if mc->nvdimm_support makes sense.
> 
> But:
> 
> > +        object_class_property_add_bool(oc, "nvdimm",
> > +                                       machine_get_nvdimm, 
> > machine_set_nvdimm,
> > +                                       &error_abort);
> > +        object_class_property_set_description(oc, "nvdimm",
> > +                                             "Set on/off to enable/disable 
> > "
> > +                                             "NVDIMM instantiation", NULL);
> > +
> > +        object_class_property_add_str(oc, "nvdimm-persistence",
> > +                                      machine_get_nvdimm_persistence,
> > +                                      machine_set_nvdimm_persistence,
> > +                                      &error_abort);
> > +        object_class_property_set_description(oc, "nvdimm-persistence",
> > +                                             "Set NVDIMM persistence"
> > +                                             "Valid values are cpu, 
> > mem-ctrl",
> > +                                              NULL);
> 
> Calling object_class_property_add_*() inside instance_init
> defeats the purpose of class properties.  I suggest either using
> object_property_add_*(), or registering these class properties in
> machine_class_base_init().

I just realized that machine_class_base_init() won't work, as
it is called before the subclasses' class_init functions.

The only solutions I see here are using object_property_add*() or
creating a nvdimm_machine_class_init(MachineClass*) helper that
will set mc->nvdimm_supported=true and register the class properties.

-- 
Eduardo