Re: [PATCH V2] memory: RAM_NAMED_FILE flag

[Peter Xu]

On Wed, Jun 07, 2023 at 10:05:11AM -0400, Steven Sistare wrote:
> On 2/8/2023 2:22 PM, Peter Xu wrote:
> > On Wed, Feb 08, 2023 at 01:34:18PM -0500, Steven Sistare wrote:
> >> On 2/7/2023 3:23 PM, Peter Xu wrote:
> >>> On Tue, Feb 07, 2023 at 11:03:33AM -0800, Steve Sistare wrote:
> >>>> migrate_ignore_shared() is an optimization that avoids copying memory
> >>>> that is visible and can be mapped on the target.  However, a
> >>>> memory-backend-ram or a memory-backend-memfd block with the RAM_SHARED
> >>>> flag set is not migrated when migrate_ignore_shared() is true.  This is
> >>>> wrong, because the block has no named backing store, and its contents 
> >>>> will
> >>>> be lost.  To fix, ignore shared memory iff it is a named file.  Define a
> >>>> new flag RAM_NAMED_FILE to distinguish this case.
> >>>
> >>> There's also TYPE_MEMORY_BACKEND_EPC.  Reading the commit message it seems
> >>> it can still be used in similar ways.  Pasting commit message from 
> >>> c6c0232:
> >>>
> >>>     Because of its unique requirements, Linux manages EPC separately from
> >>>     normal memory.  Similar to memfd, the device /dev/sgx_vepc can be
> >>>     opened to obtain a file descriptor which can in turn be used to mmap()
> >>>     EPC memory.
> >>>
> >>> I'm not sure whether it means that should apply for RAM_NAMED_FILE too,
> >>> neither do I think it's super important..  Still better to define it
> >>> properly.
> >>
> >> The RAM_NAMED_FILE flag will be false for TYPE_MEMORY_BACKEND_EPC, so 
> >> ramblock_is_ignored will return false, and the contents will be copied
> >> over the socket to the target, and the segment will be recreated there.
> >>
> >> However, perhaps I do not understand your point.
> > 
> > My point was it looked like it should apply RAM_NAMED_FILE too, because
> > it's also a named file.
> > 
> > But..  I don't think another QEMU can share the same data if opening the
> > same file.  Based on my zero knowledge on it... I quickly looked up
> > sgx_vepc_open() in the Linux impl where sgx_vepc.page_array plays a role of
> > page cache iiuc, while it's private per vma even if VM_SHARED.
> > 
> > So please ignore my comment..
> > 
> >>
> >>> Another comment is, AFAIK this patch will modify senamtics of the old
> >>> capability "x-ignore-shared".  But I'd say in a sensible way.  Maybe worth
> >>> directly modify qapi/migration.json to reflect it (especially it's x-
> >>> prefixed) to something like:
> >>>
> >>> # @x-ignore-shared: If enabled, QEMU will not migrate named shared memory
> >>> #                   (since 4.0) 
> >>
> >> Good idea.  I propose:
> >>
> >> # @x-ignore-shared: If enabled, QEMU will not migrate shared memory that is
> >> #                   accessible on the target. (since 4.0)
> > 
> > I would use s/target/destination host/ because target can be misread as the
> > VM itself (at least a general term in migration code base).  Other than
> > that looks good to me.
> 
> Hi Peter, I will rebase to the tip, modify the wording, and repost.  
> Can I add your RB?

Yes. thanks,

-- 
Peter Xu