On Wed, Feb 07, 2018 at 03:33:27PM +0800, Haozhong Zhang wrote:
> @@ -156,11 +157,17 @@ static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, const void *buf,
>  {
>      MemoryRegion *mr;
>      PCDIMMDevice *dimm = PC_DIMM(nvdimm);
> +    bool is_pmem = object_property_get_bool(OBJECT(dimm->hostmem),
> +                                            "pmem", NULL);
>      uint64_t backend_offset;
>  
>      nvdimm_validate_rw_label_data(nvdimm, size, offset);
>  
> -    memcpy(nvdimm->label_data + offset, buf, size);
> +    if (!is_pmem) {
> +        memcpy(nvdimm->label_data + offset, buf, size);
> +    } else {
> +        pmem_memcpy_persist(nvdimm->label_data + offset, buf, size);
> +    }

Is this enough to prevent label corruption in case of power failure?

pmem_memcpy_persist() is not atomic.  Power failure can result in a mix
of the old and new label data.

If we want this operation to be 100% safe there needs to be some kind of
update protocol that makes the change atomic, like a Label A and Label B
area with a single Label Index field that can be updated atomically to
point to the active Label A/B area.

Stefan