Re: [PATCH RFC 0/9] qemu: Support mapped-ram migration capability

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.

> 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.