Patchwork [9/9,v2] Make monitor command 'dump-guest-memory' dump in kdump-compressed format

login
register
mail settings
Submitter Qiao Nuohan
Date May 15, 2013, 2:29 a.m.
Message ID <1368584998-20053-10-git-send-email-qiaonuohan@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/243868/
State New
Headers show

Comments

Qiao Nuohan - May 15, 2013, 2:29 a.m.
Make monitor command 'dump-guest-memory' dump in kdump-compressed format.
The command's usage:
  dump [-p] protocol [begin] [length] [format]
'format' is used to specified the format of vmcore and can be:
1. 'elf': ELF format, without compression
2. 'zlib': kdump-compressed format, with zlib-compressed
3. 'lzo': kdump-compressed format, with lzo-compressed
4. 'snappy': kdump-compressed format, with snappy-compressed
And without 'format' being set, vmcore will be in ELF format.

Note:
  1. The kdump-compressed format is readable only with the crash utility, and it
     can be smaller than the ELF format because of the compression support.
  2. The kdump-compressed format is the 5th edition.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Signed-off-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 configure             |   50 +++++++++++++++++++++++
 dump.c                |  106 +++++++++++++++++++++++++++++++++++++++++++-----
 hmp-commands.hx       |   12 ++++--
 hmp.c                 |   23 ++++++++++-
 include/sysemu/dump.h |    8 ++++
 qapi-schema.json      |   23 ++++++++++-
 qmp-commands.hx       |    6 ++-
 7 files changed, 208 insertions(+), 20 deletions(-)
Eric Blake - May 15, 2013, 2:55 a.m.
On 05/14/2013 08:29 PM, Qiao Nuohan wrote:
> Make monitor command 'dump-guest-memory' dump in kdump-compressed format.
> The command's usage:
>   dump [-p] protocol [begin] [length] [format]
> 'format' is used to specified the format of vmcore and can be:
> 1. 'elf': ELF format, without compression
> 2. 'zlib': kdump-compressed format, with zlib-compressed
> 3. 'lzo': kdump-compressed format, with lzo-compressed
> 4. 'snappy': kdump-compressed format, with snappy-compressed
> And without 'format' being set, vmcore will be in ELF format.
> 
> Note:
>   1. The kdump-compressed format is readable only with the crash utility, and it
>      can be smaller than the ELF format because of the compression support.
>   2. The kdump-compressed format is the 5th edition.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Signed-off-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---

