All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory
@ 2016-02-16  7:50 Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}() Peter Xu
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

Changes from v6:
- patch 9: fix leak of local_err due to patch switch.. [Fam]
- patch 10: assert "result" before use [Fam]
- patch 11: add Fam's reviewed-by.

For older patch, please refers to v6 series:

https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01299.html

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}().
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 02/11] dump-guest-memory: add "detach" flag for QMP/HMP interfaces Peter Xu
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 96e1fc1..769c5f9 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;
     }
 
@@ -1087,7 +1074,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;
         }
 
@@ -1104,7 +1091,7 @@ static void write_dump_bitmap(DumpState *s, Error **errp)
         ret = set_dump_bitmap(last_pfn, last_pfn + bits_per_buf, 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;
         }
     }
@@ -1237,7 +1224,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
     ret = write_cache(&page_data, buf, s->dump_info.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;
     }
 
@@ -1253,7 +1240,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 {
@@ -1278,7 +1265,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
@@ -1291,7 +1278,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
@@ -1305,7 +1292,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
@@ -1321,7 +1308,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
                 ret = write_cache(&page_data, buf,
                                   s->dump_info.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;
                 }
             }
@@ -1333,7 +1320,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;
             }
         }
@@ -1341,12 +1328,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;
     }
 
@@ -1390,7 +1377,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;
     }
 
@@ -1414,11 +1401,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)
@@ -1706,6 +1691,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 02/11] dump-guest-memory: add "detach" flag for QMP/HMP interfaces.
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}() Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus Peter Xu
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 769c5f9..c4a62d9 100644
--- a/dump.c
+++ b/dump.c
@@ -1609,8 +1609,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 c6419da..3a0d9d4 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 8d04897..caff580 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2195,6 +2195,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
@@ -2211,8 +2214,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 020e5ee..5dc7738 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -838,8 +838,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,
     },
@@ -855,6 +855,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}() Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 02/11] dump-guest-memory: add "detach" flag for QMP/HMP interfaces Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16 12:32   ` Andrew Jones
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 04/11] dump-guest-memory: add dump_in_progress() helper function Peter Xu
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 c4a62d9..434bc60 100644
--- a/dump.c
+++ b/dump.c
@@ -1442,6 +1442,14 @@ static void get_max_mapnr(DumpState *s)
     s->max_mapnr = dump_paddr_to_pfn(s, 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)
@@ -1676,24 +1684,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 2f04b24..21fc02d 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -38,6 +38,7 @@
 
 #include "sysemu/dump-arch.h"
 #include "sysemu/memory_mapping.h"
+#include "qapi-types.h"
 
 typedef struct QEMU_PACKED MakedumpfileHeader {
     char signature[16];     /* = "makedumpfile" */
@@ -176,6 +177,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 caff580..ccd30c8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2219,6 +2219,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 04/11] dump-guest-memory: add dump_in_progress() helper function
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (2 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 05/11] dump-guest-memory: introduce dump_process() " Peter Xu
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 434bc60..158d6ea 100644
--- a/dump.c
+++ b/dump.c
@@ -1450,6 +1450,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)
@@ -1628,6 +1634,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 f557be7..59ab759 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -494,4 +494,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 6ae7230..f4caf5a 100644
--- a/qmp.c
+++ b/qmp.c
@@ -103,6 +103,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 {
@@ -175,6 +182,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 05/11] dump-guest-memory: introduce dump_process() helper function.
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (3 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 04/11] dump-guest-memory: add dump_in_progress() helper function Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 06/11] dump-guest-memory: disable dump when in INMIGRATE state Peter Xu
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 158d6ea..fed84a6 100644
--- a/dump.c
+++ b/dump.c
@@ -1465,6 +1465,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);
@@ -1623,6 +1626,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,
@@ -1708,16 +1728,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 21fc02d..1da3ddb 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -178,6 +178,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 06/11] dump-guest-memory: disable dump when in INMIGRATE state
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (4 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 05/11] dump-guest-memory: introduce dump_process() " Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 07/11] dump-guest-memory: add "detach" support Peter Xu
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 fed84a6..923e3a5 100644
--- a/dump.c
+++ b/dump.c
@@ -1654,6 +1654,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 07/11] dump-guest-memory: add "detach" support
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (5 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 06/11] dump-guest-memory: disable dump when in INMIGRATE state Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields Peter Xu
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 923e3a5..9210a72 100644
--- a/dump.c
+++ b/dump.c
@@ -1643,6 +1643,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,
@@ -1653,6 +1667,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.");
@@ -1684,6 +1699,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
@@ -1733,7 +1751,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 1da3ddb..06393c3 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -181,6 +181,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 04db3ac..c8855de 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -178,6 +178,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;
@@ -241,6 +242,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (6 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 07/11] dump-guest-memory: add "detach" support Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16 12:39   ` Andrew Jones
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump" Peter Xu
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 9210a72..ca2400d 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;
     }
 }
 
