Re: [PATCH v2 1/4] conf: Introduce dynamicMemslots attribute for virtio-mem
On 08.01.24 09:38, Peter Krempa wrote:
On Fri, Jan 05, 2024 at 18:30:56 +0100, David Hildenbrand wrote:
> On 05.01.24 14:58, Peter Krempa wrote:
>> On Fri, Jan 05, 2024 at 14:34:53 +0100, David Hildenbrand wrote:
>>> On 05.01.24 13:40, Peter Krempa wrote:
>>>> On Fri, Jan 05, 2024 at 13:33:32 +0100, Michal Privoznik wrote:
[...]
>> This series is very specifically adding it only for virtio-mem, which
>> AFAIK doesn't have any vhost-user variant, thus having this as a
>> configurable knob is almost pointless.
>
> No, that problem is the combination. Assume you have VM with both virtio-mem
> to hotplug memory and virtio-user-fs+virtiofsd.
>
> The memory that virtio-mem provides has to be usable by
> virtio-user-fs+virtiofsd. Therefore, we have to map that shared memory
> (provided by virtio-mem) into the virtiofsd process.
>
> However, virtiofsd only supports 32 memslots and virito-mem wants to use up
> to 256 (like having 256 DIMMs). virtiofsd support is in the works but not
> upstream yet.
>
> So if you start a vm with
>
> -device virtio-mem-pci,dynamic-memslots=on,...
> -device virtio-user-fs-pci, ...
>
> QEMU might reject to start if the virtio-user-fs-pci backend has
> insufficient memory slots.
>
> In that case (and only then), we want to disable dynamic memslots. Without
> the virtio-user complication (that I hate), this wouldn't even be a
> configurable toggle.
Ah I see. So we can't simply enable it by default because AND not even
automatically:
1) It would break existing configs (but solvable before startup)
Right. One could keep the toggle disabled for old QEMU machines, or
disable it if any vhost-user device is configured as well. yes, that is
more complicated and better avoided.
2) it would break hotplug into VMs (when hotplugging e.g. virtiofs)
Right, although not a common setup, especially not with virtio-mem.
This means that if we'd want to implement it now it MUST be a
configurable toggle.
Note that libvirt will have to carry this code forever even once qemu
enables this by default and thus the toggle in libvirt becomes
effectively pointless. Thus if this will be a short-term workaround I'd
really welcome if we'd re-consider doing this.
I assume that in the long-term everything based on the vhost-user rust
crate will become compatible. And at this point, we primarily only care
about virtiofsd with virito-mem.
If the benefits from enabling the feature, and the timeline of
auto-enabling it in qemu are such that we want to implement the toggle
in libvirt for the time being, this patch needs to document (in
formatdomain.rst, not the commit message the following):
1) the fact that it's beneficial to enable it
2) which setup it could block (mentioning also hotplug)
3) that qemu will auto-enable the feature once it's fully available
Having this toggle without any of the above will result in basically
nobody enabling it.
I'm hoping that we can enable it always in RHEL where we can control
which version of vortiofsd we ship, and can better control what we
actually support.
--
Cheers,
David / dhildenb