Re: [Qemu-devel] [PATCH v2 03/15] pc-dimm: keep the state of the whole backend memory

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 05/31/2016 02:42 AM, Stefan Hajnoczi wrote:
> On Fri, May 20, 2016 at 04:20:00PM +0800, Xiao Guangrong wrote:
>> QEMU keeps the state of memory of dimm device during live migration,
>> however, it is not enough for nvdimm device as its memory does not
>> contain its label data, so that we should protect the whole backend
>> memory instead
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>>   hw/mem/pc-dimm.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 6de2275..72b33ba 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -105,9 +105,16 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>>       }
>>
>>       memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
>> -    vmstate_register_ram(mr, dev);
>>       numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
>>
>> +    /*
>> +     * save the state only for @mr is not enough as it does not contain
>> +     * the label data of NVDIMM device, so that we keep the state of
>> +     * whole hostmem instead.
>> +     */
>> +    vmstate_register_ram(host_memory_backend_get_memory(dimm->hostmem, errp),
>> +                         dev);
>> +
>>   out:
>>       error_propagate(errp, local_err);
>>   }
>
> In Patch 1 you introduced a callback to get the guest-visible memory
> region.  Instead of mentioning NVDIMM in generic pc-dimm.c code, it
> would be cleaner to add another callback to get the vmstate memory
> region:
>
>    .get_guest_memory_region() - Patch 1
>    .get_vmstate_memory_region() - a new patch in this series
>

It is good to me, will do it. Thanks!