Re: [Qemu-devel] [RESEND PATCH v3 5/8] pc-dimm: Add pc_dimm_unrealize() for memory hot unplug support.

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2014/9/4 21:28, Igor Mammedov wrote:
> On Wed, 27 Aug 2014 16:08:36 +0800
> Tang Chen<tangchen@cn.fujitsu.com>  wrote:
>
>> From: Hu Tao<hutao@cn.fujitsu.com>
>>
>> Implement unrealize function for pc-dimm device. It delete subregion from
> s/delete/removes/
>
>> hotplug region, and delete ram address range from guest ram list.
>>
>> Signed-off-by: Hu Tao<hutao@cn.fujitsu.com>
>> Signed-off-by: Tang Chen<tangchen@cn.fujitsu.com>
>> ---
>>   hw/mem/pc-dimm.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 20fe0dc..34109a2 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -270,12 +270,22 @@ static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
>>       return host_memory_backend_get_memory(dimm->hostmem,&error_abort);
>>   }
>>
>> +static void pc_dimm_unrealize(DeviceState *dev, Error **errp)
>> +{
>> +    PCDIMMDevice *dimm = PC_DIMM(dev);
>> +    MemoryRegion *mr = pc_dimm_get_memory_region(dimm);
>> +
>> +    memory_region_del_subregion(mr->container, mr);
> usually MemoryRegion is treated as opaque and it's fields
> accessed via memory_region_foo() helpers.
>
>> +    vmstate_unregister_ram(mr, dev);
> these 2 lines look like a job for PCMachine which did original mapping/vmstate registration
>

Actually, this patch also fix the bug *when hotplug memory failing in
the place where after pc_dimm_plug but before the end of device_set_realized,
it does not clear the work done by pc_dimm_plug*.

For there is no callback like pc_dimm_plug_fail_cb for us to call when fail,
Maybe pc_dimm_unrealize is the only place where we can do the clear work...

Thanks,
zhanghailiang

>> +}
>> +
>>   static void pc_dimm_class_init(ObjectClass *oc, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(oc);
>>       PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
>>
>>       dc->realize = pc_dimm_realize;
>> +    dc->unrealize = pc_dimm_unrealize;
>>       dc->props = pc_dimm_properties;
>>
>>       ddc->get_memory_region = pc_dimm_get_memory_region;
>
>
>
>