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.. > 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? 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). Thanks, -- Peter Xu