> -static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> -                     int64_t begin, int64_t length, Error **errp)
> +static int dump_init(DumpState *s, int fd, bool compress_format,
> +                    DumpGuestMemoryFormat format, bool paging,
> +                    bool has_filter, int64_t begin, int64_t length, Error **errp)
>  {

Why do you need compress_format as a separate parameter, when that
information is redundant with the contents of format?

> +void qmp_dump_guest_memory(bool paging, const char *protocol, bool has_begin,
> +                            int64_t begin, bool has_length,
> +                            int64_t length, bool has_format,
> +                            DumpGuestMemoryFormat format, Error **errp)
>  {
>      const char *p;
>      int fd = -1;
>      DumpState *s;
>      int ret;
> +    int compress_format = 0;

You are using this as a bool, so type it as a bool rather than int.

> +++ b/include/sysemu/dump.h
> @@ -37,6 +37,13 @@
>  #endif
>  
>  /*
> + * dump format
> + */
> +#define FLAG_DUMP_COMPRESS_ZLIB     (0x1)   /* compressed with zlib */
> +#define FLAG_DUMP_COMPRESS_LZO      (0x2)   /* compressed with lzo */
> +#define FLAG_DUMP_COMPRESS_SNAPPY   (0x4)   /* compressed with snappy */

Why are you skipping 3?  Besides, these aren't flags that can be
bitwise-or'd together, so the name FLAG_ is misleading.  Why not just
make it an enum, with value 0, 1, 2 (or with 1, 2, 3)?  In fact, why
even declare this at all, instead of reusing the DumpGuestMemoryFormat
enum created for you by the QMP code generation from the schema?

> +
> +/*
>   * flag used in page desc of kdump-compressed format
>   */
>  #define DUMP_DH_COMPRESSED_ZLIB     (0x1)
> @@ -46,6 +53,7 @@
>  #define KDUMP_SIGNATURE             "KDUMP   "
>  #define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
>  #define DISKDUMP_HEADER_BLOCKS      (1)
> +#define PAGE_SIZE                   (4096)

Is that true for all platforms?

> +++ b/qapi-schema.json
> @@ -2381,6 +2381,18 @@
>  { 'command': 'device_del', 'data': {'id': 'str'} }
>  
>  ##
> +# @DumpGuestMemoryFormat
> +#
> +# Format of Guest memory dump
> +#
> +# Each represents a format of guest memory dump.
> +#
> +# Since: 1.6
> +##
> +{ 'enum': 'DumpGuestMemoryFormat',
> +  'data': [ 'elf', 'zlib', 'lzo', 'snappy' ] }

For several other enums, we document what each enum value means here.
That way, if the enum gets used more than once, you won't have to repeat...

> +
> +##
>  # @dump-guest-memory
>  #
>  # Dump guest's memory to vmcore. It is a synchronous operation that can take
> @@ -2416,13 +2428,20 @@
>  #          want to dump all guest's memory, please specify the start @begin
>  #          and @length
>  #
> +# @format: #optional if specified, the format of guest memory dump.
> +#
> +#            1. elf: dump file will be in elf format
> +#            2. zlib: dump file will be in kdump format with zlib-compressed
> +#            3. lzo: dump file will be in kdump format with lzo-compressed
> +#            4. snappy: dump file will be in kdump format with snappy-compressed

...this text in each other use of the enum.  Also, if we extend the enum
in the future, you don't have to revisit quite as much documentation.

> +#
>  # Returns: nothing on success
>  #
> -# Since: 1.2
> +# Since: 1.6

Not quite right.  The dump-guest-memory command is still Since: 1.2.
Rather, it is the @format: line that should add "(since 1.6)" to show
that one additional option was added later.  Look at drive-mirror's
@buf-size for an example.

Interface is looking better, though; thanks for folding in the review
suggestions.
Qiao Nuohan - May 15, 2013, 5:27 a.m.
On 05/15/2013 10:55 AM, Eric Blake wrote:
> On 05/14/2013 08:29 PM, Qiao Nuohan wrote:
>> Make monitor command 'dump-guest-memory' dump in kdump-compressed format.
>> The command's usage:
>>    dump [-p] protocol [begin] [length] [format]
>> 'format' is used to specified the format of vmcore and can be:
>> 1. 'elf': ELF format, without compression
>> 2. 'zlib': kdump-compressed format, with zlib-compressed
>> 3. 'lzo': kdump-compressed format, with lzo-compressed
>> 4. 'snappy': kdump-compressed format, with snappy-compressed
>> And without 'format' being set, vmcore will be in ELF format.
>>
>> Note:
>>    1. The kdump-compressed format is readable only with the crash utility, and it
>>       can be smaller than the ELF format because of the compression support.
>>    2. The kdump-compressed format is the 5th edition.
>>
>> Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com>
>> Signed-off-by: Zhang Xiaohe<zhangxh@cn.fujitsu.com>
>> ---
>
>> -static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>> -                     int64_t begin, int64_t length, Error **errp)
>> +static int dump_init(DumpState *s, int fd, bool compress_format,
>> +                    DumpGuestMemoryFormat format, bool paging,
>> +                    bool has_filter, int64_t begin, int64_t length, Error **errp)
>>   {
>
> Why do you need compress_format as a separate parameter, when that
> information is redundant with the contents of format?

I need this variable to decide whether in kdump-compressed format or
not. Without using it, I need to repeat "has_format && format !=
DUMP_GUEST_MEMORY_FORMAT_ELF" three times.

>
>> +void qmp_dump_guest_memory(bool paging, const char *protocol, bool has_begin,
>> +                            int64_t begin, bool has_length,
>> +                            int64_t length, bool has_format,
>> +                            DumpGuestMemoryFormat format, Error **errp)
>>   {
>>       const char *p;
>>       int fd = -1;
>>       DumpState *s;
>>       int ret;
>> +    int compress_format = 0;
>
> You are using this as a bool, so type it as a bool rather than int.

Got it.

>
>> +++ b/include/sysemu/dump.h
>> @@ -37,6 +37,13 @@
>>   #endif
>>
>>   /*
>> + * dump format
>> + */
>> +#define FLAG_DUMP_COMPRESS_ZLIB     (0x1)   /* compressed with zlib */
>> +#define FLAG_DUMP_COMPRESS_LZO      (0x2)   /* compressed with lzo */
>> +#define FLAG_DUMP_COMPRESS_SNAPPY   (0x4)   /* compressed with snappy */
>
> Why are you skipping 3?  Besides, these aren't flags that can be
> bitwise-or'd together, so the name FLAG_ is misleading.  Why not just
> make it an enum, with value 0, 1, 2 (or with 1, 2, 3)?  In fact, why
> even declare this at all, instead of reusing the DumpGuestMemoryFormat
> enum created for you by the QMP code generation from the schema?

Sorry about this, I didn't use these but forgot to remove them.

>
>> +
>> +/*
>>    * flag used in page desc of kdump-compressed format
>>    */
>>   #define DUMP_DH_COMPRESSED_ZLIB     (0x1)
>> @@ -46,6 +53,7 @@
>>   #define KDUMP_SIGNATURE             "KDUMP   "
>>   #define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
>>   #define DISKDUMP_HEADER_BLOCKS      (1)
>> +#define PAGE_SIZE                   (4096)
>
> Is that true for all platforms?

No. In "note" of patch 0/9,

"The guest should be x86 or x86_64. The other arch is not supported
now."

PAGE_SIZE here is only supposed to support x86 or x86_64.
Eric Blake - May 15, 2013, 12:10 p.m.
On 05/14/2013 11:27 PM, Qiao Nuohan wrote:

>>> +static int dump_init(DumpState *s, int fd, bool compress_format,
>>> +                    DumpGuestMemoryFormat format, bool paging,
>>> +                    bool has_filter, int64_t begin, int64_t length,
>>> Error **errp)
>>>   {
>>
>> Why do you need compress_format as a separate parameter, when that
>> information is redundant with the contents of format?
> 
> I need this variable to decide whether in kdump-compressed format or
> not. Without using it, I need to repeat "has_format && format !=
> DUMP_GUEST_MEMORY_FORMAT_ELF" three times.

Then set it up as a local stack-allocated variable within the function
body.  No need to pass it as part of the function signature, to make all
callers pass redundant information.

Patch

diff --git a/configure b/configure
index 9439f1c..b89beea 100755
--- a/configure
+++ b/configure
@@ -229,6 +229,8 @@  libusb=""
 usb_redir=""
 glx=""
 zlib="yes"
+lzo="no"
+snappy="no"
 guest_agent="yes"
 want_tools="yes"
 libiscsi=""
@@ -898,6 +900,10 @@  for opt do
   ;;
   --disable-zlib-test) zlib="no"
   ;;
+  --enable-lzo) lzo="yes"
+  ;;
+  --enable-snappy) snappy="yes"
+  ;;
   --enable-guest-agent) guest_agent="yes"
   ;;
   --disable-guest-agent) guest_agent="no"
