Re: [PATCH v2 1/4] conf: Introduce dynamicMemslots attribute for virtio-mem
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)
2) it would break hotplug into VMs (when hotplugging e.g. virtiofs)
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.
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.