Re: [Qemu-devel] [PATCH v5 00/11] Add basic "detach" support for dump-guest-memory

[Fam Zheng]

On Mon, 12/07 13:56, Peter Xu wrote:
> v5 changes:
> - patch 1
>   - comment English fix [Fam]
> - patch 2
>   - pass has_detach=true always in hmp_dump_guest_memory [Paolo]
> - patch 3
>   - always use local_err and error_propagate() when need to check
>     the result [Fam]
> - patch 8
>   - add "DumpQueryResult" in DUMP_COMPLETED event [Eric]
>     (since DumpQueryResult is introduced in patch 10, so doing it in
>     patch 10 for convenience. Please let me know if I should not do
>     this, e.g., if patch re-ordering is required)

Apart from the questions that I am not sure,

Reviewed-by: Fam Zheng <address@hidden>

> 
> v4 changes:
> - patch 2:
>   - hmp: fix default value lost [Eric]
>   - English errors [Eric]
> - patch 3:
>   - use global DumpState, leverage C99 struct init [Paolo]
>   - English errors [Eric]
> - patch 5:
>   - more cleanup for dump_process [Paolo]
> - patch 8:
>   - make sure qmp-events.txt is sorted [Eric]
>   - enhance error_get_pretty() [Eric]
>   - emit DUMP_COMPLETED no matter detach or not
> - patch 10:
>   - use g_new0 to replace g_malloc0 [Eric]
>   - rename "written_bytes" to "completed", "total_bytes" to "total"
>     [Eric]
>   - use atomic ops and [rw]mb to protect status read/write [Paolo]
> - patch 12:
>   - English errors [Eric]
>   - merge contents into older patches [Eric]
> 
> v3 changes (patch number corresponds to v2 patch set):
> - patch 1
>   - fix commit message. no memory leak, only code cleanup [Fam]
> - patch 2
>   - better documentation for "dump-guest-memory" (new patch 9) [Fam]
> - patch 3
>   - remove rcu lock/unlock in dump_init() [Fam, Paolo]
>   - embed mr pointer into GuestPhysBlock [Paolo]
>   - remove global dump state [Paolo]
> - patch 4
>   - fix memory leak for error [Fam]
>   - evt DUMP_COMPLETED data: change to an optional "*error" [Paolo]
> - patch 5
>   - fix documents [Fam]
>   - change "dump-query" to "query-dump", HMP to "info dump" [Paolo]
> - patch 6
>   - for query-dump command: define enum for DumpStatus, use "int"
>     for written/total [Paolo]
> - all
>   - reorder the commits as suggested, no fake values [Paolo]
>   - split big commit into smaller ones [me]
> 
> v2 changes:
> - fixed English errors [Drew]
> - reordered the "detach" field, first make it optional, then make sure
>   it's order is consistent [Drew, Fam]
> - added doc for new detach flag [Eric]
> - collected error msg even detached [Drew]
> - added qmp event DUMP_COMPLETED to notify user [Eric, Fam]
> - added "dump-query" QMP & HMP commands to query dump status [Eric]
> - "stop" is not allowed when dump in background (also include
>   "cont" and "dump-guest-memory") [Fam]
> - added codes to calculate how many dump work finished, which could
>   be queried from "dump-query" [Laszlo]
> - added list to track all used MemoryRegion objects, also ref before
>   use [Paolo]
> - dump-guest-memory will be forbidden during incoming migrate [Paolo]
> - taking rcu lock when collecting memory info [Paolo]
> 
> Test Done:
> - QMP & HMP
>   - test default dump (sync), work as usual
>   - test detached dump, command return immediately.
>   - When dump finished, will receive event DUMP_COMPLETED. 
>   - test query-dump before/during/after dump
>   - test kdump with zlib compression, w/ and w/o detach
> - libvirt
>   - test "virsh dump --memory-only" with default format and
>     kdump-zlib format, work as usual
> 
> Peter Xu (11):
>   dump-guest-memory: cleanup: removing dump_{error|cleanup}().
>   dump-guest-memory: add "detach" flag for QMP/HMP interfaces.
>   dump-guest-memory: using static DumpState, add DumpStatus
>   dump-guest-memory: add dump_in_progress() helper function
>   dump-guest-memory: introduce dump_process() helper function.
>   dump-guest-memory: disable dump when in INMIGRATE state
>   dump-guest-memory: add "detach" support
>   dump-guest-memory: add qmp event DUMP_COMPLETED
>   DumpState: adding total_size and written_size fields
>   Dump: add qmp command "query-dump"
>   Dump: add hmp command "info dump"
> 
>  docs/qmp-events.txt             |  19 ++++
>  dump.c                          | 215 
> ++++++++++++++++++++++++++++++----------
>  hmp-commands-info.hx            |  14 +++
>  hmp-commands.hx                 |   5 +-
>  hmp.c                           |  27 ++++-
>  hmp.h                           |   1 +
>  include/qemu-common.h           |   4 +
>  include/sysemu/dump.h           |  15 +++
>  include/sysemu/memory_mapping.h |   4 +
>  memory_mapping.c                |   3 +
>  qapi-schema.json                |  57 ++++++++++-
>  qapi/event.json                 |  16 +++
>  qmp-commands.hx                 |  32 +++++-
>  qmp.c                           |  14 +++
>  14 files changed, 363 insertions(+), 63 deletions(-)
> 
> -- 
> 2.4.3
>