Re: [Qemu-devel] [PATCH V6 0/7] nvdimm: guarantee persistence of QEMU writes to persistent memory

[Junyan He]

According to my understand, the file node on real persistent memory do not need 
to be pmem=on

pmem=on is a feature for file backend class. For example, if we do not have 
enough hard disk space

and we have enough pmem, we can use file on pmem the same as normal 
file-backend on hard disk.

That is the file-backend on pmem does not necessary to a nvdimm backend, it can 
be normal file mapping as well.

so

 > detect that backing file is located on pmem storage

may not  be a good manner.


> (1) Maybe we should error out if pmem=on but compiled without libpmem
> and add 3rd state pmem=force for testing purposes.

I think we can print a warning message to give user a hint, it can really help 
user

when they mis-configure.


________________________________
From: Qemu-devel <address@hidden> on behalf of Igor Mammedov <address@hidden>
Sent: Tuesday, June 12, 2018 2:55:46 PM
To: Junyan He
Cc: Haozhong Zhang; address@hidden; address@hidden; address@hidden; 
address@hidden; address@hidden; address@hidden; address@hidden; Junyan He; 
address@hidden; address@hidden; address@hidden
Subject: Re: [Qemu-devel] [PATCH V6 0/7] nvdimm: guarantee persistence of QEMU 
writes to persistent memory

On Tue, 12 Jun 2018 13:38:08 +0000
Junyan He <address@hidden> wrote:

> He have pmem_persist and pmem_memcpy_persist stub functions.
>
> If no libpmem and user really specify pmem=on, we just do nothing or just 
> memcpy.
>
> Real persistent memory always require libpmem support its load/save.
>
> If pmem=on and without libpmem, we can think that user want to imitate
>
> pmem=on while the HW environment is without real persistent memory existing.
>
> It may help debug on some machine without real pmem.

For unaware user it would be easy to misconfigure and think that feature
works while it isn't, which cloud lead to data loss.

(1) Maybe we should error out if pmem=on but compiled without libpmem
and add 3rd state pmem=force for testing purposes.

Also can we detect that backing file is located on pmem storage and do [1] if 
it's not?

>
> ________________________________
> From: Qemu-devel <address@hidden> on behalf of Igor Mammedov <address@hidden>
> Sent: Tuesday, June 12, 2018 12:06:43 PM
> To: address@hidden
> Cc: Haozhong Zhang; address@hidden; address@hidden; address@hidden; 
> address@hidden; address@hidden; address@hidden; Junyan He; address@hidden; 
> address@hidden; address@hidden; address@hidden
> Subject: Re: [Qemu-devel] [PATCH V6 0/7] nvdimm: guarantee persistence of 
> QEMU writes to persistent memory
>
> On Fri,  1 Jun 2018 16:10:22 +0800
> address@hidden wrote:
>
> > From: Junyan He <address@hidden>
> >
> > QEMU writes to vNVDIMM backends in the vNVDIMM label emulation and
> > live migration. If the backend is on the persistent memory, QEMU needs
> > to take proper operations to ensure its writes persistent on the
> > persistent memory. Otherwise, a host power failure may result in the
> > loss the guest data on the persistent memory.
>
> extra question, what are expected behavior when QEMU is built without
> libpmem and user specifies pmem=on for backend?
>
> >
> > This v3 patch series is based on Marcel's patch "mem: add share
> > parameter to memory-backend-ram" [1] because of the changes in patch 1.
> >
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg03858.html
> >
> > Previous versions can be found at
> > v5: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg02258.html
> > V4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06993.html
> > v3: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04365.html
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg01579.html
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg05040.html
> >
> > Changes in v6:
> > * (Patch 1) Expose all ram block flags rather than redefine the flags.
> > * (Patch 4) Use pkg-config rather the hard check when configure.
> > * (Patch 7) Sync and flush all the pmem data when migration completes,
> > rather than sync pages one by one in previous version.
> >
> > Changes in v5:
> > * (Patch 9) Add post copy check and output some messages for nvdimm.
> >
> > Changes in v4:
> > * (Patch 2) Fix compilation errors found by patchew.
> >
> > Changes in v3:
> > * (Patch 5) Add a is_pmem flag to ram_handle_compressed() and handle
> > PMEM writes in it, so we don't need the _common function.
> > * (Patch 6) Expose qemu_get_buffer_common so we can remove the
> > unnecessary qemu_get_buffer_to_pmem wrapper.
> > * (Patch 8) Add a is_pmem flag to xbzrle_decode_buffer() and handle
> > PMEM writes in it, so we can remove the unnecessary
> > xbzrle_decode_buffer_{common, to_pmem}.
> > * Move libpmem stubs to stubs/pmem.c and fix the compilation failures
> > of test-{xbzrle,vmstate}.c.
> >
> > Changes in v2:
> > * (Patch 1) Use a flags parameter in file ram allocation functions.
> > * (Patch 2) Add a new option 'pmem' to hostmem-file.
> > * (Patch 3) Use libpmem to operate on the persistent memory, rather
> > than re-implementing those operations in QEMU.
> > * (Patch 5-8) Consider the write persistence in the migration path.
> >
> >
> > Junyan:
> > [1/7] memory, exec: Expose all memory block related flags.
> > [6/7] migration/ram: Add check and info message to nvdimm post copy.
> > [7/7] migration/ram: ensure write persistence on loading all date to PMEM.
> >
> > Haozhong:
> > [5/7] mem/nvdimm: ensure write persistence to PMEM in label emulation
> >
> > Haozhong & Junyan:
> > [2/7] memory, exec: switch file ram allocation functions to 'flags' 
> > parameters
> > [3/7] hostmem-file: add the 'pmem' option
> > [4/7] configure: add libpmem support
> >
> >
> > Signed-off-by: Haozhong Zhang <address@hidden>
> > Signed-off-by: Junyan He <address@hidden>
> >
> > ---
> > backends/hostmem-file.c | 28 +++++++++++++++++++++++++++-
> > configure               | 29 +++++++++++++++++++++++++++++
> > docs/nvdimm.txt         | 14 ++++++++++++++
> > exec.c                  | 36 ++++++++++++++----------------------
> > hw/mem/nvdimm.c         |  9 ++++++++-
> > include/exec/memory.h   | 31 +++++++++++++++++++++++++++++--
> > include/exec/ram_addr.h | 28 ++++++++++++++++++++++++++--
> > include/qemu/pmem.h     | 24 ++++++++++++++++++++++++
> > memory.c                |  8 +++++---
> > migration/ram.c         | 18 ++++++++++++++++++
> > numa.c                  |  2 +-
> > qemu-options.hx         |  7 +++++++
> > stubs/Makefile.objs     |  1 +
> > stubs/pmem.c            | 23 +++++++++++++++++++++++
> > 14 files changed, 226 insertions(+), 32 deletions(-)
>
>