@@ -1324,6 +1326,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);
@@ -1456,6 +1459,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)
@@ -1467,6 +1494,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) {
@@ -1498,6 +1526,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 06393c3..ef931be 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -182,6 +182,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump"
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (7 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields Peter Xu
@ 2016-02-16  7:50 ` Peter Xu
  2016-02-16  8:35   ` Fam Zheng
  2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump" Peter Xu
  2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 11/11] dump-guest-memory: add qmp event DUMP_COMPLETED Peter Xu
  10 siblings, 1 reply; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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           | 23 +++++++++++++++++++----
 qapi-schema.json | 32 +++++++++++++++++++++++++++++++-
 qmp-commands.hx  | 27 ++++++++++++++++++++++++++-
 3 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/dump.c b/dump.c
index ca2400d..c697259 100644
--- a/dump.c
+++ b/dump.c
@@ -1456,7 +1456,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
@@ -1669,9 +1669,12 @@ 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));
 
+    error_propagate(errp, local_err);
     dump_cleanup(s);
 }
 
@@ -1689,6 +1692,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,
@@ -1779,7 +1794,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 ccd30c8..7b8f2a1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2196,7 +2196,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.
 #
@@ -2237,6 +2238,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 5dc7738..47351f3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -856,7 +856,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
@@ -896,6 +897,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump"
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (8 preceding siblings ...)
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump" Peter Xu
@ 2016-02-16  7:51 ` Peter Xu
  2016-02-16  8:35   ` Fam Zheng
  2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 11/11] dump-guest-memory: add qmp event DUMP_COMPLETED Peter Xu
  10 siblings, 1 reply; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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 3a0d9d4..92cf014 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2381,3 +2381,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 && 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v7 11/11] dump-guest-memory: add qmp event DUMP_COMPLETED
  2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
                   ` (9 preceding siblings ...)
  2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump" Peter Xu
@ 2016-02-16  7:51 ` Peter Xu
  10 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16  7:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: drjones, famz, armbru, peterx, lcapitulino, pbonzini, lersek

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>
Reviewed-by:   Fam Zheng <famz@redhat.com>
---
 docs/qmp-events.txt | 18 ++++++++++++++++++
 dump.c              | 18 ++++++++++++------
 qapi/event.json     | 16 ++++++++++++++++
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index 52eb7e2..4e3eb9e 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 c697259..a5c40e6 100644
--- a/dump.c
+++ b/dump.c
@@ -25,6 +25,7 @@
 #include "sysemu/cpus.h"
 #include "qapi/qmp/qerror.h"
 #include "qmp-commands.h"
+#include "qapi-event.h"
 
 #include <zlib.h>
 #ifdef CONFIG_LZO
@@ -1662,6 +1663,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);
@@ -1674,6 +1676,15 @@ 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);
 }
