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

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.

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