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

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

[Peter Xu]

v6 changes:
- patch 10
  - English error fix [Fam]
- patch 11
  - remove useless var: "not_used" [me]
- all
  - move patch 8 to the end to be patch 11 (v5 patches 9-11 become
  v6 patches 8-10) [Eric]

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)

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
  DumpState: adding total_size and written_size fields
  Dump: add qmp command "query-dump"
  Dump: add hmp command "info dump"
  dump-guest-memory: add qmp event DUMP_COMPLETED

 docs/qmp-events.txt             |  18 ++++
 dump.c                          | 215 ++++++++++++++++++++++++++++++----------
 hmp-commands-info.hx            |  14 +++
 hmp-commands.hx                 |   5 +-
 hmp.c                           |  26 ++++-
 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                |  56 ++++++++++-
 qapi/event.json                 |  16 +++
 qmp-commands.hx                 |  31 +++++-
 qmp.c                           |  14 +++
 14 files changed, 359 insertions(+), 63 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v6 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}().

[Peter Xu]

It might be a little bit confusing and error prone to do
dump_cleanup() in these two functions. A better way is to do
dump_cleanup() before dump finish, no matter whether dump has
succeeded or not.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c | 78 +++++++++++++++++++++++++++---------------------------------------
 1 file changed, 32 insertions(+), 46 deletions(-)

diff --git a/dump.c b/dump.c
index 78b7d84..445e739 100644
--- a/dump.c
+++ b/dump.c
@@ -82,12 +82,6 @@ static int dump_cleanup(DumpState *s)
     return 0;
 }
 
-static void dump_error(DumpState *s, const char *reason, Error **errp)
-{
-    dump_cleanup(s);
-    error_setg(errp, "%s", reason);
-}
-
 static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
 {
     DumpState *s = opaque;
@@ -128,7 +122,7 @@ static void write_elf64_header(DumpState *s, Error **errp)
 
     ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write elf header", errp);
+        error_setg(errp, "dump: failed to write elf header");
     }
 }
 
@@ -159,7 +153,7 @@ static void write_elf32_header(DumpState *s, Error **errp)
 
     ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write elf header", errp);
+        error_setg(errp, "dump: failed to write elf header");
     }
 }
 
@@ -182,7 +176,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
 
     ret = fd_write_vmcore(&phdr, sizeof(Elf64_Phdr), s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write program header table", errp);
+        error_setg(errp, "dump: failed to write program header table");
     }
 }
 
@@ -205,7 +199,7 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
 
     ret = fd_write_vmcore(&phdr, sizeof(Elf32_Phdr), s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write program header table", errp);
+        error_setg(errp, "dump: failed to write program header table");
     }
 }
 
@@ -225,7 +219,7 @@ static void write_elf64_note(DumpState *s, Error **errp)
 
     ret = fd_write_vmcore(&phdr, sizeof(Elf64_Phdr), s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write program header table", errp);
+        error_setg(errp, "dump: failed to write program header table");
     }
 }
 
@@ -245,7 +239,7 @@ static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
         id = cpu_index(cpu);
         ret = cpu_write_elf64_note(f, cpu, id, s);
         if (ret < 0) {
-            dump_error(s, "dump: failed to write elf notes", errp);
+            error_setg(errp, "dump: failed to write elf notes");
             return;
         }
     }
@@ -253,7 +247,7 @@ static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
     CPU_FOREACH(cpu) {
         ret = cpu_write_elf64_qemunote(f, cpu, s);
         if (ret < 0) {
-            dump_error(s, "dump: failed to write CPU status", errp);
+            error_setg(errp, "dump: failed to write CPU status");
             return;
         }
     }
@@ -275,7 +269,7 @@ static void write_elf32_note(DumpState *s, Error **errp)
 
     ret = fd_write_vmcore(&phdr, sizeof(Elf32_Phdr), s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write program header table", errp);
+        error_setg(errp, "dump: failed to write program header table");
     }
 }
 
@@ -290,7 +284,7 @@ static void write_elf32_notes(WriteCoreDumpFunction f, DumpState *s,
         id = cpu_index(cpu);
         ret = cpu_write_elf32_note(f, cpu, id, s);
         if (ret < 0) {
-            dump_error(s, "dump: failed to write elf notes", errp);
+            error_setg(errp, "dump: failed to write elf notes");
             return;
         }
     }
@@ -298,7 +292,7 @@ static void write_elf32_notes(WriteCoreDumpFunction f, DumpState *s,
     CPU_FOREACH(cpu) {
         ret = cpu_write_elf32_qemunote(f, cpu, s);
         if (ret < 0) {
-            dump_error(s, "dump: failed to write CPU status", errp);
+            error_setg(errp, "dump: failed to write CPU status");
             return;
         }
     }
@@ -326,7 +320,7 @@ static void write_elf_section(DumpState *s, int type, Error **errp)
 
     ret = fd_write_vmcore(&shdr, shdr_size, s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write section header table", errp);
+        error_setg(errp, "dump: failed to write section header table");
     }
 }
 
@@ -336,7 +330,7 @@ static void write_data(DumpState *s, void *buf, int length, Error **errp)
 
     ret = fd_write_vmcore(buf, length, s);
     if (ret < 0) {
-        dump_error(s, "dump: failed to save memory", errp);
+        error_setg(errp, "dump: failed to save memory");
     }
 }
 
@@ -568,11 +562,6 @@ static void dump_begin(DumpState *s, Error **errp)
     }
 }
 
-static void dump_completed(DumpState *s)
-{
-    dump_cleanup(s);
-}
-
 static int get_next_block(DumpState *s, GuestPhysBlock *block)
 {
     while (1) {
@@ -624,8 +613,6 @@ static void dump_iterate(DumpState *s, Error **errp)
         }
 
     } while (!get_next_block(s, block));
-
-    dump_completed(s);
 }
 
 static void create_vmcore(DumpState *s, Error **errp)
@@ -765,7 +752,7 @@ static void create_header32(DumpState *s, Error **errp)
     dh->status = cpu_to_dump32(s, status);
 
     if (write_buffer(s->fd, 0, dh, size) < 0) {
-        dump_error(s, "dump: failed to write disk dump header", errp);
+        error_setg(errp, "dump: failed to write disk dump header");
         goto out;
     }
 
@@ -784,7 +771,7 @@ static void create_header32(DumpState *s, Error **errp)
 
     if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
                      block_size, kh, size) < 0) {
-        dump_error(s, "dump: failed to write kdump sub header", errp);
+        error_setg(errp, "dump: failed to write kdump sub header");
         goto out;
     }
 
