Re: [Qemu-devel] [PATCH V9 4/6] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap()

[Eduardo Habkost]

On Mon, Jan 21, 2019 at 01:15:36PM +0800, Yi Zhang wrote:
> On 2019-01-18 at 16:11:47 -0200, Eduardo Habkost wrote:
[...]
> > Anyway, I see a more fundamental problem in each version of this
> > patch: the semantics of the command-line options are not clearly
> > documented.
> > 
> > We have at least 3 different possible use cases we might need to
> > support:
> > 
> > 1) pmem=on, MAP_SYNC not desired
> > 2) pmem=on, MAP_SYNC desired but optional
> 
> Form V9, As Michael suggest, We removed the sync option, MAP_SYNC will
> force on while we set pmem=on. So we only have 2 user cases, Will update
> to user documentation.
> 1) pmem=on, MAP_SYNC not desired
> We will not pass the flag to mmap2

If this use case is supported, how the command-line should look
like to enable it?


> 2) pmem=on, MAP_SYNC desired
> We will pass the flag to mmap2

Same question as above: how the command-line should look like for
this use case?

> 
> > 3) pmem=on, MAP_SYNC required, not optional
> > 
> > Which cases from the list above we need to support?
> > 
> > From the cases above, what's the expected semantics of "pmem=on"
> > with no extra options?

We still need to answer that question.

The current semantics of pmem=on (with no extra options) is (1).
It looks like we can't change it to (2) without breaking existing
configurations.  If you make existing configurations stop working
on hosts where they currently work, you need to explain why it's
OK to do that.


> > 
> > If these questions are not answered (in the commit message and
> > user documentation), we won't be able to review and discuss the
> > code.
> > 
> > 
[...]


-- 
Eduardo