Re: [PATCH V1 19/26] physmem: preserve ram blocks for cpr

[Peter Xu <peterx@redhat.com>]

On Wed, May 29, 2024 at 01:31:53PM -0400, Steven Sistare wrote:
> On 5/28/2024 5:44 PM, Peter Xu wrote:
> > On Mon, Apr 29, 2024 at 08:55:28AM -0700, Steve Sistare wrote:
> > > Preserve fields of RAMBlocks that allocate their host memory during CPR so
> > > the RAM allocation can be recovered.
> > 
> > This sentence itself did not explain much, IMHO.  QEMU can share memory
> > using fd based memory already of all kinds, as long as the memory backend
> > is path-based it can be shared by sharing the same paths to dst.
> > 
> > This reads very confusing as a generic concept.  I mean, QEMU migration
> > relies on so many things to work right.  We mostly asks the users to "use
> > exactly the same cmdline for src/dst QEMU unless you know what you're
> > doing", otherwise many things can break.  That should also include ramblock
> > being matched between src/dst due to the same cmdlines provided on both
> > sides.  It'll be confusing to mention this when we thought the ramblocks
> > also rely on that fact.
> > 
> > So IIUC this sentence should be dropped in the real patch, and I'll try to
> > guess the real reason with below..
> 
> The properties of the implicitly created ramblocks must be preserved.
> The defaults can and do change between qemu releases, even when the command-line
> parameters do not change for the explicit objects that cause these implicit
> ramblocks to be created.

AFAIU, QEMU relies on ramblocks to be the same before this series.  Do you
have an example?  Would that already cause issue when migrate?

> 
> > > Mirror the mr->align field in the RAMBlock to simplify the vmstate.
> > > Preserve the old host address, even though it is immediately discarded,
> > > as it will be needed in the future for CPR with iommufd.  Preserve
> > > guest_memfd, even though CPR does not yet support it, to maintain vmstate
> > > compatibility when it becomes supported.
> > 
> > .. It could be about the vfio vaddr update feature that you mentioned and
> > only for iommufd (as IIUC vfio still relies on iova ranges, then it won't
> > help here)?
> > 
> > If so, IMHO we should have this patch (or any variance form) to be there
> > for your upcoming vfio support.  Keeping this around like this will make
> > the series harder to review.  Or is it needed even before VFIO?
> 
> This patch is needed independently of vfio or iommufd.
> 
> guest_memfd is independent of vfio or iommufd.  It is a recent addition
> which I have not tried to support, but I added this placeholder field
> to it can be supported in the future without adding a new field later
> and maintaining backwards compatibility.

Is guest_memfd the only user so far, then?  If so, would it be possible we
split it as a separate effort on top of the base cpr-exec support?

-- 
Peter Xu