Re: [Qemu-devel] [PATCH v3 3/3] nvdimm: add 'unarmed' option

[Haozhong Zhang]

On 12/01/17 10:44 +0000, Stefan Hajnoczi wrote:
> On Mon, Nov 27, 2017 at 12:35:17PM +0800, Haozhong Zhang wrote:
> > @@ -312,6 +315,10 @@ nvdimm_build_structure_memdev(GArray *structures, 
> > DeviceState *dev)
> >  
> >      /* Only one interleave for PMEM. */
> >      nfit_memdev->interleave_ways = cpu_to_le16(1);
> > +
> > +    if (nvdimm->unarmed) {
> > +        nfit_memdev->flags |= ACPI_NFIT_MEM_NOT_ARMED;
> 
> Missing cpu_to_le16()?
> 
> > +static void nvdimm_set_unarmed(Object *obj, bool value, Error **errp)
> > +{
> > +    NVDIMMDevice *nvdimm = NVDIMM(obj);
> > +    Error *local_err = NULL;
> > +
> > +    if (memory_region_size(&nvdimm->nvdimm_mr)) {
> > +        error_setg(&local_err, "cannot change property value");
> 
> This error message does not explain why the value cannot be changed.  I
> guess this is checking if the device has been realized.
> 
> Please call qdev_prop_set_after_realize() instead:
> 
>   if (memory_region_size(&nvdimm->nvdimm_mr)) {
>       qdev_prop_set_after_realize(DEVICE(obj), "unarmed", &local_err);
>       goto out;
>   }

I'll change in the next version.

Thanks,
Haozhong