@@ -800,7 +787,7 @@ static void create_header32(DumpState *s, Error **errp)
     }
     if (write_buffer(s->fd, offset_note, s->note_buf,
                      s->note_size) < 0) {
-        dump_error(s, "dump: failed to write notes", errp);
+        error_setg(errp, "dump: failed to write notes");
         goto out;
     }
 
@@ -865,7 +852,7 @@ static void create_header64(DumpState *s, Error **errp)
     dh->status = cpu_to_dump32(s, status);
 
     if (write_buffer(s->fd, 0, dh, size) < 0) {
-        dump_error(s, "dump: failed to write disk dump header", errp);
+        error_setg(errp, "dump: failed to write disk dump header");
         goto out;
     }
 
@@ -884,7 +871,7 @@ static void create_header64(DumpState *s, Error **errp)
 
     if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
                      block_size, kh, size) < 0) {
-        dump_error(s, "dump: failed to write kdump sub header", errp);
+        error_setg(errp, "dump: failed to write kdump sub header");
         goto out;
     }
 
@@ -901,7 +888,7 @@ static void create_header64(DumpState *s, Error **errp)
 
     if (write_buffer(s->fd, offset_note, s->note_buf,
                      s->note_size) < 0) {
-        dump_error(s, "dump: failed to write notes", errp);
+        error_setg(errp, "dump: failed to write notes");
         goto out;
     }
 
@@ -1064,7 +1051,7 @@ static void write_dump_bitmap(DumpState *s, Error **errp)
     while (get_next_page(&block_iter, &pfn, NULL, s)) {
         ret = set_dump_bitmap(last_pfn, pfn, true, dump_bitmap_buf, s);
         if (ret < 0) {
-            dump_error(s, "dump: failed to set dump_bitmap", errp);
+            error_setg(errp, "dump: failed to set dump_bitmap");
             goto out;
         }
 
@@ -1081,7 +1068,7 @@ static void write_dump_bitmap(DumpState *s, Error **errp)
         ret = set_dump_bitmap(last_pfn, last_pfn + PFN_BUFBITMAP, false,
                               dump_bitmap_buf, s);
         if (ret < 0) {
-            dump_error(s, "dump: failed to sync dump_bitmap", errp);
+            error_setg(errp, "dump: failed to sync dump_bitmap");
             goto out;
         }
     }
@@ -1214,7 +1201,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
     ret = write_cache(&page_data, buf, TARGET_PAGE_SIZE, false);
     g_free(buf);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write page data (zero page)", errp);
+        error_setg(errp, "dump: failed to write page data (zero page)");
         goto out;
     }
 
@@ -1230,7 +1217,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
             ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
                               false);
             if (ret < 0) {
-                dump_error(s, "dump: failed to write page desc", errp);
+                error_setg(errp, "dump: failed to write page desc");
                 goto out;
             }
         } else {
@@ -1255,7 +1242,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
 
                 ret = write_cache(&page_data, buf_out, size_out, false);
                 if (ret < 0) {
-                    dump_error(s, "dump: failed to write page data", errp);
+                    error_setg(errp, "dump: failed to write page data");
                     goto out;
                 }
 #ifdef CONFIG_LZO
@@ -1268,7 +1255,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
 
                 ret = write_cache(&page_data, buf_out, size_out, false);
                 if (ret < 0) {
-                    dump_error(s, "dump: failed to write page data", errp);
+                    error_setg(errp, "dump: failed to write page data");
                     goto out;
                 }
 #endif
@@ -1282,7 +1269,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
 
                 ret = write_cache(&page_data, buf_out, size_out, false);
                 if (ret < 0) {
-                    dump_error(s, "dump: failed to write page data", errp);
+                    error_setg(errp, "dump: failed to write page data");
                     goto out;
                 }
 #endif
@@ -1297,7 +1284,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
 
                 ret = write_cache(&page_data, buf, TARGET_PAGE_SIZE, false);
                 if (ret < 0) {
-                    dump_error(s, "dump: failed to write page data", errp);
+                    error_setg(errp, "dump: failed to write page data");
                     goto out;
                 }
             }
@@ -1309,7 +1296,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
 
             ret = write_cache(&page_desc, &pd, sizeof(PageDescriptor), false);
             if (ret < 0) {
-                dump_error(s, "dump: failed to write page desc", errp);
+                error_setg(errp, "dump: failed to write page desc");
                 goto out;
             }
         }
@@ -1317,12 +1304,12 @@ static void write_dump_pages(DumpState *s, Error **errp)
 
     ret = write_cache(&page_desc, NULL, 0, true);
     if (ret < 0) {
-        dump_error(s, "dump: failed to sync cache for page_desc", errp);
+        error_setg(errp, "dump: failed to sync cache for page_desc");
         goto out;
     }
     ret = write_cache(&page_data, NULL, 0, true);
     if (ret < 0) {
-        dump_error(s, "dump: failed to sync cache for page_data", errp);
+        error_setg(errp, "dump: failed to sync cache for page_data");
         goto out;
     }
 
@@ -1366,7 +1353,7 @@ static void create_kdump_vmcore(DumpState *s, Error **errp)
 
     ret = write_start_flat_header(s->fd);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write start flat header", errp);
+        error_setg(errp, "dump: failed to write start flat header");
         return;
     }
 
@@ -1390,11 +1377,9 @@ static void create_kdump_vmcore(DumpState *s, Error **errp)
 
     ret = write_end_flat_header(s->fd);
     if (ret < 0) {
-        dump_error(s, "dump: failed to write end flat header", errp);
+        error_setg(errp, "dump: failed to write end flat header");
         return;
     }
-
-    dump_completed(s);
 }
 
 static ram_addr_t get_start_block(DumpState *s)
@@ -1677,6 +1662,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
         create_vmcore(s, errp);
     }
 
+    dump_cleanup(s);
     g_free(s);
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v6 02/11] dump-guest-memory: add "detach" flag for QMP/HMP interfaces.

[Peter Xu]

This patch only adds the interfaces, but does not implement them.
"detach" parameter is made optional, to make sure that all the old
dump-guest-memory requests will still be able to work.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c           | 5 +++--
 hmp-commands.hx  | 5 +++--
 hmp.c            | 9 +++++++--
 qapi-schema.json | 8 ++++++--
 qmp-commands.hx  | 6 ++++--
 5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/dump.c b/dump.c
index 445e739..d79e0ed 100644
--- a/dump.c
+++ b/dump.c
@@ -1580,8 +1580,9 @@ cleanup:
     dump_cleanup(s);
 }
 
