Re: [Qemu-devel] [PATCH v2 1/1] nvdimm: disable balloon

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

On 26.01.2016 13:38, Igor Mammedov wrote:
> On Mon, 25 Jan 2016 19:50:31 +0300
> "Denis V. Lunev" <den@openvz.org> wrote:
>
>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> NVDIMM for now is planned to use as a backing store for DAX filesystem
>> in the guest and thus this memory is excluded from guest memory management
>> and LRUs.
>>
>> In this case libvirt running QEMU along with configured balloon almost
>> immediately inflates balloon and effectively kill the guest as
>> qemu counts nvdimm as part of the ram.
> Isn't issue in ballooning impl. and not of pc-dimm/nvdimm,
> so make ballooning code to distinguish between kinds of memory
> rather than adding not related fields to PCDIMMDeviceClass
>
> why don't just move get_current_ram_size() into the sole user
> virtio-balloon.c and ignore NVDIMMs when counting ram in
> get_current_ram_size(),
> that would be much less intrusive patch.

Ok, will do.

>
>> Counting dimm devices as part of the ram for ballooning was started from
>> commit 463756d03:
>>   virtio-balloon: Fix balloon not working correctly when hotplug memory
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> CC: "Michael S. Tsirkin" <mst@redhat.com>
>> CC: Igor Mammedov <imammedo@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> ---
>> v2:
>>      - some rewordings, thanks to Eric Blake
>>
>>   hw/mem/nvdimm.c          | 4 ++++
>>   hw/mem/pc-dimm.c         | 7 ++++++-
>>   include/hw/mem/pc-dimm.h | 1 +
>>   qapi-schema.json         | 5 ++++-
>>   4 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
>> index 4fd397f..4f4d29a 100644
>> --- a/hw/mem/nvdimm.c
>> +++ b/hw/mem/nvdimm.c
>> @@ -27,9 +27,13 @@
>>   static void nvdimm_class_init(ObjectClass *oc, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(oc);
>> +    PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
>>   
>>       /* nvdimm hotplug has not been supported yet. */
>>       dc->hotpluggable = false;
>> +
>> +    /* ballooning is not supported */
>> +    ddc->in_ram = false;
>>   }
>>   
>>   static TypeInfo nvdimm_info = {
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index d5cdab2..e0f869d 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -164,6 +164,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>>               MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
>>               PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
>>               DeviceClass *dc = DEVICE_GET_CLASS(obj);
>> +            PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
>>               PCDIMMDevice *dimm = PC_DIMM(obj);
>>   
>>               if (dev->id) {
>> @@ -172,6 +173,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>>               }
>>               di->hotplugged = dev->hotplugged;
>>               di->hotpluggable = dc->hotpluggable;
>> +            di->in_ram = ddc->in_ram;
>>               di->addr = dimm->addr;
>>               di->slot = dimm->slot;
>>               di->node = dimm->node;
>> @@ -205,7 +207,9 @@ ram_addr_t get_current_ram_size(void)
>>           if (value) {
>>               switch (value->type) {
>>               case MEMORY_DEVICE_INFO_KIND_DIMM:
>> -                size += value->u.dimm->size;
>> +                if (value->u.dimm->in_ram) {
>> +                    size += value->u.dimm->size;
>> +                }
>>                   break;
>>               default:
>>                   break;
>> @@ -444,6 +448,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
>>       dc->props = pc_dimm_properties;
>>       dc->desc = "DIMM memory module";
>>   
>> +    ddc->in_ram = true;
>>       ddc->get_memory_region = pc_dimm_get_memory_region;
>>   }
>>   
>> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
>> index d83bf30..3bcb505 100644
>> --- a/include/hw/mem/pc-dimm.h
>> +++ b/include/hw/mem/pc-dimm.h
>> @@ -65,6 +65,7 @@ typedef struct PCDIMMDevice {
>>   typedef struct PCDIMMDeviceClass {
>>       /* private */
>>       DeviceClass parent_class;
>> +    bool in_ram;
>>   
>>       /* public */
>>       MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 2e960db..3cafa2b 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3922,6 +3922,8 @@
>>   #
>>   # @hotpluggable: true if device if could be added/removed while machine is running
>>   #
>> +# @in-ram: true if device is counted in current ram size (since 2.6)
>> +#
>>   # Since: 2.1
>>   ##
>>   { 'struct': 'PCDIMMDeviceInfo',
>> @@ -3932,7 +3934,8 @@
>>               'node': 'int',
>>               'memdev': 'str',
>>               'hotplugged': 'bool',
>> -            'hotpluggable': 'bool'
>> +            'hotpluggable': 'bool',
>> +            'in-ram': 'bool'
>>             }
>>   }
>>   


-- 
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.