Re: [PATCH v5 05/16] qemu: Build command line for virtio-mem

On 15.09.21 13:07, Michal Prívozník wrote: > On 9/14/21 3:05 PM, David Hildenbrand wrote: >> On 13.09.21 16:52, Michal Privoznik wrote: >>> Nothing special is happening here. All important changes were >>> done when for 'virtio-pmem' (adjusting the code to put virtio >>> memory on PCI bus, generating alias using >>> qemuDomainDeviceAliasIndex(). The only bit that might look >>> suspicious is no prealloc for virtio-mem. But if you think about >>> it, the whole purpose of this device is to change amount of >>> memory exposed to guest on the fly. There is no point in locking >>> the whole backend in memory. >> >> Do I understand correctly that we'll always try setting "reserve=off", >> and "prealloc=off"? That would be awesome :) > > prealloc=off would be set (almost) unconditionally for all > memory-backend-* objects that are associated with virtio-mem device. And > if QEMU is new enough to have .reserve attribute then reserve=off is set > too. > Ah, perfect. >> >> I do wonder if we want to warn or bail out if "priv->memPrealloc" is set >> but we are temporarily not able to support it -- as discussed, because >> virtio-mem in QEMU yet has to be taught about it. > > priv->memPrealloc might not be what you think it is. The fact whether > immediate allocation was requested is stored in def->mem.allocation; and > priv->memPrealloc is just there to track whether -mem-prealloc what > already put onto (paritally) generated cmd line and thus the rest of > generators must refrain from setting prealloc=on. > > Codewise speaking, if you'd look at qemuBuildCommandLine() (this is > where cmd line generation happens) then you'd see > qemuBuildMemCommandLine() being called first and only after that > qemuBuildMemoryDeviceCommandLine() being called second. > > The former is responsible for generating generic memory for guest (e.g. > -m XX -mem-prealloc -mem-path ...  which is the old style, nowadays a > combination of -machine memory-backend= + -object memory-backend-*  is > generated). > > Long story short, if priv->memPrealloc isn't true at this point, then > libvirt doesn't have to set prealloc=off explicitly, because off is the > default. That makes sense. Yet I wonder if we should warn that preallocation is currently not supported for virtio-mem and will be ignored. Your decision :)