-void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
-                           int64_t begin, bool has_length,
+void qmp_dump_guest_memory(bool paging, const char *file,
+                           bool has_detach, bool detach,
+                           bool has_begin, int64_t begin, bool has_length,
                            int64_t length, bool has_format,
                            DumpGuestMemoryFormat format, Error **errp)
 {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..664d794 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1056,10 +1056,11 @@ ETEXI
 
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:i?,length:i?",
-        .params     = "[-p] [-z|-l|-s] filename [begin length]",
+        .args_type  = "paging:-p,detach:-d,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:i?,length:i?",
+        .params     = "[-p] [-d] [-z|-l|-s] filename [begin length]",
         .help       = "dump guest memory into file 'filename'.\n\t\t\t"
                       "-p: do paging to get guest's memory mapping.\n\t\t\t"
+                      "-d: return immediately (do not wait for completion).\n\t\t\t"
                       "-z: dump in kdump-compressed format, with zlib compression.\n\t\t\t"
                       "-l: dump in kdump-compressed format, with lzo compression.\n\t\t\t"
                       "-s: dump in kdump-compressed format, with snappy compression.\n\t\t\t"
diff --git a/hmp.c b/hmp.c
index 2140605..1f4d0b6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1586,8 +1586,10 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     const char *file = qdict_get_str(qdict, "filename");
     bool has_begin = qdict_haskey(qdict, "begin");
     bool has_length = qdict_haskey(qdict, "length");
+    bool has_detach = qdict_haskey(qdict, "detach");
     int64_t begin = 0;
     int64_t length = 0;
+    bool detach = false;
     enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
     char *prot;
 
@@ -1615,11 +1617,14 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     if (has_length) {
         length = qdict_get_int(qdict, "length");
     }
+    if (has_detach) {
+        detach = qdict_get_bool(qdict, "detach");
+    }
 
     prot = g_strconcat("file:", file, NULL);
 
-    qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
-                          true, dump_format, &err);
+    qmp_dump_guest_memory(paging, prot, true, detach, has_begin, begin,
+                          has_length, length, true, dump_format, &err);
     hmp_handle_error(mon, &err);
     g_free(prot);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423..97c3ac4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2115,6 +2115,9 @@
 #            2. fd: the protocol starts with "fd:", and the following string
 #               is the fd's name.
 #
+# @detach: #optional if true, QMP will return immediately rather than
+#          waiting for the dump to finish. (since 2.6).
+#
 # @begin: #optional if specified, the starting physical address.
 #
 # @length: #optional if specified, the memory size, in bytes. If you don't
@@ -2131,8 +2134,9 @@
 # Since: 1.2
 ##
 { 'command': 'dump-guest-memory',
-  'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
-            '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
+  'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
+            '*begin': 'int', '*length': 'int',
+            '*format': 'DumpGuestMemoryFormat'} }
 
 ##
 # @DumpGuestMemoryCapability:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d8b42f..6b51585 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -840,8 +840,8 @@ EQMP
 
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:b,protocol:s,begin:i?,end:i?,format:s?",
-        .params     = "-p protocol [begin] [length] [format]",
+        .args_type  = "paging:b,protocol:s,detach:b?,begin:i?,end:i?,format:s?",
+        .params     = "-p protocol [-d] [begin] [length] [format]",
         .help       = "dump guest memory to file",
         .mhandler.cmd_new = qmp_marshal_dump_guest_memory,
     },
@@ -857,6 +857,8 @@ Arguments:
 - "paging": do paging to get guest's memory mapping (json-bool)
 - "protocol": destination file(started with "file:") or destination file
               descriptor (started with "fd:") (json-string)
+- "detach": if specified, command will return immediately, without waiting
+            for the dump to finish (json-bool)
 - "begin": the starting physical address. It's optional, and should be specified
            with length together (json-int)
 - "length": the memory size, in bytes. It's optional, and should be specified
-- 
2.4.3

[Qemu-devel] [PATCH v6 03/11] dump-guest-memory: using static DumpState, add DumpStatus

[Peter Xu]

Instead of malloc/free each time for DumpState, make it
static. Added DumpStatus to show status for dump.

This is to be used for detached dump.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c                | 21 ++++++++++++++++-----
 include/sysemu/dump.h |  2 ++
 qapi-schema.json      | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/dump.c b/dump.c
index d79e0ed..b48cec2 100644
--- a/dump.c
+++ b/dump.c
@@ -1418,6 +1418,14 @@ static void get_max_mapnr(DumpState *s)
     s->max_mapnr = paddr_to_pfn(last_block->target_end);
 }
 
+static DumpState dump_state_global = { .status = DUMP_STATUS_NONE };
+
+static void dump_state_prepare(DumpState *s)
+{
+    /* zero the struct, setting status to active */
+    *s = (DumpState) { .status = DUMP_STATUS_ACTIVE };
+}
+
 static void dump_init(DumpState *s, int fd, bool has_format,
                       DumpGuestMemoryFormat format, bool paging, bool has_filter,
                       int64_t begin, int64_t length, Error **errp)
@@ -1647,24 +1655,27 @@ void qmp_dump_guest_memory(bool paging, const char *file,
         return;
     }
 
-    s = g_malloc0(sizeof(DumpState));
+    s = &dump_state_global;
+    dump_state_prepare(s);
 
     dump_init(s, fd, has_format, format, paging, has_begin,
               begin, length, &local_err);
     if (local_err) {
-        g_free(s);
         error_propagate(errp, local_err);
+        s->status = DUMP_STATUS_FAILED;
         return;
     }
 
     if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
-        create_kdump_vmcore(s, errp);
+        create_kdump_vmcore(s, &local_err);
     } else {
-        create_vmcore(s, errp);
+        create_vmcore(s, &local_err);
     }
 
+    s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
+    error_propagate(errp, local_err);
+
     dump_cleanup(s);
-    g_free(s);
 }
 
 DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 7e4ec5c..affef38 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -45,6 +45,7 @@
 
 #include "sysemu/dump-arch.h"
 #include "sysemu/memory_mapping.h"
+#include "qapi-types.h"
 
 typedef struct QEMU_PACKED MakedumpfileHeader {
     char signature[16];     /* = "makedumpfile" */
@@ -183,6 +184,7 @@ typedef struct DumpState {
     off_t offset_page;          /* offset of page part in vmcore */
     size_t num_dumpable;        /* number of page that can be dumped */
     uint32_t flag_compress;     /* indicate the compression format */
+    DumpStatus status;          /* current dump status */
 } DumpState;
 
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
diff --git a/qapi-schema.json b/qapi-schema.json
index 97c3ac4..691a130 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2139,6 +2139,24 @@
             '*format': 'DumpGuestMemoryFormat'} }
 
 ##
