On 8/8/24 17:46, Jim Fehlig wrote:
On 8/7/24 12:39, Daniel P. Berrangé wrote:
On Wed, Aug 07, 2024 at 12:04:18PM -0600, Jim Fehlig wrote:
On 8/7/24 09:45, Daniel P. Berrangé wrote:
On Thu, Jun 13, 2024 at 04:43:14PM -0600, Jim Fehlig via Devel wrote:
The QEMU mapped-ram capability currently does not support directio. Fabino is working on that now [3]. This complicates merging support in libvirt. I don't think it's reasonable to enable mapped-ram by default when BYPASS_CACHE cannot be supported. Should we wait until the mapped-ram directio support is merged in QEMU before supporting mapped-ram in libvirt?
For the moment, compression is ignored in the new save version. Currently, libvirt connects the output of QEMU's save stream to the specified compression program via a pipe. This approach is incompatible with mapped-ram since the fd provided to QEMU must be seekable. One option is to reopen and compress the saved image after the actual save operation has completed. This has the downside of requiring the iohelper to handle BYPASS_CACHE, which would preclude us from removing it sometime in the future. Other suggestions much welcomed.
Going back to the original motivation for mapped-ram. The first key factor was that it will make it more viable to use multi-fd for parallelized saving and restoring, as it lets threads write concurrently without needing synchronization. The predictable worst case file size when the VM is live & dirtying memory, was an added benefit.
Correct. The main motivation for mapped-ram is parallelizing save/restore. To that end, we could base the whole feature on whether parallel is requested. If so, we save with mapped-ram (failing if not supported by underlying qemu) and don't support compression. If parallel not requested, use existing save/restore implementation.
We had discussed that as a strategy previously and thought it was a good idea.
Your other reply to me though makes me re-consider. You're showing that /without/ O_DIRECT and multifd, then mapped-ram is already a big win over the legacy method. The scale of that improvement is somewhat surprising to me and if reliably so, that is compelling to want all the time.
How do we have it all of the time without issues of spareness you describe? Seems it will always be opt-in, unless using the new parallel option.
Overall I'm wondering if we need to give a direct choice to mgmt apps.
We added a the save/restore variants that accept virTypedParameters, so we could define a VIR_DOMAIN_SAVE_PARAM_FORMAT, which accepts 'stream' and 'mapped' as options. This choice would then influence whether we save in v2 or v3 format. On restore we don't need a parameter as we just probe the on disk format.
I worry these options will confuse users. IMO we'd need a basic description of the stream formats, along with caveats of file size, lack of compression support, etc. It seems like quite a bit for the average user to absorb.
As a documentation task we can then save that compression is incompatible with 'mapped'.
Annoyingly we already have a 'save_image_formt' in qemu.conf though taking 'raw', 'zstd', 'lzop', etc to choose the compression type. So we have a terminology clash.
We probably should have exposed this compression choice in the API too, via a VIR_DOMAIN_SAVE_PARAM_COMPRESSION typed parameter, taking values 'default', 'raw', 'zstd', 'lzop', etc, where 'default' (or omitted) means honour the qemu.conf setting and all others override qemu.conf
Agree with that suggestion. And VIR_DOMAIN_SAVE_PARAM_COMPRESSION would be easy to describe and clear to average user :-).
Might suggest we name the new API parameter VR_DOMAIN_SAVE_PARAM_LAYOUT for stream vs mapped choice ?
These stream formats seem like an implementation detail we should attempt to hide from the user.
Yeah it is rather an impl detail, but perhaps it is an impl detail we cannot avoid exposing users to. After all they need to be aware of use of sparseness in order to handle the files without ballooning up their size.
As for a name for the new parameter, I can't think of anything better than your suggestion of VIR_DOMAIN_SAVE_PARAM_LAYOUT. I much prefer _FORMAT, but yeah, that ship has sailed.
Before working on the API parameter, I'll summarize my understanding of the options for supporting mapped-ram:
1. No user control. If qemu supports mapped-ram, use it 2. Control at driver level
Forgot to mention: I sent the implementation for this option since it was ready anyhow https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/MNBHG... Regards, Jim