@@ -1494,6 +1500,42 @@  fi
 libs_softmmu="$libs_softmmu -lz"
 
 ##########################################
+# lzo check
+
+if test "$lzo" != "no" ; then
+    cat > $TMPC << EOF
+#include <lzo/lzo1x.h>
+int main(void) { lzo_version(); return 0; }
+EOF
+    if compile_prog "" "-llzo2" ; then
+        :
+    else
+        error_exit "lzo check failed" \
+            "Make sure to have the lzo libs and headers installed."
+    fi
+
+    libs_softmmu="$libs_softmmu -llzo2"
+fi
+
+##########################################
+# snappy check
+
+if test "$snappy" != "no" ; then
+    cat > $TMPC << EOF
+#include <snappy-c.h>
+int main(void) { snappy_max_compressed_length(4096); return 0; }
+EOF
+    if compile_prog "" "-lsnappy" ; then
+        :
+    else
+        error_exit "snappy check failed" \
+            "Make sure to have the snappy libs and headers installed."
+    fi
+
+    libs_softmmu="$libs_softmmu -lsnappy"
+fi
+
+##########################################
 # libseccomp check
 
 if test "$seccomp" != "no" ; then
@@ -3891,6 +3933,14 @@  if test "$glx" = "yes" ; then
   echo "GLX_LIBS=$glx_libs" >> $config_host_mak
 fi
 