+# @DumpStatus
+#
+# Describe the status of a long-running background guest memory dump.
+#
+# @none: no dump-guest-memory has started yet.
+#
+# @active: there is one dump running in background.
+#
+# @completed: the last dump has finished successfully.
+#
+# @failed: the last dump has failed.
+#
+# Since 2.6
+##
+{ 'enum': 'DumpStatus',
+  'data': [ 'none', 'active', 'completed', 'failed' ] }
+
+##
 # @DumpGuestMemoryCapability:
 #
 # A list of the available formats for dump-guest-memory
-- 
2.4.3

[Qemu-devel] [PATCH v6 04/11] dump-guest-memory: add dump_in_progress() helper function

[Peter Xu]

For now, it has no effect. It will be used in dump detach support.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c                | 13 +++++++++++++
 include/qemu-common.h |  4 ++++
 qmp.c                 | 14 ++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/dump.c b/dump.c
index b48cec2..ccd56c8 100644
--- a/dump.c
+++ b/dump.c
@@ -1426,6 +1426,12 @@ static void dump_state_prepare(DumpState *s)
     *s = (DumpState) { .status = DUMP_STATUS_ACTIVE };
 }
 
+bool dump_in_progress(void)
+{
+    DumpState *state = &dump_state_global;
+    return (state->status == DUMP_STATUS_ACTIVE);
+}
+
 static void dump_init(DumpState *s, int fd, bool has_format,
                       DumpGuestMemoryFormat format, bool paging, bool has_filter,
                       int64_t begin, int64_t length, Error **errp)
@@ -1599,6 +1605,13 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     DumpState *s;
     Error *local_err = NULL;
 
+    /* if there is a dump in background, we should wait until the dump
+     * finished */
+    if (dump_in_progress()) {
+        error_setg(errp, "There is a dump in process, please wait.");
+        return;
+    }
+
     /*
      * kdump-compressed format need the whole memory dumped, so paging or
      * filter is not supported here.
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 405364f..7b74961 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -501,4 +501,8 @@ int parse_debug_env(const char *name, int max, int initial);
 const char *qemu_ether_ntoa(const MACAddr *mac);
 void page_size_init(void);
 
+/* returns non-zero if dump is in progress, otherwise zero is
+ * returned. */
+bool dump_in_progress(void);
+
 #endif
diff --git a/qmp.c b/qmp.c
index 0a1fa19..055586d 100644
--- a/qmp.c
+++ b/qmp.c
@@ -102,6 +102,13 @@ void qmp_quit(Error **errp)
 
 void qmp_stop(Error **errp)
 {
+    /* if there is a dump in background, we should wait until the dump
+     * finished */
+    if (dump_in_progress()) {
+        error_setg(errp, "There is a dump in process, please wait.");
+        return;
+    }
+
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         autostart = 0;
     } else {
@@ -174,6 +181,13 @@ void qmp_cont(Error **errp)
     BlockBackend *blk;
     BlockDriverState *bs;
 
+    /* if there is a dump in background, we should wait until the dump
+     * finished */
+    if (dump_in_progress()) {
+        error_setg(errp, "There is a dump in process, please wait.");
+        return;
+    }
+
     if (runstate_needs_reset()) {
         error_setg(errp, "Resetting the Virtual Machine is required");
         return;
-- 
2.4.3

[Qemu-devel] [PATCH v6 05/11] dump-guest-memory: introduce dump_process() helper function.

[Peter Xu]

No functional change. Cleanup only.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c                | 31 +++++++++++++++++++++----------
 include/sysemu/dump.h |  3 +++
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/dump.c b/dump.c
index ccd56c8..f0ee9a8 100644
--- a/dump.c
+++ b/dump.c
@@ -1441,6 +1441,9 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     Error *err = NULL;
     int ret;
 
+    s->has_format = has_format;
+    s->format = format;
+
     /* kdump-compressed is conflict with paging and filter */
     if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
         assert(!paging && !has_filter);
@@ -1594,6 +1597,23 @@ cleanup:
     dump_cleanup(s);
 }
 
+/* this operation might be time consuming. */
+static void dump_process(DumpState *s, Error **errp)
+{
+    Error *local_err = NULL;
+
+    if (s->has_format && s->format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
+        create_kdump_vmcore(s, &local_err);
+    } else {
+        create_vmcore(s, &local_err);
+    }
+
+    s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
+    error_propagate(errp, local_err);
+
+    dump_cleanup(s);
+}
+
 void qmp_dump_guest_memory(bool paging, const char *file,
                            bool has_detach, bool detach,
                            bool has_begin, int64_t begin, bool has_length,
@@ -1679,16 +1699,7 @@ void qmp_dump_guest_memory(bool paging, const char *file,
         return;
     }
 
-    if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
-        create_kdump_vmcore(s, &local_err);
-    } else {
-        create_vmcore(s, &local_err);
-    }
-
-    s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
-    error_propagate(errp, local_err);
-
-    dump_cleanup(s);
+    dump_process(s, errp);
 }
 
 DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index affef38..d6f4a9c 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -185,6 +185,9 @@ typedef struct DumpState {
     size_t num_dumpable;        /* number of page that can be dumped */
     uint32_t flag_compress;     /* indicate the compression format */
     DumpStatus status;          /* current dump status */
+
+    bool has_format;              /* whether format is provided */
+    DumpGuestMemoryFormat format; /* valid only if has_format == true */
 } DumpState;
 
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
-- 
2.4.3

[Qemu-devel] [PATCH v6 06/11] dump-guest-memory: disable dump when in INMIGRATE state

[Peter Xu]

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/dump.c b/dump.c
index f0ee9a8..aa9d1f8 100644
--- a/dump.c
+++ b/dump.c
@@ -1625,6 +1625,11 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     DumpState *s;
     Error *local_err = NULL;
 
+    if (runstate_check(RUN_STATE_INMIGRATE)) {
+        error_setg(errp, "Dump not allowed during incoming migration.");
+        return;
+    }
+
     /* if there is a dump in background, we should wait until the dump
      * finished */
     if (dump_in_progress()) {
-- 
2.4.3

[Qemu-devel] [PATCH v6 07/11] dump-guest-memory: add "detach" support

[Peter Xu]

If "detach" is provided, one thread is created to do the dump work,
while main thread will return immediately. For each GuestPhysBlock,
adding one more field "mr" to points to MemoryRegion that it
belongs, also ref the mr before use.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c                          | 27 ++++++++++++++++++++++++++-
 include/sysemu/dump.h           |  1 +
 include/sysemu/memory_mapping.h |  4 ++++
 memory_mapping.c                |  3 +++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/dump.c b/dump.c
index aa9d1f8..630c67c 100644
--- a/dump.c
+++ b/dump.c
@@ -1614,6 +1614,20 @@ static void dump_process(DumpState *s, Error **errp)
     dump_cleanup(s);
 }
 
