
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