Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

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

On 05.02.2016 12:53, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
>> On 04.02.2016 09:20, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>>>
>>>> On 03.02.2016 18:42, Markus Armbruster wrote:
>>>>> Eric Blake <eblake@redhat.com> writes:
>>>>>
>>>>>> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>>>>> 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.
>>>>>>>
>>>>>>> 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>
>>>>>>> ---
>>>>>>> @@ -308,7 +309,9 @@ static 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 (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {
>>>>>> If you fix 2/3 to use a QAPI enum, then this will be an integer compare
>>>>>> instead of a strcmp().
>>>>> Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
>>>>> for the subtypes that should be counted here, and accumulate the sizes
>>>>> of devices where the flag is set.  Requires iterating directly over the
>>>>> devices here (like qmp_pc_dimm_device_list() does under the hood) rather
>>>>> than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),
>>>> It was my first approach but it was rejected)
> Sounds like your first approach was the right approach.
>
>>>> As another option I can make a function iterating over the devices and
>>>> return list of them, and then use it instead of
>>>> qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can
>>>> use object_dynamic_cast.
>>> I fail to see how splitting a tree walk doing stuff into a tree walk
>>> creating a list and a list walk doing stuff makes things better :)
>> It will allow me not touch qapi)
> Whatever it takes to get this fix past the gaggle of maintainers (I can
> relate to that).
>
> [...]

Everything is in your hands). I'm not mind if someone take any of this 
series versions, tune it as he wants and merge. It is a bug fix, but the 
discussion looks like I'm trying to add a new feature)


-- 
Best regards,
Vladimir