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.
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.
Another thing to ask: does this idea also need to rely on some future
iommufd kernel support? If there's anything that's not merged in current
Linux upstream, this series needs to be marked as RFC, so it's not target
for merging. This will also be true if this patch is "preparing" for that
work. It means if this patch only services iommufd purpose, even if it
doesn't require any kernel header to be referenced, we should only merge it
together with the full iommufd support comes later (and that'll be after
iommufd kernel supports land).
It does not rely on future kernel support.
- Steve
