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

[Michal Prívozník]

On 9/17/21 2:54 PM, David Hildenbrand wrote:
> 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 :)

We could do that, yes. Let me respin the series with that change - these
patches don't apply cleanly anymore anyway. Should a similar warning be
produced when hugepages are configured?

Michal