[Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory

Haozhong Zhang posted 8 patches 7 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180228072558.7434-1-haozhong.zhang@intel.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
There is a newer version of this series
backends/hostmem-file.c             | 27 +++++++++++++++++++-
configure                           | 35 ++++++++++++++++++++++++++
docs/nvdimm.txt                     | 14 +++++++++++
exec.c                              | 20 ++++++++++++---
hw/mem/nvdimm.c                     |  9 ++++++-
include/exec/memory.h               | 12 +++++++--
include/exec/ram_addr.h             | 28 +++++++++++++++++++--
include/migration/qemu-file-types.h |  2 ++
include/qemu/pmem.h                 | 27 ++++++++++++++++++++
memory.c                            |  8 +++---
migration/qemu-file.c               | 29 ++++++++++++++--------
migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
migration/ram.h                     |  2 +-
migration/rdma.c                    |  2 +-
migration/xbzrle.c                  |  8 ++++--
migration/xbzrle.h                  |  3 ++-
numa.c                              |  2 +-
qemu-options.hx                     |  9 ++++++-
stubs/Makefile.objs                 |  1 +
stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
tests/Makefile.include              |  4 +--
tests/test-xbzrle.c                 |  4 +--
22 files changed, 285 insertions(+), 47 deletions(-)
create mode 100644 include/qemu/pmem.h
create mode 100644 stubs/pmem.c
[Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Haozhong Zhang 7 years, 7 months ago
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.

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
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 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.

Haozhong Zhang (8):
  [1/8] memory, exec: switch file ram allocation functions to 'flags' parameters
  [2/8] hostmem-file: add the 'pmem' option
  [3/8] configure: add libpmem support
  [4/8] mem/nvdimm: ensure write persistence to PMEM in label emulation
  [5/8] migration/ram: ensure write persistence on loading zero pages to PMEM
  [6/8] migration/ram: ensure write persistence on loading normal pages to PMEM
  [7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM
  [8/8] migration/ram: ensure write persistence on loading xbzrle pages to PMEM

 backends/hostmem-file.c             | 27 +++++++++++++++++++-
 configure                           | 35 ++++++++++++++++++++++++++
 docs/nvdimm.txt                     | 14 +++++++++++
 exec.c                              | 20 ++++++++++++---
 hw/mem/nvdimm.c                     |  9 ++++++-
 include/exec/memory.h               | 12 +++++++--
 include/exec/ram_addr.h             | 28 +++++++++++++++++++--
 include/migration/qemu-file-types.h |  2 ++
 include/qemu/pmem.h                 | 27 ++++++++++++++++++++
 memory.c                            |  8 +++---
 migration/qemu-file.c               | 29 ++++++++++++++--------
 migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
 migration/ram.h                     |  2 +-
 migration/rdma.c                    |  2 +-
 migration/xbzrle.c                  |  8 ++++--
 migration/xbzrle.h                  |  3 ++-
 numa.c                              |  2 +-
 qemu-options.hx                     |  9 ++++++-
 stubs/Makefile.objs                 |  1 +
 stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
 tests/Makefile.include              |  4 +--
 tests/test-xbzrle.c                 |  4 +--
 22 files changed, 285 insertions(+), 47 deletions(-)
 create mode 100644 include/qemu/pmem.h
 create mode 100644 stubs/pmem.c

-- 
2.14.1


Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Haozhong Zhang 7 years, 7 months ago
On 02/28/18 15:25 +0800, Haozhong Zhang wrote:
> 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.
>


> 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

I forgot to remove this part. v4 can be applied on the current master
branch now because above [1] has already been merged.

Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Stefan Hajnoczi 7 years, 7 months ago
On Wed, Feb 28, 2018 at 03:25:50PM +0800, Haozhong Zhang wrote:
> 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.
> 
> 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
> 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 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.
> 
> Haozhong Zhang (8):
>   [1/8] memory, exec: switch file ram allocation functions to 'flags' parameters
>   [2/8] hostmem-file: add the 'pmem' option
>   [3/8] configure: add libpmem support
>   [4/8] mem/nvdimm: ensure write persistence to PMEM in label emulation
>   [5/8] migration/ram: ensure write persistence on loading zero pages to PMEM
>   [6/8] migration/ram: ensure write persistence on loading normal pages to PMEM
>   [7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM
>   [8/8] migration/ram: ensure write persistence on loading xbzrle pages to PMEM
> 
>  backends/hostmem-file.c             | 27 +++++++++++++++++++-
>  configure                           | 35 ++++++++++++++++++++++++++
>  docs/nvdimm.txt                     | 14 +++++++++++
>  exec.c                              | 20 ++++++++++++---
>  hw/mem/nvdimm.c                     |  9 ++++++-
>  include/exec/memory.h               | 12 +++++++--
>  include/exec/ram_addr.h             | 28 +++++++++++++++++++--
>  include/migration/qemu-file-types.h |  2 ++
>  include/qemu/pmem.h                 | 27 ++++++++++++++++++++
>  memory.c                            |  8 +++---
>  migration/qemu-file.c               | 29 ++++++++++++++--------
>  migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
>  migration/ram.h                     |  2 +-
>  migration/rdma.c                    |  2 +-
>  migration/xbzrle.c                  |  8 ++++--
>  migration/xbzrle.h                  |  3 ++-
>  numa.c                              |  2 +-
>  qemu-options.hx                     |  9 ++++++-
>  stubs/Makefile.objs                 |  1 +
>  stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
>  tests/Makefile.include              |  4 +--
>  tests/test-xbzrle.c                 |  4 +--
>  22 files changed, 285 insertions(+), 47 deletions(-)
>  create mode 100644 include/qemu/pmem.h
>  create mode 100644 stubs/pmem.c

A few thoughts:

1. Can you use pmem_is_pmem() to auto-detect the pmem=on|off value?

2. The migration/ram code is invasive.  Is it really necessary to
   persist data each time pages are loaded from a migration stream?  It
   seems simpler to migrate as normal and call pmem_persist() just once
   after RAM has been migrated but before the migration completes.

3. This is independent of this patch series and can be done later.
   NVDIMM seems incompatible with post-copy live migration.  It would be
   good to have a postcopy_add_blocker() API so that a nice error
   message is printed if post-copy live migration is attempted.

The code itself seems fine though:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Haozhong Zhang 7 years, 7 months ago
On 03/12/18 15:39 +0000, Stefan Hajnoczi wrote:
> On Wed, Feb 28, 2018 at 03:25:50PM +0800, Haozhong Zhang wrote:
> > 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.
> > 
> > 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
> > 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 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.
> > 
> > Haozhong Zhang (8):
> >   [1/8] memory, exec: switch file ram allocation functions to 'flags' parameters
> >   [2/8] hostmem-file: add the 'pmem' option
> >   [3/8] configure: add libpmem support
> >   [4/8] mem/nvdimm: ensure write persistence to PMEM in label emulation
> >   [5/8] migration/ram: ensure write persistence on loading zero pages to PMEM
> >   [6/8] migration/ram: ensure write persistence on loading normal pages to PMEM
> >   [7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM
> >   [8/8] migration/ram: ensure write persistence on loading xbzrle pages to PMEM
> > 
> >  backends/hostmem-file.c             | 27 +++++++++++++++++++-
> >  configure                           | 35 ++++++++++++++++++++++++++
> >  docs/nvdimm.txt                     | 14 +++++++++++
> >  exec.c                              | 20 ++++++++++++---
> >  hw/mem/nvdimm.c                     |  9 ++++++-
> >  include/exec/memory.h               | 12 +++++++--
> >  include/exec/ram_addr.h             | 28 +++++++++++++++++++--
> >  include/migration/qemu-file-types.h |  2 ++
> >  include/qemu/pmem.h                 | 27 ++++++++++++++++++++
> >  memory.c                            |  8 +++---
> >  migration/qemu-file.c               | 29 ++++++++++++++--------
> >  migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
> >  migration/ram.h                     |  2 +-
> >  migration/rdma.c                    |  2 +-
> >  migration/xbzrle.c                  |  8 ++++--
> >  migration/xbzrle.h                  |  3 ++-
> >  numa.c                              |  2 +-
> >  qemu-options.hx                     |  9 ++++++-
> >  stubs/Makefile.objs                 |  1 +
> >  stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
> >  tests/Makefile.include              |  4 +--
> >  tests/test-xbzrle.c                 |  4 +--
> >  22 files changed, 285 insertions(+), 47 deletions(-)
> >  create mode 100644 include/qemu/pmem.h
> >  create mode 100644 stubs/pmem.c
> 
> A few thoughts:
> 
> 1. Can you use pmem_is_pmem() to auto-detect the pmem=on|off value?

The manpage [1] of pmem_is_pmem says:

 "The result of pmem_is_pmem() query is only valid for the mappings
  created using pmem_map_file().  For other memory regions, in
  particular those created by a direct call to mmap(2), pmem_is_pmem()
  always returns false, even if the queried range is entirely
  persistent memory."

QEMU is using mmap for NVDIMM mapping, so pmem_is_pmem does not work.

[1] http://pmem.io/pmdk/manpages/linux/master/libpmem/pmem_is_pmem.3#caveats

> 
> 2. The migration/ram code is invasive.  Is it really necessary to
>    persist data each time pages are loaded from a migration stream?  It
>    seems simpler to migrate as normal and call pmem_persist() just once
>    after RAM has been migrated but before the migration completes.

The concern is about the overhead of cache flush.

In this patch series, if possible, QEMU will use pmem_mem{set,cpy}_nodrain
APIs to copy NVDIMM blocks. Those APIs use movnt (if it's available) and
can avoid the subsequent cache flush.

Anyway, I'll make some microbenchmark to check which one will be better.


> 
> 3. This is independent of this patch series and can be done later.
>    NVDIMM seems incompatible with post-copy live migration.  It would be
>    good to have a postcopy_add_blocker() API so that a nice error
>    message is printed if post-copy live migration is attempted.

Post-copy with NVDIMM currently fails with message "Postcopy on shared
RAM (...) is not yet supported". Is it enough?

> 
> The code itself seems fine though:
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks,
Haozhong

Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Dr. David Alan Gilbert 7 years, 7 months ago
* Haozhong Zhang (haozhong.zhang@intel.com) wrote:
> On 03/12/18 15:39 +0000, Stefan Hajnoczi wrote:
> > On Wed, Feb 28, 2018 at 03:25:50PM +0800, Haozhong Zhang wrote:
> > > 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.
> > > 
> > > 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
> > > 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 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.
> > > 
> > > Haozhong Zhang (8):
> > >   [1/8] memory, exec: switch file ram allocation functions to 'flags' parameters
> > >   [2/8] hostmem-file: add the 'pmem' option
> > >   [3/8] configure: add libpmem support
> > >   [4/8] mem/nvdimm: ensure write persistence to PMEM in label emulation
> > >   [5/8] migration/ram: ensure write persistence on loading zero pages to PMEM
> > >   [6/8] migration/ram: ensure write persistence on loading normal pages to PMEM
> > >   [7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM
> > >   [8/8] migration/ram: ensure write persistence on loading xbzrle pages to PMEM
> > > 
> > >  backends/hostmem-file.c             | 27 +++++++++++++++++++-
> > >  configure                           | 35 ++++++++++++++++++++++++++
> > >  docs/nvdimm.txt                     | 14 +++++++++++
> > >  exec.c                              | 20 ++++++++++++---
> > >  hw/mem/nvdimm.c                     |  9 ++++++-
> > >  include/exec/memory.h               | 12 +++++++--
> > >  include/exec/ram_addr.h             | 28 +++++++++++++++++++--
> > >  include/migration/qemu-file-types.h |  2 ++
> > >  include/qemu/pmem.h                 | 27 ++++++++++++++++++++
> > >  memory.c                            |  8 +++---
> > >  migration/qemu-file.c               | 29 ++++++++++++++--------
> > >  migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
> > >  migration/ram.h                     |  2 +-
> > >  migration/rdma.c                    |  2 +-
> > >  migration/xbzrle.c                  |  8 ++++--
> > >  migration/xbzrle.h                  |  3 ++-
> > >  numa.c                              |  2 +-
> > >  qemu-options.hx                     |  9 ++++++-
> > >  stubs/Makefile.objs                 |  1 +
> > >  stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
> > >  tests/Makefile.include              |  4 +--
> > >  tests/test-xbzrle.c                 |  4 +--
> > >  22 files changed, 285 insertions(+), 47 deletions(-)
> > >  create mode 100644 include/qemu/pmem.h
> > >  create mode 100644 stubs/pmem.c
> > 
> > A few thoughts:
> > 
> > 1. Can you use pmem_is_pmem() to auto-detect the pmem=on|off value?
> 
> The manpage [1] of pmem_is_pmem says:
> 
>  "The result of pmem_is_pmem() query is only valid for the mappings
>   created using pmem_map_file().  For other memory regions, in
>   particular those created by a direct call to mmap(2), pmem_is_pmem()
>   always returns false, even if the queried range is entirely
>   persistent memory."
> 
> QEMU is using mmap for NVDIMM mapping, so pmem_is_pmem does not work.
> 
> [1] http://pmem.io/pmdk/manpages/linux/master/libpmem/pmem_is_pmem.3#caveats
> 
> > 
> > 2. The migration/ram code is invasive.  Is it really necessary to
> >    persist data each time pages are loaded from a migration stream?  It
> >    seems simpler to migrate as normal and call pmem_persist() just once
> >    after RAM has been migrated but before the migration completes.
> 
> The concern is about the overhead of cache flush.
> 
> In this patch series, if possible, QEMU will use pmem_mem{set,cpy}_nodrain
> APIs to copy NVDIMM blocks. Those APIs use movnt (if it's available) and
> can avoid the subsequent cache flush.
> 
> Anyway, I'll make some microbenchmark to check which one will be better.

The problem is not just the overhead; the problem is the code
complexity; this series makes all the paths through the migration code
more complex in places we wouldn't expect to change.

> 
> > 
> > 3. This is independent of this patch series and can be done later.
> >    NVDIMM seems incompatible with post-copy live migration.  It would be
> >    good to have a postcopy_add_blocker() API so that a nice error
> >    message is printed if post-copy live migration is attempted.
> 
> Post-copy with NVDIMM currently fails with message "Postcopy on shared
> RAM (...) is not yet supported". Is it enough?

Once shared support arrives (see my patch series) that check goes
though; it might get trapped by one of the other checks though as well;
I'll need to try simulated pmem to find out.

Dave

> > 
> > The code itself seems fine though:
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Thanks,
> Haozhong
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Stefan Hajnoczi 7 years, 7 months ago
On Tue, Mar 13, 2018 at 08:11:50AM +0800, Haozhong Zhang wrote:
> On 03/12/18 15:39 +0000, Stefan Hajnoczi wrote:
> > On Wed, Feb 28, 2018 at 03:25:50PM +0800, Haozhong Zhang wrote:
> > > 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.
> > > 
> > > 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
> > > 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 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.
> > > 
> > > Haozhong Zhang (8):
> > >   [1/8] memory, exec: switch file ram allocation functions to 'flags' parameters
> > >   [2/8] hostmem-file: add the 'pmem' option
> > >   [3/8] configure: add libpmem support
> > >   [4/8] mem/nvdimm: ensure write persistence to PMEM in label emulation
> > >   [5/8] migration/ram: ensure write persistence on loading zero pages to PMEM
> > >   [6/8] migration/ram: ensure write persistence on loading normal pages to PMEM
> > >   [7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM
> > >   [8/8] migration/ram: ensure write persistence on loading xbzrle pages to PMEM
> > > 
> > >  backends/hostmem-file.c             | 27 +++++++++++++++++++-
> > >  configure                           | 35 ++++++++++++++++++++++++++
> > >  docs/nvdimm.txt                     | 14 +++++++++++
> > >  exec.c                              | 20 ++++++++++++---
> > >  hw/mem/nvdimm.c                     |  9 ++++++-
> > >  include/exec/memory.h               | 12 +++++++--
> > >  include/exec/ram_addr.h             | 28 +++++++++++++++++++--
> > >  include/migration/qemu-file-types.h |  2 ++
> > >  include/qemu/pmem.h                 | 27 ++++++++++++++++++++
> > >  memory.c                            |  8 +++---
> > >  migration/qemu-file.c               | 29 ++++++++++++++--------
> > >  migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
> > >  migration/ram.h                     |  2 +-
> > >  migration/rdma.c                    |  2 +-
> > >  migration/xbzrle.c                  |  8 ++++--
> > >  migration/xbzrle.h                  |  3 ++-
> > >  numa.c                              |  2 +-
> > >  qemu-options.hx                     |  9 ++++++-
> > >  stubs/Makefile.objs                 |  1 +
> > >  stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
> > >  tests/Makefile.include              |  4 +--
> > >  tests/test-xbzrle.c                 |  4 +--
> > >  22 files changed, 285 insertions(+), 47 deletions(-)
> > >  create mode 100644 include/qemu/pmem.h
> > >  create mode 100644 stubs/pmem.c
> > 
> > A few thoughts:
> > 
> > 1. Can you use pmem_is_pmem() to auto-detect the pmem=on|off value?
> 
> The manpage [1] of pmem_is_pmem says:
> 
>  "The result of pmem_is_pmem() query is only valid for the mappings
>   created using pmem_map_file().  For other memory regions, in
>   particular those created by a direct call to mmap(2), pmem_is_pmem()
>   always returns false, even if the queried range is entirely
>   persistent memory."
> 
> QEMU is using mmap for NVDIMM mapping, so pmem_is_pmem does not work.
> 
> [1] http://pmem.io/pmdk/manpages/linux/master/libpmem/pmem_is_pmem.3#caveats

Now that the PMDK library dependency is being added to QEMU,
pmem_map_file() could be used too.

> > 3. This is independent of this patch series and can be done later.
> >    NVDIMM seems incompatible with post-copy live migration.  It would be
> >    good to have a postcopy_add_blocker() API so that a nice error
> >    message is printed if post-copy live migration is attempted.
> 
> Post-copy with NVDIMM currently fails with message "Postcopy on shared
> RAM (...) is not yet supported". Is it enough?

Yes, that's better than I expected!

Stefan
Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Dr. David Alan Gilbert 7 years, 6 months ago
* Haozhong Zhang (haozhong.zhang@intel.com) wrote:

<snip>

> Post-copy with NVDIMM currently fails with message "Postcopy on shared
> RAM (...) is not yet supported". Is it enough?

What does it say now that postcopy-shared support is in?

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Haozhong Zhang 7 years, 6 months ago
On 03/29/18 20:12 +0100, Dr. David Alan Gilbert wrote:
> * Haozhong Zhang (haozhong.zhang@intel.com) wrote:
> 
> <snip>
> 
> > Post-copy with NVDIMM currently fails with message "Postcopy on shared
> > RAM (...) is not yet supported". Is it enough?
> 
> What does it say now that postcopy-shared support is in?
> 

I'll check it later.

Haozhong

Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory
Posted by Haozhong Zhang 7 years, 7 months ago
Ping?

On 02/28/18 15:25 +0800, Haozhong Zhang wrote:
> 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.
> 
> 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
> 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 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.
> 
> Haozhong Zhang (8):
>   [1/8] memory, exec: switch file ram allocation functions to 'flags' parameters
>   [2/8] hostmem-file: add the 'pmem' option
>   [3/8] configure: add libpmem support
>   [4/8] mem/nvdimm: ensure write persistence to PMEM in label emulation
>   [5/8] migration/ram: ensure write persistence on loading zero pages to PMEM
>   [6/8] migration/ram: ensure write persistence on loading normal pages to PMEM
>   [7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM
>   [8/8] migration/ram: ensure write persistence on loading xbzrle pages to PMEM
> 
>  backends/hostmem-file.c             | 27 +++++++++++++++++++-
>  configure                           | 35 ++++++++++++++++++++++++++
>  docs/nvdimm.txt                     | 14 +++++++++++
>  exec.c                              | 20 ++++++++++++---
>  hw/mem/nvdimm.c                     |  9 ++++++-
>  include/exec/memory.h               | 12 +++++++--
>  include/exec/ram_addr.h             | 28 +++++++++++++++++++--
>  include/migration/qemu-file-types.h |  2 ++
>  include/qemu/pmem.h                 | 27 ++++++++++++++++++++
>  memory.c                            |  8 +++---
>  migration/qemu-file.c               | 29 ++++++++++++++--------
>  migration/ram.c                     | 49 +++++++++++++++++++++++++++----------
>  migration/ram.h                     |  2 +-
>  migration/rdma.c                    |  2 +-
>  migration/xbzrle.c                  |  8 ++++--
>  migration/xbzrle.h                  |  3 ++-
>  numa.c                              |  2 +-
>  qemu-options.hx                     |  9 ++++++-
>  stubs/Makefile.objs                 |  1 +
>  stubs/pmem.c                        | 37 ++++++++++++++++++++++++++++
>  tests/Makefile.include              |  4 +--
>  tests/test-xbzrle.c                 |  4 +--
>  22 files changed, 285 insertions(+), 47 deletions(-)
>  create mode 100644 include/qemu/pmem.h
>  create mode 100644 stubs/pmem.c
> 
> -- 
> 2.14.1
>