+static void *dump_thread(void *data)
+{
+    Error *err = NULL;
+    DumpState *s = (DumpState *)data;
+
+    dump_process(s, &err);
+
+    if (err) {
+        /* TODO: notify user the error */
+        error_free(err);
+    }
+    return NULL;
+}
+
 void qmp_dump_guest_memory(bool paging, const char *file,
                            bool has_detach, bool detach,
                            bool has_begin, int64_t begin, bool has_length,
@@ -1624,6 +1638,7 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     int fd = -1;
     DumpState *s;
     Error *local_err = NULL;
+    bool detach_p = false;
 
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         error_setg(errp, "Dump not allowed during incoming migration.");
@@ -1655,6 +1670,9 @@ void qmp_dump_guest_memory(bool paging, const char *file,
         error_setg(errp, QERR_MISSING_PARAMETER, "begin");
         return;
     }
+    if (has_detach) {
+        detach_p = detach;
+    }
 
     /* check whether lzo/snappy is supported */
 #ifndef CONFIG_LZO
@@ -1704,7 +1722,14 @@ void qmp_dump_guest_memory(bool paging, const char *file,
         return;
     }
 
-    dump_process(s, errp);
+    if (detach_p) {
+        /* detached dump */
+        qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
+                           s, QEMU_THREAD_DETACHED);
+    } else {
+        /* sync dump */
+        dump_process(s, errp);
+    }
 }
 
 DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index d6f4a9c..31930c6 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -188,6 +188,7 @@ typedef struct DumpState {
 
     bool has_format;              /* whether format is provided */
     DumpGuestMemoryFormat format; /* valid only if has_format == true */
+    QemuThread dump_thread;       /* thread for detached dump */
 } DumpState;
 
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index a75d59a..d46d879 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -16,6 +16,7 @@
 
 #include "qemu/queue.h"
 #include "qemu/typedefs.h"
+#include "exec/memory.h"
 
 typedef struct GuestPhysBlock {
     /* visible to guest, reflects PCI hole, etc */
@@ -27,6 +28,9 @@ typedef struct GuestPhysBlock {
     /* points into host memory */
     uint8_t *host_addr;
 
+    /* points to the MemoryRegion that this block belongs to */
+    MemoryRegion *mr;
+
     QTAILQ_ENTRY(GuestPhysBlock) next;
 } GuestPhysBlock;
 
diff --git a/memory_mapping.c b/memory_mapping.c
index 36d6b26..f4f0622 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -177,6 +177,7 @@ void guest_phys_blocks_free(GuestPhysBlockList *list)
 
     QTAILQ_FOREACH_SAFE(p, &list->head, next, q) {
         QTAILQ_REMOVE(&list->head, p, next);
+        memory_region_unref(p->mr);
         g_free(p);
     }
     list->num = 0;
@@ -240,6 +241,8 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
         block->target_start = target_start;
         block->target_end   = target_end;
         block->host_addr    = host_addr;
+        block->mr           = section->mr;
+        memory_region_ref(section->mr);
 
         QTAILQ_INSERT_TAIL(&g->list->head, block, next);
         ++g->list->num;
-- 
2.4.3

[Qemu-devel] [PATCH v6 08/11] DumpState: adding total_size and written_size fields

[Peter Xu]

Here, total_size is the size in bytes to be dumped (raw data, which
means before compression), while written_size are bytes handled (raw
size too).

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 dump.c                | 32 ++++++++++++++++++++++++++++++++
 include/sysemu/dump.h |  9 +++++++++
 2 files changed, 41 insertions(+)

diff --git a/dump.c b/dump.c
index 630c67c..334b256 100644
--- a/dump.c
+++ b/dump.c
@@ -331,6 +331,8 @@ static void write_data(DumpState *s, void *buf, int length, Error **errp)
     ret = fd_write_vmcore(buf, length, s);
     if (ret < 0) {
         error_setg(errp, "dump: failed to save memory");
+    } else {
+        s->written_size += length;
     }
 }
 
@@ -1300,6 +1302,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
                 goto out;
             }
         }
+        s->written_size += TARGET_PAGE_SIZE;
     }
 
     ret = write_cache(&page_desc, NULL, 0, true);
@@ -1432,6 +1435,30 @@ bool dump_in_progress(void)
     return (state->status == DUMP_STATUS_ACTIVE);
 }
 
+/* calculate total size of memory to be dumped (taking filter into
+ * acoount.) */
+static int64_t dump_calculate_size(DumpState *s)
+{
+    GuestPhysBlock *block;
+    int64_t size = 0, total = 0, left = 0, right = 0;
+
+    QTAILQ_FOREACH(block, &s->guest_phys_blocks.head, next) {
+        if (s->has_filter) {
+            /* calculate the overlapped region. */
+            left = MAX(s->begin, block->target_start);
+            right = MIN(s->begin + s->length, block->target_end);
+            size = right - left;
+            size = size > 0 ? size : 0;
+        } else {
+            /* count the whole region in */
+            size = (block->target_end - block->target_start);
+        }
+        total += size;
+    }
+
+    return total;
+}
+
 static void dump_init(DumpState *s, int fd, bool has_format,
                       DumpGuestMemoryFormat format, bool paging, bool has_filter,
                       int64_t begin, int64_t length, Error **errp)
@@ -1443,6 +1470,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
 
     s->has_format = has_format;
     s->format = format;
+    s->written_size = 0;
 
     /* kdump-compressed is conflict with paging and filter */
     if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