+if test "$lzo" = "yes" ; then
+  echo "CONFIG_LZO=y" >> $config_host_mak
+fi
+
+if test "$snappy" = "yes" ; then
+  echo "CONFIG_SNAPPY=y" >> $config_host_mak
+fi
+
 if test "$libiscsi" = "yes" ; then
   echo "CONFIG_LIBISCSI=y" >> $config_host_mak
 fi
diff --git a/dump.c b/dump.c
index c04e2ae..74aa727 100644
--- a/dump.c
+++ b/dump.c
@@ -1027,6 +1027,16 @@  static int create_header64(DumpState *s)
     return 0;
 }
 
+static void get_max_mapnr(DumpState *s)
+{
+    MemoryMapping *memory_mapping;
+
+    QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
+        s->max_mapnr = paddr_to_pfn(memory_mapping->phys_addr +
+                        memory_mapping->length, s->page_shift);
+    }
+}
+
 /*
  * gather data of header and sub header
  */
@@ -1395,12 +1405,14 @@  out:
     return ret;
 }
 
-static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
-                     int64_t begin, int64_t length, Error **errp)
+static int dump_init(DumpState *s, int fd, bool compress_format,
+                    DumpGuestMemoryFormat format, bool paging,
+                    bool has_filter, int64_t begin, int64_t length, Error **errp)
 {
     CPUArchState *env;
     int nr_cpus;
     int ret;
+    unsigned long tmp;
 
     if (runstate_is_running()) {
         vm_stop(RUN_STATE_SAVE_VM);
@@ -1455,6 +1467,56 @@  static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
         qemu_get_guest_simple_memory_mapping(&s->list);
     }
 
+    /* init for kdump-compressed format */
+    if (compress_format) {
+        switch (format) {
+        case DUMP_GUEST_MEMORY_FORMAT_ZLIB:
+            s->flag_compress = DUMP_DH_COMPRESSED_ZLIB;
+            break;
+        case DUMP_GUEST_MEMORY_FORMAT_LZO:
+            s->flag_compress = DUMP_DH_COMPRESSED_LZO;
+            break;
+        case DUMP_GUEST_MEMORY_FORMAT_SNAPPY:
+            s->flag_compress = DUMP_DH_COMPRESSED_SNAPPY;
+            break;
+        default:
+            s->flag_compress = 0;
+        }
+
+        s->nr_cpus = nr_cpus;
+        s->page_size = PAGE_SIZE;
+        s->page_shift = ffs(s->page_size) - 1;
+
+        get_max_mapnr(s);
+
+        tmp = divideup(divideup(s->max_mapnr, CHAR_BIT), s->page_size);
+        s->len_dump_bitmap = tmp * s->page_size * 2;
+
+        /*
+         * gather data of header, dump_bitmap, page_desc and page_data, then
+         * cache them in tmp files
+         */
+        ret = create_header(s);
+        if (ret < 0) {
+            error_set(errp, QERR_UNSUPPORTED);
+            goto cleanup;
+        }
+
+        ret = create_dump_bitmap(s);
+        if (ret < 0) {
+            error_set(errp, QERR_UNSUPPORTED);
+            goto cleanup;
+        }
+
+        ret = create_pages(s);
+        if (ret < 0) {
+            error_set(errp, QERR_UNSUPPORTED);
+            goto cleanup;
+        }
+
+        return 0;
+    }
+
     if (s->has_filter) {
         memory_mapping_filter(&s->list, s->begin, s->length);
     }
@@ -1530,14 +1592,26 @@  static void clean_state(DumpState *s)
     free_cache_data(s->page_data);
 }
 
