[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU wr
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v4 0/8] nvdimm: guarantee persistence of QEMU writes to persistent memory |
Date: |
Mon, 12 Mar 2018 15:39:54 +0000 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
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 <address@hidden>
signature.asc
Description: PGP signature