@@ -1474,6 +1502,10 @@ static void dump_init(DumpState *s, int fd, bool has_format,
 
     guest_phys_blocks_init(&s->guest_phys_blocks);
     guest_phys_blocks_append(&s->guest_phys_blocks);
+    s->total_size = dump_calculate_size(s);
+#ifdef DEBUG_DUMP_GUEST_MEMORY
+    fprintf(stderr, "DUMP: total memory to dump: %lu\n", s->total_size);
+#endif
 
     s->start = get_start_block(s);
     if (s->start == -1) {
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 31930c6..ce262fa 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -189,6 +189,15 @@ typedef struct DumpState {
     bool has_format;              /* whether format is provided */
     DumpGuestMemoryFormat format; /* valid only if has_format == true */
     QemuThread dump_thread;       /* thread for detached dump */
+
+    int64_t total_size;          /* total memory size (in bytes) to
+                                  * be dumped. When filter is
+                                  * enabled, this will only count
+                                  * those to be written. */
+    int64_t written_size;        /* written memory size (in bytes),
+                                  * this could be used to calculate
+                                  * how much work we have
+                                  * finished. */
 } DumpState;
 
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
-- 
2.4.3

[Qemu-devel] [PATCH v6 09/11] Dump: add qmp command "query-dump"

[Peter Xu]

When dump-guest-memory is requested with detach flag, after its
return, user could query its status using "query-dump" command (with
no argument). The result contains:

- status: current dump status
- completed: bytes written in the latest dump
- total: bytes to write in the latest dump

>From completed and total, we could know how much work
finished by calculating:

  100.0 * completed / total (%)

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 dump.c           | 22 ++++++++++++++++++----
 qapi-schema.json | 32 +++++++++++++++++++++++++++++++-
 qmp-commands.hx  | 27 ++++++++++++++++++++++++++-
 3 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/dump.c b/dump.c
index 334b256..80afa1e 100644
--- a/dump.c
+++ b/dump.c
@@ -1432,7 +1432,7 @@ static void dump_state_prepare(DumpState *s)
 bool dump_in_progress(void)
 {
     DumpState *state = &dump_state_global;
-    return (state->status == DUMP_STATUS_ACTIVE);
+    return (atomic_read(&state->status) == DUMP_STATUS_ACTIVE);
 }
 
 /* calculate total size of memory to be dumped (taking filter into
@@ -1640,8 +1640,10 @@ static void dump_process(DumpState *s, Error **errp)
         create_vmcore(s, &local_err);
     }
 
-    s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
-    error_propagate(errp, local_err);
+    /* make sure status is written after written_size updates */
+    smp_wmb();
+    atomic_set(&s->status,
+               (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));
 
     dump_cleanup(s);
 }
@@ -1660,6 +1662,18 @@ static void *dump_thread(void *data)
     return NULL;
 }
 
+DumpQueryResult *qmp_query_dump(Error **errp)
+{
+    DumpQueryResult *result = g_new(DumpQueryResult, 1);
+    DumpState *state = &dump_state_global;
+    result->status = atomic_read(&state->status);
+    /* make sure we are reading status and written_size in order */
+    smp_rmb();
+    result->completed = state->written_size;
+    result->total = state->total_size;
+    return result;
+}
+
 void qmp_dump_guest_memory(bool paging, const char *file,
                            bool has_detach, bool detach,
                            bool has_begin, int64_t begin, bool has_length,
@@ -1750,7 +1764,7 @@ void qmp_dump_guest_memory(bool paging, const char *file,
               begin, length, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        s->status = DUMP_STATUS_FAILED;
+        atomic_set(&s->status, DUMP_STATUS_FAILED);
         return;
     }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 691a130..11447a5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2116,7 +2116,8 @@
 #               is the fd's name.
 #
 # @detach: #optional if true, QMP will return immediately rather than
-#          waiting for the dump to finish. (since 2.6).
+#          waiting for the dump to finish. The user can track progress
+#          using "query-dump". (since 2.6).
 #
 # @begin: #optional if specified, the starting physical address.
 #
@@ -2157,6 +2158,35 @@
   'data': [ 'none', 'active', 'completed', 'failed' ] }
 
 ##
+# @DumpQueryResult
+#
+# The result format for 'query-dump'.
+#
+# @status: enum of @DumpStatus, which shows current dump status
+#
+# @completed: bytes written in latest dump (uncompressed)
+#
+# @total: total bytes to be written in latest dump (uncompressed)
+#
+# Since 2.6
+##
+{ 'struct': 'DumpQueryResult',
+  'data': { 'status': 'DumpStatus',
+            'completed': 'int',
+            'total': 'int' } }
+
+##
+# @query-dump
+#
+# Query latest dump status.
+#
+# Returns: A @DumpStatus object showing the dump status.
+#
+# Since: 2.6
+##
+{ 'command': 'query-dump', 'returns': 'DumpQueryResult' }
+
+##
 # @DumpGuestMemoryCapability:
 #
 # A list of the available formats for dump-guest-memory
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6b51585..a740ea5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -858,7 +858,8 @@ Arguments:
 - "protocol": destination file(started with "file:") or destination file
               descriptor (started with "fd:") (json-string)
 - "detach": if specified, command will return immediately, without waiting
-            for the dump to finish (json-bool)
+            for the dump to finish. The user can track progress using
+            "query-dump". (json-bool)
 - "begin": the starting physical address. It's optional, and should be specified
            with length together (json-int)
 - "length": the memory size, in bytes. It's optional, and should be specified
@@ -898,6 +899,30 @@ Example:
 
 EQMP
 
+    {
+        .name       = "query-dump",
+        .args_type  = "",
+        .params     = "",
+        .help       = "query background dump status",
+        .mhandler.cmd_new = qmp_marshal_query_dump,
+    },
+
+SQMP
+query-dump
+----------
+
+Query background dump status.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "query-dump" }
+<- { "return": { "status": "active", "completed": 1024000,
+                 "total": 2048000 } }
+
+EQMP
+
 #if defined TARGET_S390X
     {
         .name       = "dump-skeys",
-- 
2.4.3

[Qemu-devel] [PATCH v6 10/11] Dump: add hmp command "info dump"

[Peter Xu]

It will calculate percentage of finished work from completed and
total.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hmp-commands-info.hx | 14 ++++++++++++++
 hmp.c                | 17 +++++++++++++++++
 hmp.h                |  1 +
 3 files changed, 32 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 9b71351..52539c3 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -786,6 +786,20 @@ STEXI
 Display the value of a storage key (s390 only)
 ETEXI
 
+    {
+        .name       = "dump",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Display the latest dump status",
+        .mhandler.cmd = hmp_info_dump,
+    },
+
+STEXI
+@item info dump
+@findex dump
+Display the latest dump status.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 1f4d0b6..c824064 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2383,3 +2383,20 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
 
     qapi_free_RockerOfDpaGroupList(list);
 }
+
+void hmp_info_dump(Monitor *mon, const QDict *qdict)
+{
+    DumpQueryResult *result = qmp_query_dump(NULL);
+
+    assert(result->status < DUMP_STATUS_MAX);
+    monitor_printf(mon, "Status: %s\n", DumpStatus_lookup[result->status]);
+
+    if (result->status == DUMP_STATUS_ACTIVE) {
+        float percent = 0;
+        assert(result->total != 0);
+        percent = 100.0 * result->completed / result->total;
+        monitor_printf(mon, "Finished: %.2f %%\n", percent);
+    }
+
+    qapi_free_DumpQueryResult(result);
+}
diff --git a/hmp.h b/hmp.h
index a8c5b5a..093d65f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -131,5 +131,6 @@ void hmp_rocker(Monitor *mon, const QDict *qdict);
 void hmp_rocker_ports(Monitor *mon, const QDict *qdict);
 void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
 void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
