[PATCH v2 05/13] conf: Introduce virtio-mem <memory/> model

[David Hildenbrand]

On 22.02.21 16:46, Michal Privoznik wrote:
> On 2/18/21 4:00 PM, David Hildenbrand wrote:
>> On 18.02.21 14:31, Michal Privoznik wrote:
> 
>>>
>>> Since now we have (possibly) two or more devices that allow
>>> memory inflation/deflation and accounting for all of them (and
>>> thus keeping <currentMemory/> updated) might be hard. Therefore,
>>> I'm deliberately forbidding memballoon. It's okay - virtio-mem is
>>> superior to memballoon anyway. We can always reevaluate later.
>>
>> That's a bad idea. It'll still be used for getting memory stats, free
>> page hinting and free page reporting.
>>
>> Very weird use cases might even want to mix balloon inflation/deflation
>> with virtio-mem ...
>>
> 
> I understand that, but then accounting for memory at libvirt level
> becomes hard. What I mean, currently we have three elements for
> reporting guest memory:
> 
>     <domain>
>       <maxMemory slots='16' unit='KiB'>1524288</maxMemory>
>       <memory unit='KiB'>524288</memory>
>       <currentMemory unit='KiB'>524288</currentMemory>
>       ...
>     </domain>
> 
> <maxMemory/> is for memory hotplug (corresponds to -m maxmem=X,slots=Y)
> 
> <memory/> is the initial amount guest has PLUS sum of sizes of all
> memory devices. For instance if I want to give 1GiB initial amount and
> have 1 pc-dimm of 2GiB size this element will report 3GiB.
> 
> <currentMemory/> then reflects actual balloon size (gets updated on
> BALLOON_CHANGE event and when libvirt finds fit via query-balloon).
> 
> In my experiments, balloon does not account for virtio-mem actual size.
> Therefore, I can see following options:
> 
> 1) fix <currentMemory/> so that it does balloon size +
> sum(virtio-mem.actual), or
> 
> 2) don't touch <currentMemory/> at all and let mgmt apps compute the sum
> themselves (should they need it- which I believe they will),
> 
> 3) come up with new, forth element (awful).
> 
> The benefit of 1) is that the element will report the same number as
> 'free' ran within guest. Problem is that I'd have to change all places
> where we take the size of the balloon device directly and add sum of
> actual sizes of virtio-mem devices (e.g. all stats APIs, ballon changed
> event that we fire when we see qemu's event). And okay, if I do that -
> then the only way to learn true value of balloon would be to take
> current memory and subtract the sum of virtio-mem actual sizes. Which is
> like option 2) but reversed. And since I could not decide which way to
> go, I made it problem of future me by forbidding balloon and virtio-mem
> coexistence.

In QEMU, we could make "info balloon" etc. also include virtio-mem 
provided memory.

The main reason I did not do so initially is that
1) It's racy when reading/writing the balloon size. The QEMU interface
    is broken as we don't get/set the size of the balloon size but
    instead the logical VM size. While someone sets the logical VM size
    via virtio-balloon, the logical VM size might change due to virtio-
    mem guest activity.
2) I don't want people to be tempted to use both at the same
    time.

Maybe the right think to do is make "info balloon" report the current 
logical VM size. If there are races, bad luck - better not use both 
things at the same time.

-- 
Thanks,

David / dhildenb