Re: [Qemu-devel] [PATCH v1] numa: report all DIMM/NVDIMMs as plugged memory

[David Hildenbrand <david@redhat.com>]

On 27.06.2018 15:54, Igor Mammedov wrote:
> On Wed, 27 Jun 2018 15:38:23 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 27.06.2018 13:21, Igor Mammedov wrote:
>>> On Fri, 22 Jun 2018 16:40:45 +0200
>>> David Hildenbrand <david@redhat.com> wrote:
>>>   
>>>> Right now, there is some inconsistency between hotplugged and
>>>> coldplugged memory. DIMMs added via "-device" result in different stats
>>>> than DIMMs added using "device_add".
>>>>
>>>> E.g.
>>>>     [...]
>>>>     -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
>>>>     -m 4G,maxmem=20G,slots=2 \
>>>>     -object memory-backend-ram,id=mem0,size=8G \
>>>>     -device pc-dimm,id=dimm0,memdev=mem0 \
>>>>     -object memory-backend-ram,id=mem1,size=8G \
>>>>     -device nvdimm,id=dimm1,memdev=mem1,node=1
>>>>
>>>> Results in NUMA info
>>>>     (qemu) info numa
>>>>     info numa
>>>>     2 nodes
>>>>     node 0 cpus: 0 1
>>>>     node 0 size: 10240 MB
>>>>     node 0 plugged: 0 MB
>>>>     node 1 cpus: 2 3
>>>>     node 1 size: 10240 MB
>>>>     node 1 plugged: 0 MB  
>>> ...   
>>>> But in memory size summary:
>>>>     (qemu) info memory_size_summary
>>>>     info memory_size_summary
>>>>     base memory: 4294967296
>>>>     plugged memory: 17179869184  
>>> this looks like right amount wrt used CLI,
>>> is this supposed to be correct output or broken one?
>>> ('but' implies incorrect one)  
>>
>> 'but' just highlights the inconsistency here. As explained above, all
>> dimms should be reported as plugged (hot and coldplugged).
>>
>> So in my opinion "memory size summary" does the right thing right now.
> maybe add to commit message that says what exactly is being fixed,
> otherwise it's not clear what is expected/wrong beside of output being inconsistent.

I will add some more details.

> 
>>
>>>   
>>>>
>>>> Make this consistent by reporting all hot and coldplugged
>>>> memory a.k.a. DIMM and NVDIMM as "plugged".
> maybe mention 'info numa' and maybe QMP equivalent if it exists,
> otherwise above text is a bit vague for bystander (could be /me|you in a day-month-year)

Yes, will do, thanks!

-- 

Thanks,

David / dhildenb