@@ -1682,13 +1693,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 390fd45..1a45a6c 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -369,3 +369,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump"
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump" Peter Xu
@ 2016-02-16  8:35   ` Fam Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2016-02-16  8:35 UTC (permalink / raw)
  To: Peter Xu; +Cc: drjones, armbru, qemu-devel, lcapitulino, pbonzini, lersek

On Tue, 02/16 15:50, Peter Xu wrote:
> 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           | 23 +++++++++++++++++++----
>  qapi-schema.json | 32 +++++++++++++++++++++++++++++++-
>  qmp-commands.hx  | 27 ++++++++++++++++++++++++++-
>  3 files changed, 76 insertions(+), 6 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index ca2400d..c697259 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1456,7 +1456,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
> @@ -1669,9 +1669,12 @@ 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));
>  
> +    error_propagate(errp, local_err);
>      dump_cleanup(s);
>  }
>  
> @@ -1689,6 +1692,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,
> @@ -1779,7 +1794,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 ccd30c8..7b8f2a1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2196,7 +2196,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.
>  #
> @@ -2237,6 +2238,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 5dc7738..47351f3 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -856,7 +856,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
> @@ -896,6 +897,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
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump"
  2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump" Peter Xu
@ 2016-02-16  8:35   ` Fam Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2016-02-16  8:35 UTC (permalink / raw)
  To: Peter Xu; +Cc: drjones, armbru, qemu-devel, lcapitulino, pbonzini, lersek

On Tue, 02/16 15:51, 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 3a0d9d4..92cf014 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2381,3 +2381,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 && 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
> 
Reviewed-by: Fam Zheng <famz@redhat.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus Peter Xu
@ 2016-02-16 12:32   ` Andrew Jones
  2016-02-16 13:11     ` Peter Xu
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Jones @ 2016-02-16 12:32 UTC (permalink / raw)
  To: Peter Xu; +Cc: famz, armbru, qemu-devel, lcapitulino, pbonzini, lersek

On Tue, Feb 16, 2016 at 03:50:53PM +0800, Peter Xu wrote:
> Instead of malloc/free each time for DumpState, make it
> static. Added DumpStatus to show status for dump.

I see that the motivation for making DumpState static is for
dump_in_progress(). DumpState isn't massive, but it isn't tiny
either, so maybe we should just have a global pointer instead?
dump_in_progress() can report DUMP_STATUS_NONE when the pointer
is NULL, or whatever the status is, when it's not.

drew

> 
> 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 c4a62d9..434bc60 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1442,6 +1442,14 @@ static void get_max_mapnr(DumpState *s)
>      s->max_mapnr = dump_paddr_to_pfn(s, 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)
> @@ -1676,24 +1684,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 2f04b24..21fc02d 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -38,6 +38,7 @@
>  
>  #include "sysemu/dump-arch.h"
>  #include "sysemu/memory_mapping.h"
> +#include "qapi-types.h"
>  
>  typedef struct QEMU_PACKED MakedumpfileHeader {
>      char signature[16];     /* = "makedumpfile" */
> @@ -176,6 +177,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 caff580..ccd30c8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2219,6 +2219,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
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields
  2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields Peter Xu
@ 2016-02-16 12:39   ` Andrew Jones
  2016-02-17 13:08     ` Peter Xu
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Jones @ 2016-02-16 12:39 UTC (permalink / raw)
  To: Peter Xu; +Cc: famz, armbru, qemu-devel, lcapitulino, pbonzini, lersek

On Tue, Feb 16, 2016 at 03:50:58PM +0800, Peter Xu wrote:
> 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 9210a72..ca2400d 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;
>      }
>  }
>  
> @@ -1324,6 +1326,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
>                  goto out;
>              }
>          }
> +        s->written_size += TARGET_PAGE_SIZE;

Don't use TARGET_PAGE_SIZE, use s->dump_info.page_size, see 8161befd