+void hmp_info_dump(Monitor *mon, const QDict *qdict);
 
 #endif
-- 
2.4.3

[Qemu-devel] [PATCH v6 11/11] dump-guest-memory: add qmp event DUMP_COMPLETED

[Peter Xu]

One new QMP event DUMP_COMPLETED is added. When a dump finishes, one
DUMP_COMPLETED event will occur to notify the user.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 docs/qmp-events.txt | 18 ++++++++++++++++++
 dump.c              | 19 +++++++++++++------
 qapi/event.json     | 16 ++++++++++++++++
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index d2f1ce4..3a03004 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -220,6 +220,24 @@ Data:
   },
   "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
 
+DUMP_COMPLETED
+--------------
+
+Emitted when the guest has finished one memory dump.
+
+Data:
+
+- "result": DumpQueryResult type described in qapi-schema.json
+- "error": Error message when dump failed. This is only a
+  human-readable string provided when dump failed. It should not be
+  parsed in any way (json-string, optional)
+
+Example:
+
+{ "event": "DUMP_COMPLETED",
+  "data": {"result": {"total": 1090650112, "status": "completed",
+                      "completed": 1090650112} } }
+
 GUEST_PANICKED
 --------------
 
diff --git a/dump.c b/dump.c
index 80afa1e..30be595 100644
--- a/dump.c
+++ b/dump.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qmp-commands.h"
+#include "qapi-event.h"
 
 #include <zlib.h>
 #ifdef CONFIG_LZO
@@ -1633,6 +1634,7 @@ cleanup:
 static void dump_process(DumpState *s, Error **errp)
 {
     Error *local_err = NULL;
+    DumpQueryResult *result = NULL;
 
     if (s->has_format && s->format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
         create_kdump_vmcore(s, &local_err);
@@ -1645,6 +1647,16 @@ static void dump_process(DumpState *s, Error **errp)
     atomic_set(&s->status,
                (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));
 
+    /* send DUMP_COMPLETED message (unconditionally) */
+    result = qmp_query_dump(NULL);
+    /* should never fail */
+    assert(result);
+    qapi_event_send_dump_completed(result, !!local_err, (local_err ? \
+                                   error_get_pretty(local_err) : NULL),
+                                   &error_abort);
+    qapi_free_DumpQueryResult(result);
+
+    error_propagate(errp, local_err);
     dump_cleanup(s);
 }
 
@@ -1652,13 +1664,8 @@ static void *dump_thread(void *data)
 {
     Error *err = NULL;
     DumpState *s = (DumpState *)data;
-
     dump_process(s, &err);
-
-    if (err) {
-        /* TODO: notify user the error */
-        error_free(err);
-    }
+    error_free(err);
     return NULL;
 }
 
diff --git a/qapi/event.json b/qapi/event.json
index f0cef01..bf7dd61 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -356,3 +356,19 @@
 ##
 { 'event': 'MEM_UNPLUG_ERROR',
   'data': { 'device': 'str', 'msg': 'str' } }
+
+##
+# @DUMP_COMPLETED
+#
+# Emitted when background dump has completed
+#
+# @result: DumpQueryResult type described in qapi-schema.json.
+#
+# @error: #optional human-readable error string that provides
+#         hint on why dump failed. Only presents on failure. The
+#         user should not try to interpret the error string.
+#
+# Since: 2.6
+##
+{ 'event': 'DUMP_COMPLETED' ,
+  'data': { 'result': 'DumpQueryResult', '*error': 'str' } }
-- 
2.4.3

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

[Peter Xu]

Still think this might help in some cases, so ping again...

Peter

On Wed, Dec 09, 2015 at 10:42:12AM +0800, Peter Xu wrote:
> v6 changes:
> - patch 10
>   - English error fix [Fam]
> - patch 11
>   - remove useless var: "not_used" [me]
> - all
>   - move patch 8 to the end to be patch 11 (v5 patches 9-11 become
>   v6 patches 8-10) [Eric]
> 
> 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)
> 
> 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
>   DumpState: adding total_size and written_size fields
>   Dump: add qmp command "query-dump"
>   Dump: add hmp command "info dump"
>   dump-guest-memory: add qmp event DUMP_COMPLETED
> 
>  docs/qmp-events.txt             |  18 ++++
>  dump.c                          | 215 ++++++++++++++++++++++++++++++----------
>  hmp-commands-info.hx            |  14 +++
>  hmp-commands.hx                 |   5 +-
>  hmp.c                           |  26 ++++-
>  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                |  56 ++++++++++-
>  qapi/event.json                 |  16 +++
>  qmp-commands.hx                 |  31 +++++-
>  qmp.c                           |  14 +++
>  14 files changed, 359 insertions(+), 63 deletions(-)
> 
> -- 
> 2.4.3
> 

Re: [Qemu-devel] [PATCH v6 09/11] Dump: add qmp command "query-dump"

[Fam Zheng]

On Wed, 12/09 10:42, Peter Xu wrote:
> -    s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
> -    error_propagate(errp, local_err);

This means the error is released by caller, ...

> +    /* make sure status is written after written_size updates */
> +    smp_wmb();
> +    atomic_set(&s->status,
> +               (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));

but it is removed now.  Is local_err leaked now?

Fam

Re: [Qemu-devel] [PATCH v6 10/11] Dump: add hmp command "info dump"

[Fam Zheng]

