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

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.