>      }
>  
>      ret = write_cache(&page_desc, NULL, 0, true);
> @@ -1456,6 +1459,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)
> @@ -1467,6 +1494,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) {
> @@ -1498,6 +1526,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 06393c3..ef931be 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -182,6 +182,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
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus
  2016-02-16 12:32   ` Andrew Jones
@ 2016-02-16 13:11     ` Peter Xu
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-16 13:11 UTC (permalink / raw)
  To: Andrew Jones; +Cc: famz, armbru, qemu-devel, lcapitulino, pbonzini, lersek

On Tue, Feb 16, 2016 at 01:32:00PM +0100, Andrew Jones wrote:
> On Tue, Feb 16, 2016 at 03:50:53PM +0800, Peter Xu wrote:
> > Instead of malloc/free each time for DumpState, make it
> > static. Added DumpStatus to show status for dump.
> 
> I see that the motivation for making DumpState static is for
> dump_in_progress(). DumpState isn't massive, but it isn't tiny
> either, so maybe we should just have a global pointer instead?
> dump_in_progress() can report DUMP_STATUS_NONE when the pointer
> is NULL, or whatever the status is, when it's not.

I chose to use static dump state mostly for the simplicity of
implementation.

If I use a dynamic one, I can save some memory spaces when there is
no dump task, however since it's async dump with dedicated thread, I
need at least a lock to protect the global pointer when to
use/malloc/free it, since there can be concurrent operations (e.g.,
when dump thread finished and trying to free the dump state, one
user might be querying dump status too). So I just chose not to save
the 200+ bytes.

One more thing a static dump state could bring is that, instead of
letting the user know "there is/is't a dump running", we can let him
know something more, like: "no dump started" or "latest dump
finished, with total written bytes XXX". Also, user can always check
for all the information with the last time he/she did the dump
(which I take it as a tiny small enhancement too).

Thanks.
Peter

> 
> drew
> 
> > 
> > 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 c4a62d9..434bc60 100644
> > --- a/dump.c
> > +++ b/dump.c
> > @@ -1442,6 +1442,14 @@ static void get_max_mapnr(DumpState *s)
> >      s->max_mapnr = dump_paddr_to_pfn(s, 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)
> > @@ -1676,24 +1684,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 2f04b24..21fc02d 100644
> > --- a/include/sysemu/dump.h
> > +++ b/include/sysemu/dump.h
> > @@ -38,6 +38,7 @@
> >  
> >  #include "sysemu/dump-arch.h"
> >  #include "sysemu/memory_mapping.h"
> > +#include "qapi-types.h"
> >  
> >  typedef struct QEMU_PACKED MakedumpfileHeader {
> >      char signature[16];     /* = "makedumpfile" */
> > @@ -176,6 +177,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 caff580..ccd30c8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -2219,6 +2219,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
> > 
> > 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields
  2016-02-16 12:39   ` Andrew Jones
@ 2016-02-17 13:08     ` Peter Xu
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Xu @ 2016-02-17 13:08 UTC (permalink / raw)
  To: Andrew Jones; +Cc: famz, armbru, qemu-devel, lcapitulino, pbonzini, lersek

On Tue, Feb 16, 2016 at 01:39:36PM +0100, Andrew Jones wrote:
> On Tue, Feb 16, 2016 at 03:50:58PM +0800, Peter Xu wrote:
> > 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 9210a72..ca2400d 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;
> >      }
> >  }
> >  
> > @@ -1324,6 +1326,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
> >                  goto out;
> >              }
> >          }
> > +        s->written_size += TARGET_PAGE_SIZE;
> 
> Don't use TARGET_PAGE_SIZE, use s->dump_info.page_size, see 8161befd
> 

Will fix. Thanks!

Peter

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-02-17 13:08 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16  7:50 [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}() Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 02/11] dump-guest-memory: add "detach" flag for QMP/HMP interfaces Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus Peter Xu
2016-02-16 12:32   ` Andrew Jones
2016-02-16 13:11     ` Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 04/11] dump-guest-memory: add dump_in_progress() helper function Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 05/11] dump-guest-memory: introduce dump_process() " Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 06/11] dump-guest-memory: disable dump when in INMIGRATE state Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 07/11] dump-guest-memory: add "detach" support Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields Peter Xu
2016-02-16 12:39   ` Andrew Jones
2016-02-17 13:08     ` Peter Xu
2016-02-16  7:50 ` [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump" Peter Xu
2016-02-16  8:35   ` Fam Zheng
2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump" Peter Xu
2016-02-16  8:35   ` Fam Zheng
2016-02-16  7:51 ` [Qemu-devel] [PATCH v7 11/11] dump-guest-memory: add qmp event DUMP_COMPLETED Peter Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.