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

[Eduardo Habkost]

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().


> [...]

-- 
Eduardo