-void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
-                           int64_t begin, bool has_length, int64_t length,
-                           Error **errp)
+void qmp_dump_guest_memory(bool paging, const char *protocol, bool has_begin,
+                            int64_t begin, bool has_length,
+                            int64_t length, bool has_format,
+                            DumpGuestMemoryFormat format, Error **errp)
 {
     const char *p;
     int fd = -1;
     DumpState *s;
     int ret;
+    int compress_format = 0;
+
+    if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
+        compress_format = 1;
+    }
+
+    /* kdump-compressed format is invalid with paging or filter */
+    if (compress_format && (paging || has_begin || has_length)) {
+        error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+        return;
+    }
 
     if (has_begin && !has_length) {
         error_set(errp, QERR_MISSING_PARAMETER, "length");
@@ -1549,7 +1623,7 @@  void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     }
 
 #if !defined(WIN32)
-    if (strstart(file, "fd:", &p)) {
+    if (strstart(protocol, "fd:", &p)) {
         fd = monitor_get_fd(cur_mon, p, errp);
         if (fd == -1) {
             return;
@@ -1557,7 +1631,7 @@  void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     }
 #endif
 
-    if  (strstart(file, "file:", &p)) {
+    if  (strstart(protocol, "file:", &p)) {
         fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
         if (fd < 0) {
             error_set(errp, QERR_OPEN_FILE_FAILED, p);
@@ -1570,17 +1644,27 @@  void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
         return;
     }
 
-    s = g_malloc(sizeof(DumpState));
+    /* initialize DumpState to zero */
+    s = g_malloc0(sizeof(DumpState));
 
-    ret = dump_init(s, fd, paging, has_begin, begin, length, errp);
+    ret = dump_init(s, fd, compress_format, format, paging, has_begin,
+                    begin, length, errp);
     if (ret < 0) {
         g_free(s);
         return;
     }
 
-    if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
-        error_set(errp, QERR_IO_ERROR);
+    if (compress_format) {
+        if (create_kdump_vmcore(s) < 0 && !error_is_set(s->errp)) {
+            error_set(errp, QERR_IO_ERROR);
+        }
+    } else {
+        if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
+            error_set(errp, QERR_IO_ERROR);
+        }
     }
 
+    clean_state(s);
+
     g_free(s);
 }
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9cea415..b12e50b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -992,17 +992,19 @@  ETEXI
 #if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
-        .params     = "[-p] filename [begin] [length]",
+        .args_type  = "paging:-p,filename:F,begin:i?,length:i?,format:s?",
+        .params     = "[-p] filename [begin] [length] [format]",
         .help       = "dump guest memory to file"
                       "\n\t\t\t begin(optional): the starting physical address"
-                      "\n\t\t\t length(optional): the memory size, in bytes",
+                      "\n\t\t\t length(optional): the memory size, in bytes"
+                      "\n\t\t\t format(optional): the format of guest memory dump,"
+                      "\n\t\t\t it can be elf|zlib|lzo|snappy",
         .mhandler.cmd = hmp_dump_guest_memory,
     },
 
 
 STEXI
-@item dump-guest-memory [-p] @var{protocol} @var{begin} @var{length}
+@item dump-guest-memory [-p] @var{protocol} @var{begin} @var{length} @var{format}
 @findex dump-guest-memory
 Dump guest memory to @var{protocol}. The file can be processed with crash or
 gdb.
@@ -1012,6 +1014,8 @@  gdb.
             specified with length together.
     length: the memory size, in bytes. It's optional, and should be specified
             with begin together.
+    format: the format of guest memory dump. It's optional, and can be
+            elf|zlib|lzo|snappy.
 ETEXI
 #endif
 
diff --git a/hmp.c b/hmp.c
index 4fb76ec..e863267 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1185,9 +1185,12 @@  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_format = qdict_haskey(qdict, "format");
     int64_t begin = 0;
     int64_t length = 0;
+    const char *format = NULL;
     char *prot;
+    enum DumpGuestMemoryFormat dump_format;
 
     if (has_begin) {
         begin = qdict_get_int(qdict, "begin");
@@ -1195,11 +1198,29 @@  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     if (has_length) {
         length = qdict_get_int(qdict, "length");
     }
+    if (has_format) {
+        format = qdict_get_str(qdict, "format");
+    }
+
+    if (strcmp(format, "elf") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
+    } else if (strcmp(format, "zlib") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_ZLIB;
+    } else if (strcmp(format, "lzo") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_LZO;
+    } else if (strcmp(format, "snappy") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_SNAPPY;
+    } else {
+        error_set(&errp, QERR_INVALID_PARAMETER_VALUE,
+                  "format", "elf|zlib|lzo|snappy");
+        hmp_handle_error(mon, &errp);
+        return;
+    }
 
     prot = g_strconcat("file:", file, NULL);
 
     qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
-                          &errp);
+                          has_format, dump_format, &errp);
     hmp_handle_error(mon, &errp);
     g_free(prot);
 }
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 000455f..4da8a33 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -37,6 +37,13 @@ 
 #endif
 
 /*
+ * dump format
+ */
+#define FLAG_DUMP_COMPRESS_ZLIB     (0x1)   /* compressed with zlib */
+#define FLAG_DUMP_COMPRESS_LZO      (0x2)   /* compressed with lzo */
+#define FLAG_DUMP_COMPRESS_SNAPPY   (0x4)   /* compressed with snappy */
+
+/*
  * flag used in page desc of kdump-compressed format
  */
 #define DUMP_DH_COMPRESSED_ZLIB     (0x1)
