[PATCH v1 1/3] softmmu/physmem: fallback to opening guest RAM file as readonly in a MAP_PRIVATE mapping

[Peter Xu]

On Fri, Aug 11, 2023 at 07:39:37PM +0200, David Hildenbrand wrote:
> On 11.08.23 18:54, Peter Xu wrote:
> > On Fri, Aug 11, 2023 at 06:25:14PM +0200, David Hildenbrand wrote:
> > > On 11.08.23 18:22, Peter Xu wrote:
> > > > On Fri, Aug 11, 2023 at 06:17:05PM +0200, David Hildenbrand wrote:
> > > > > We wouldn't touch "-mem-path".
> > > > 
> > > > But still the same issue when someone uses -object memory-backend-file for
> > > > hugetlb, mapping privately, expecting ram discard to work?
> > > > 
> > > > Basically I see that example as, "hugetlb" in general made the private
> > > > mapping over RW file usable, so forbidden that anywhere may take a risk.
> > > 
> > > These users can be directed to using hugetlb
> > > 
> > > a) using MAP_SHARED
> > > b) using memory-backend-memfd, if MAP_PRIVATE is desired
> > > 
> > > Am I missing any important use case? Are we being a bit to careful about
> > > virtio-balloon and postcopy simply not being available for these corner
> > > cases?
> > 
> > The current immediate issue is not really mem=rw + fd=rw + private case
> > (which was a known issue), but how to make mem=rw + fd=ro + private work
> > for ThinnerBloger, iiuc.
> > 
> > I'd just think it safer to expose that cap to solve problem A (vm
> > templating) without affecting problem B (fallcate-over-private not working
> > right), when B is uncertain.
> 
> Right, and I'm thinking about if B is worth the effort.
> 
> > 
> > I'm also copy Daniel & libvirt list in case there's quick comment from
> > there. Say, maybe libvirt never use private mapping on hugetlb files over
> > memory-backend-file at all, then it's probably fine.
> 
> libvirt certainly allows setting <access mode="shared"/> with <source
> type="file">.
> 
> Could be that they also end up mapping "<hugepages>" to memory-backend-file
> instead of memory-backend-memfd (e.g., compatibility with older kernels?).
> 
> > 
> > In all cases, you and Igor should have the final grasp; no stand on a
> > strong opinon from my side.
> 
> I do value your opinion, so I'm still trying to figure out if there are sane
> use cases that really need a new parameter. Let's recap:
> 
> When opening the file R/O, resulting in fallocate() refusing to work:
> * virtio-balloon will fail to discard RAM but continue to "be alive"
> * virtio-mem will discard any private pages, but cannot free up disk
>   blocks using fallocate.
> * postcopy would fail early
> 
> Postcopy:
> * Works on shmem (MAP_SHARED / MAP_PRIVATE)
> * Works on hugetlb (MAP_SHARED / MAP_PRIVATE)
> * Does not work on file-backed memory (including MAP_PRIVATE)
> 
> We can ignore virtio-mem for now. What remains is postcopy and
> virtio-balloon.
> 
> memory-backend-file with MAP_PRIVATE on shmem/tmpfs results in a double
> memory consumption, so we can mostly cross that out as "sane use case".
> Rather make such users aware of that :D
> 
> memory-backend-file with MAP_PRIVATE on hugetlb works. virtio-balloon is not
> really compatible with hugetlb, free-page-reporting might work (although
> quite non-nonsensical). So postcopy as the most important use case remains.
> 
> memory-backend-file with MAP_PRIVATE on file-backed memory works. postcopy
> does not apply. virtio-balloon should work I guess.
> 
> 
> So the two use cases that are left are:
> * postcopy with hugetlb would fail
> * virtio-balloon with file-backed memory cannot free up disk blocks
> 
> Am I missing a case?

Looks complete.  I don't want to say so, but afaik postcopy should be
"corner case" in most cases I'd say; people do still rely mostly on
precopy.  It's probably a matter of whether we'd like take the risk.

-- 
Peter Xu