Re: [Qemu-devel] [PATCH V10 4/4] docs: Added MAP_SYNC documentation

[Eduardo Habkost <ehabkost@redhat.com>]

On Wed, Jan 23, 2019 at 11:00:02AM +0800, Zhang, Yi wrote:
> From: Zhang Yi <yi.z.zhang@linux.intel.com>
> 
> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
> ---
>  docs/nvdimm.txt | 29 ++++++++++++++++++++++++++++-
>  qemu-options.hx |  4 ++++
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
> index 5f158a6..166c395 100644
> --- a/docs/nvdimm.txt
> +++ b/docs/nvdimm.txt
> @@ -142,11 +142,38 @@ backend of vNVDIMM:
>  Guest Data Persistence
>  ----------------------
>  
> +vNVDIMM is designed and implemented to guarantee the guest data
> +persistence on the backends in case of host crash or a power failures.
> +However, there are still some requirements and limitations
> +as explained below.
> +
>  Though QEMU supports multiple types of vNVDIMM backends on Linux,
> -currently the only one that can guarantee the guest write persistence
> +if MAP_SYNC is not supported by the host kernel and the backends,
> +the only backend that can guarantee the guest write persistence
>  is the device DAX on the real NVDIMM device (e.g., /dev/dax0.0), to
>  which all guest access do not involve any host-side kernel cache.
>  
> +mmap(2) flag MAP_SYNC is added since Linux kernel 4.15. On such
> +systems, QEMU can mmap(2) the dax backend files with MAP_SYNC, which
> +ensures filesystem metadata consistency in case of a host crash or a power
> +failure. Enabling MAP_SYNC in QEMU requires below conditions
> +
> + - 'pmem' option of memory-backend-file is 'on':
> +   The backend is a file supporting DAX, e.g., a file on an ext4 or
> +   xfs file system mounted with '-o dax'. if your pmem=on ,but the backend is
> +   not a file supporting DAX, mapping with this flag results in an EOPNOTSUPP
> +   error.

Won't this break existing configurations that work today on QEMU
3.1.0?  Why exactly it is OK to break compatibility here?

> +
> + - 'share' option of memory-backend-file is 'on':
> +   MAP_SYNC flag available only with the MAP_SHARED_VALIDATE mapping type.

I don't understand what this paragraph means.

> +
> + - 'MAP_SYNC' is supported on linux kernel.(default opened since Linux 4.15)
> +

I don't understand why you are making the semantics of
command-line options change depending on the host kernel.

> +Otherwise, We will ignore the MAP_SYNC flag.
> +

See the questions I sent about supported use cases at
<https://www.mail-archive.com/qemu-devel@nongnu.org/msg588822.html>.
I still don't see those questions answered:

] 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
] 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?

It's not clear to me yet if you want to support use cases (1) and (2).

Also, you seem to be choosing between use case (1) or (3) depending on
the build environment instead of command-line options.  The
meaning of command-line options must be predictable and
unambiguous, and not depend on build time variables.


> +For more details, please reference mmap(2) man page:
> +http://man7.org/linux/man-pages/man2/mmap.2.html.
> +
>  When using other types of backends, it's suggested to set 'unarmed'
>  option of '-device nvdimm' to 'on', which sets the unarmed flag of the
>  guest NVDIMM region mapping structure.  This unarmed flag indicates
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 08f8516..0cd41f4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4002,6 +4002,10 @@ using the SNIA NVM programming model (e.g. Intel NVDIMM).
>  If @option{pmem} is set to 'on', QEMU will take necessary operations to
>  guarantee the persistence of its own writes to @option{mem-path}
>  (e.g. in vNVDIMM label emulation and live migration).
> +Also, we will map the backend-file with MAP_SYNC flag, which can ensure
> +the file metadata is in sync to @option{mem-path} in case of host crash
> +or a power failure. MAP_SYNC requires support from both the host kernel
> +(since Linux kernel 4.15) and @option{mem-path} (only files supporting DAX).
>  
>  @item -object memory-backend-ram,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave}
>  
> -- 
> 2.7.4
> 

-- 
Eduardo