On Wed, 12/09 10:42, Peter Xu wrote:
> It will calculate percentage of finished work from completed and
> total.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hmp-commands-info.hx | 14 ++++++++++++++
>  hmp.c                | 17 +++++++++++++++++
>  hmp.h                |  1 +
>  3 files changed, 32 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 9b71351..52539c3 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -786,6 +786,20 @@ STEXI
>  Display the value of a storage key (s390 only)
>  ETEXI
>  
> +    {
> +        .name       = "dump",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "Display the latest dump status",
> +        .mhandler.cmd = hmp_info_dump,
> +    },
> +
> +STEXI
> +@item info dump
> +@findex dump
> +Display the latest dump status.
> +ETEXI
> +
>  STEXI
>  @end table
>  ETEXI
> diff --git a/hmp.c b/hmp.c
> index 1f4d0b6..c824064 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2383,3 +2383,20 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
>  
>      qapi_free_RockerOfDpaGroupList(list);
>  }
> +
> +void hmp_info_dump(Monitor *mon, const QDict *qdict)
> +{
> +    DumpQueryResult *result = qmp_query_dump(NULL);

If something goes wrong, it is probably better to pass in local_err to
qmp_query_dump and print the error info instead of assuming result's fields are
all valid and trying to interpret them.

Fam

> +
> +    assert(result->status < DUMP_STATUS_MAX);
> +    monitor_printf(mon, "Status: %s\n", DumpStatus_lookup[result->status]);
> +
> +    if (result->status == DUMP_STATUS_ACTIVE) {
> +        float percent = 0;
> +        assert(result->total != 0);
> +        percent = 100.0 * result->completed / result->total;
> +        monitor_printf(mon, "Finished: %.2f %%\n", percent);
> +    }
> +
> +    qapi_free_DumpQueryResult(result);
> +}
> diff --git a/hmp.h b/hmp.h
> index a8c5b5a..093d65f 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -131,5 +131,6 @@ void hmp_rocker(Monitor *mon, const QDict *qdict);
>  void hmp_rocker_ports(Monitor *mon, const QDict *qdict);
>  void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
>  void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
> +void hmp_info_dump(Monitor *mon, const QDict *qdict);
>  
>  #endif
> -- 
> 2.4.3
> 

Re: [Qemu-devel] [PATCH v6 11/11] dump-guest-memory: add qmp event DUMP_COMPLETED

[Fam Zheng]

On Wed, 12/09 10:42, Peter Xu wrote:
> One new QMP event DUMP_COMPLETED is added. When a dump finishes, one
> DUMP_COMPLETED event will occur to notify the user.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  docs/qmp-events.txt | 18 ++++++++++++++++++
>  dump.c              | 19 +++++++++++++------
>  qapi/event.json     | 16 ++++++++++++++++
>  3 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
> index d2f1ce4..3a03004 100644
> --- a/docs/qmp-events.txt
> +++ b/docs/qmp-events.txt
> @@ -220,6 +220,24 @@ Data:
>    },
>    "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>  
> +DUMP_COMPLETED
> +--------------
> +
> +Emitted when the guest has finished one memory dump.
> +
> +Data:
> +
> +- "result": DumpQueryResult type described in qapi-schema.json
> +- "error": Error message when dump failed. This is only a
> +  human-readable string provided when dump failed. It should not be
> +  parsed in any way (json-string, optional)
> +
> +Example:
> +
> +{ "event": "DUMP_COMPLETED",
> +  "data": {"result": {"total": 1090650112, "status": "completed",
> +                      "completed": 1090650112} } }
> +
>  GUEST_PANICKED
>  --------------
>  
> diff --git a/dump.c b/dump.c
> index 80afa1e..30be595 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -25,6 +25,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qmp-commands.h"
> +#include "qapi-event.h"
>  
>  #include <zlib.h>
>  #ifdef CONFIG_LZO
> @@ -1633,6 +1634,7 @@ cleanup:
>  static void dump_process(DumpState *s, Error **errp)
>  {
>      Error *local_err = NULL;
> +    DumpQueryResult *result = NULL;
>  
>      if (s->has_format && s->format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
>          create_kdump_vmcore(s, &local_err);
> @@ -1645,6 +1647,16 @@ static void dump_process(DumpState *s, Error **errp)
>      atomic_set(&s->status,
>                 (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));
>  
> +    /* send DUMP_COMPLETED message (unconditionally) */
> +    result = qmp_query_dump(NULL);
> +    /* should never fail */
> +    assert(result);
> +    qapi_event_send_dump_completed(result, !!local_err, (local_err ? \
> +                                   error_get_pretty(local_err) : NULL),
> +                                   &error_abort);
> +    qapi_free_DumpQueryResult(result);
> +
> +    error_propagate(errp, local_err);
>      dump_cleanup(s);
>  }
>  
> @@ -1652,13 +1664,8 @@ static void *dump_thread(void *data)
>  {
>      Error *err = NULL;
>      DumpState *s = (DumpState *)data;
> -
>      dump_process(s, &err);
> -
> -    if (err) {
> -        /* TODO: notify user the error */
> -        error_free(err);
> -    }
> +    error_free(err);
>      return NULL;
>  }
>  
> diff --git a/qapi/event.json b/qapi/event.json
> index f0cef01..bf7dd61 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -356,3 +356,19 @@
>  ##
>  { 'event': 'MEM_UNPLUG_ERROR',
>    'data': { 'device': 'str', 'msg': 'str' } }
> +
> +##
> +# @DUMP_COMPLETED
> +#
> +# Emitted when background dump has completed
> +#
> +# @result: DumpQueryResult type described in qapi-schema.json.
> +#
> +# @error: #optional human-readable error string that provides
> +#         hint on why dump failed. Only presents on failure. The
> +#         user should not try to interpret the error string.
> +#
> +# Since: 2.6
> +##
> +{ 'event': 'DUMP_COMPLETED' ,
> +  'data': { 'result': 'DumpQueryResult', '*error': 'str' } }
> -- 
> 2.4.3
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v6 09/11] Dump: add qmp command "query-dump"

[Peter Xu]

On Tue, Feb 16, 2016 at 02:40:59PM +0800, Fam Zheng wrote:
> On Wed, 12/09 10:42, Peter Xu wrote:
> > -    s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
> > -    error_propagate(errp, local_err);
> 
> This means the error is released by caller, ...
> 
> > +    /* make sure status is written after written_size updates */
> > +    smp_wmb();
> > +    atomic_set(&s->status,
> > +               (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));
> 
> but it is removed now.  Is local_err leaked now?

Right. Lost it during switching of patches and merges. Will
fix. Thanks!

Peter

Re: [Qemu-devel] [PATCH v6 10/11] Dump: add hmp command "info dump"

[Peter Xu]

On Tue, Feb 16, 2016 at 02:46:15PM +0800, Fam Zheng wrote:
> On Wed, 12/09 10:42, Peter Xu wrote:
> > +
> > +void hmp_info_dump(Monitor *mon, const QDict *qdict)
> > +{
> > +    DumpQueryResult *result = qmp_query_dump(NULL);
> 
> If something goes wrong, it is probably better to pass in local_err to
> qmp_query_dump and print the error info instead of assuming result's fields are
> all valid and trying to interpret them.
> 
> Fam

This is because qmp_query_dump() will never fail, and it's not using
errp. Will assert "result" too before use.

Thanks.
Peter