@@ -46,6 +53,7 @@ 
 #define KDUMP_SIGNATURE             "KDUMP   "
 #define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
 #define DISKDUMP_HEADER_BLOCKS      (1)
+#define PAGE_SIZE                   (4096)
 #define PHYS_BASE                   (0)
 #define DUMP_LEVEL                  (1)
 #define TMP_BUF_SIZE                (1024)
diff --git a/qapi-schema.json b/qapi-schema.json
index 199744a..535dbec 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2381,6 +2381,18 @@ 
 { 'command': 'device_del', 'data': {'id': 'str'} }
 
 ##
+# @DumpGuestMemoryFormat
+#
+# Format of Guest memory dump
+#
+# Each represents a format of guest memory dump.
+#
+# Since: 1.6
+##
+{ 'enum': 'DumpGuestMemoryFormat',
+  'data': [ 'elf', 'zlib', 'lzo', 'snappy' ] }
+
+##
 # @dump-guest-memory
 #
 # Dump guest's memory to vmcore. It is a synchronous operation that can take
@@ -2416,13 +2428,20 @@ 
 #          want to dump all guest's memory, please specify the start @begin
 #          and @length
 #
+# @format: #optional if specified, the format of guest memory dump.
+#
+#            1. elf: dump file will be in elf format
+#            2. zlib: dump file will be in kdump format with zlib-compressed
+#            3. lzo: dump file will be in kdump format with lzo-compressed
+#            4. snappy: dump file will be in kdump format with snappy-compressed
+#
 # Returns: nothing on success
 #
-# Since: 1.2
+# Since: 1.6
 ##
 { 'command': 'dump-guest-memory',
   'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
-            '*length': 'int' } }
+            '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
 
 ##
 # @netdev_add:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ffd130e..f106fc1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -788,8 +788,8 @@  EQMP
 
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:b,protocol:s,begin:i?,end:i?",
-        .params     = "-p protocol [begin] [length]",
+        .args_type  = "paging:b,protocol:s,begin:i?,length:i?,format:s?",
+        .params     = "-p protocol [begin] [length] [format]",
         .help       = "dump guest memory to file",
         .user_print = monitor_user_noop,
         .mhandler.cmd_new = qmp_marshal_input_dump_guest_memory,
@@ -810,6 +810,8 @@  Arguments:
            with length together (json-int)
 - "length": the memory size, in bytes. It's optional, and should be specified
             with begin together (json-int)
+- "format": the format of guest memory dump. It's optional, and can be
+            elf|zlib|lzo|snappy(json-string)
 
 Example: