[Qemu-devel] [PATCH v2 0/3] qmp/hmp: dump-guest-memory fixes

[Qemu-devel] [PATCH v2 0/3] qmp/hmp: dump-guest-memory fixes

[Luiz Capitulino]

Please, check individual patches for details.

v2

 - Fix TAB/spaces in qapi-schema.json
 - English fixes
 - Use g_strconcat() (instead of a qstring)

Luiz Capitulino (3):
  qmp: dump-guest-memory: improve schema doc (again)
  qmp: dump-guest-memory: don't spin if non-blocking fd would block
  hmp: dump-guest-memory: hardcode protocol argument to "file:"

 dump.c           | 15 ++++-----------
 hmp-commands.hx  |  8 +++-----
 hmp.c            |  8 ++++++--
 qapi-schema.json | 32 ++++++++++++++++++++------------
 4 files changed, 33 insertions(+), 30 deletions(-)

-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PATCH 1/3] qmp: dump-guest-memory: improve schema doc (again)

[Luiz Capitulino]

 o Add a note about memory allocation with paging=true
 o Fix indentation

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi-schema.json | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 14e4419..6305733 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1982,26 +1982,33 @@
 # supported on i386 and x86_64.
 #
 # @paging: if true, do paging to get guest's memory mapping. This allows
-# using gdb to process the core file. However, setting @paging to false
-# may be desirable because of two reasons:
+#          using gdb to process the core file.
 #
-#   1. The guest may be in a catastrophic state or can have corrupted
-#      memory, which cannot be trusted
-#   2. The guest can be in real-mode even if paging is enabled. For example,
-#      the guest uses ACPI to sleep, and ACPI sleep state goes in real-mode
+#          IMPORTANT: this option can make QEMU allocate several gigabytes
+#                     of RAM. This can happen for a large guest, or a
+#                     malicious guest pretending to be large.
+#
+#          Also, paging=true has the following limitations:
+#
+#             1. The guest may be in a catastrophic state or can have corrupted
+#                memory, which cannot be trusted
+#             2. The guest can be in real-mode even if paging is enabled. For
+#                example, the guest uses ACPI to sleep, and ACPI sleep state
+#                goes in real-mode
 #
 # @protocol: the filename or file descriptor of the vmcore. The supported
-# protocols are:
+#            protocols are:
 #
-#   1. file: the protocol starts with "file:", and the following string is
-#      the file's path.
-#   2. fd: the protocol starts with "fd:", and the following string is the
-#      fd's name.
+#            1. file: the protocol starts with "file:", and the following
+#               string is the file's path.
+#            2. fd: the protocol starts with "fd:", and the following string
+#               is the fd's name.
 #
 # @begin: #optional if specified, the starting physical address.
 #
 # @length: #optional if specified, the memory size, in bytes. If you don't
-# want to dump all guest's memory, please specify the start @begin and @length
+#          want to dump all guest's memory, please specify the start @begin
+#          and @length
 #
 # Returns: nothing on success
 #
@@ -2010,6 +2017,7 @@
 { 'command': 'dump-guest-memory',
   'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
             '*length': 'int' } }
+
 ##
 # @netdev_add:
 #
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PATCH 2/3] qmp: dump-guest-memory: don't spin if non-blocking fd would block

[Luiz Capitulino]

fd_write_vmcore() will indefinitely spin for a non-blocking
file-descriptor that would block. However, if the fd is non-blocking,
how does it make sense to spin?

Change this behavior to return an error instead.

Note that this can only happen with an fd provided by a management
application. The fd opened internally by dump-guest-memory is blocking.

While there, also fix 'writen_size' variable name.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 dump.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/dump.c b/dump.c
index 2bf8d8d..81c3624 100644
--- a/dump.c
+++ b/dump.c
@@ -100,18 +100,11 @@ static void dump_error(DumpState *s, const char *reason)
 static int fd_write_vmcore(void *buf, size_t size, void *opaque)
 {
     DumpState *s = opaque;
-    int fd = s->fd;
-    size_t writen_size;
+    size_t written_size;
 
-    /* The fd may be passed from user, and it can be non-blocked */
-    while (size) {
-        writen_size = qemu_write_full(fd, buf, size);
-        if (writen_size != size && errno != EAGAIN) {
-            return -1;
-        }
-
-        buf += writen_size;
-        size -= writen_size;
+    written_size = qemu_write_full(s->fd, buf, size);
+    if (written_size != size) {
+        return -1;
     }
 
     return 0;
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to "file:"

[Luiz Capitulino]

Today, it's necessary to specify the protocol you want to use
when dumping the guest memory, for example:

 (qemu) dump-guest-memory file:/tmp/guest-memory

This has a few issues:

 1. It's cumbersome to type
 2. We loose file path autocompletion
 3. Being able to specify fd:X in HMP makes little sense for humans

Because of these reasons, hardcode the 'protocol' argument to
'file:' in HMP.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp-commands.hx | 8 +++-----
 hmp.c           | 8 ++++++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..0302458 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -914,12 +914,11 @@ ETEXI
 #if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,protocol:s,begin:i?,length:i?",
-        .params     = "[-p] protocol [begin] [length]",
+        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
+        .params     = "[-p] filename [begin] [length]",
         .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",
-        .user_print = monitor_user_noop,
         .mhandler.cmd = hmp_dump_guest_memory,
     },
 
@@ -929,8 +928,7 @@ STEXI
 @findex dump-guest-memory
 Dump guest memory to @var{protocol}. The file can be processed with crash or
 gdb.
-  protocol: destination file(started with "file:") or destination file
-            descriptor (started with "fd:")
+  filename: dump file name
     paging: do paging to get guest's memory mapping
      begin: the starting physical address. It's optional, and should be
             specified with length together.
diff --git a/hmp.c b/hmp.c
index ba6fbd3..2de3140 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
     Error *errp = NULL;
     int paging = qdict_get_try_bool(qdict, "paging", 0);
-    const char *file = qdict_get_str(qdict, "protocol");
+    const char *file = qdict_get_str(qdict, "filename");
     bool has_begin = qdict_haskey(qdict, "begin");
     bool has_length = qdict_haskey(qdict, "length");
     int64_t begin = 0;
     int64_t length = 0;
+    char *prot;
 
     if (has_begin) {
         begin = qdict_get_int(qdict, "begin");
@@ -1055,9 +1056,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
         length = qdict_get_int(qdict, "length");
     }
 
-    qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
+    prot = g_strconcat("file:", file, NULL);
+
+    qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
                           &errp);
     hmp_handle_error(mon, &errp);
+    g_free(prot);
 }
 
 void hmp_netdev_add(Monitor *mon, const QDict *qdict)
-- 
1.7.12.315.g682ce8b

Re: [Qemu-devel] [PATCH v2 0/3] qmp/hmp: dump-guest-memory fixes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

On 09/26/2012 12:56 PM, Luiz Capitulino wrote:
> Please, check individual patches for details.
> 
> v2
> 
>  - Fix TAB/spaces in qapi-schema.json
>  - English fixes
>  - Use g_strconcat() (instead of a qstring)
> 
> Luiz Capitulino (3):
>   qmp: dump-guest-memory: improve schema doc (again)
>   qmp: dump-guest-memory: don't spin if non-blocking fd would block
>   hmp: dump-guest-memory: hardcode protocol argument to "file:"
> 
>  dump.c           | 15 ++++-----------
>  hmp-commands.hx  |  8 +++-----
>  hmp.c            |  8 ++++++--
>  qapi-schema.json | 32 ++++++++++++++++++++------------
>  4 files changed, 33 insertions(+), 30 deletions(-)
> 

Series:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH v2 0/3] qmp/hmp: dump-guest-memory fixes

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Please, check individual patches for details.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[Qemu-devel] [PATCH 0/3]: qmp/hmp: dump-guest-memory fixes

[Luiz Capitulino]

Please see individual patches for details.

 dump.c           | 13 +++----------
 hmp-commands.hx  |  8 +++-----
 hmp.c            | 11 ++++++++---
 qapi-schema.json | 32 ++++++++++++++++++++------------
 4 files changed, 34 insertions(+), 30 deletions(-)

[Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file:

[Luiz Capitulino]

Today, it's necessary to specify the protocol you want to use
when dumping the guest memory, for example:

 (qemu) dump-guest-memory file:/tmp/guest-memory

This has a few issues:

 1. It's cumbersome to type
 2. We loose file path autocompletion
 3. Being able to specify fd:X in HMP makes little sense for humans

Because of these reasons, hardcode the 'protocol' argument to
'file:' in HMP.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp-commands.hx |  8 +++-----
 hmp.c           | 11 ++++++++---
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..0302458 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -914,12 +914,11 @@ ETEXI
 #if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,protocol:s,begin:i?,length:i?",
-        .params     = "[-p] protocol [begin] [length]",
+        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
+        .params     = "[-p] filename [begin] [length]",
         .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",
-        .user_print = monitor_user_noop,
         .mhandler.cmd = hmp_dump_guest_memory,
     },
 
@@ -929,8 +928,7 @@ STEXI
 @findex dump-guest-memory
 Dump guest memory to @var{protocol}. The file can be processed with crash or
 gdb.
-  protocol: destination file(started with "file:") or destination file
-            descriptor (started with "fd:")
+  filename: dump file name
     paging: do paging to get guest's memory mapping
      begin: the starting physical address. It's optional, and should be
             specified with length together.
diff --git a/hmp.c b/hmp.c
index ba6fbd3..513b40b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
     Error *errp = NULL;
     int paging = qdict_get_try_bool(qdict, "paging", 0);
-    const char *file = qdict_get_str(qdict, "protocol");
+    const char *file = qdict_get_str(qdict, "filename");
     bool has_begin = qdict_haskey(qdict, "begin");
     bool has_length = qdict_haskey(qdict, "length");
     int64_t begin = 0;
     int64_t length = 0;
+    QString *prot;
 
     if (has_begin) {
         begin = qdict_get_int(qdict, "begin");
@@ -1055,9 +1056,13 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
         length = qdict_get_int(qdict, "length");
     }
 
-    qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
-                          &errp);
+    prot = qstring_from_str("file:");
+    qstring_append(prot, file);
+
+    qmp_dump_guest_memory(paging, qstring_get_str(prot), has_begin, begin,
+                          has_length, length, &errp);
     hmp_handle_error(mon, &errp);
+    QDECREF(prot);
 }
 
 void hmp_netdev_add(Monitor *mon, const QDict *qdict)
-- 
1.7.12.315.g682ce8b

Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file:

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

On 09/21/2012 11:07 AM, Luiz Capitulino wrote:
> Today, it's necessary to specify the protocol you want to use
> when dumping the guest memory, for example:
> 
>  (qemu) dump-guest-memory file:/tmp/guest-memory
> 
> This has a few issues:
> 
>  1. It's cumbersome to type
>  2. We loose file path autocompletion
>  3. Being able to specify fd:X in HMP makes little sense for humans
> 
> Because of these reasons, hardcode the 'protocol' argument to
> 'file:' in HMP.

No impact to libvirt as a QMP client, and we've already declared that
HMP can change for better UI even if it is not back-compatible.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file:

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Today, it's necessary to specify the protocol you want to use
> when dumping the guest memory, for example:
>
>  (qemu) dump-guest-memory file:/tmp/guest-memory
>
> This has a few issues:
>
>  1. It's cumbersome to type
>  2. We loose file path autocompletion
>  3. Being able to specify fd:X in HMP makes little sense for humans
>
> Because of these reasons, hardcode the 'protocol' argument to
> 'file:' in HMP.

No objection.

The QMP wart remains: protocol is a string that needs to be parsed.  I
feel that should be avoided in QMP.  We should have expressed "either
filename or file descriptor name" in the schema.  Let's avoid such
mistakes in the future.

One remark inline.

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hmp-commands.hx |  8 +++-----
>  hmp.c           | 11 ++++++++---
>  2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ed67e99..0302458 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -914,12 +914,11 @@ ETEXI
>  #if defined(CONFIG_HAVE_CORE_DUMP)
>      {
>          .name       = "dump-guest-memory",
> -        .args_type  = "paging:-p,protocol:s,begin:i?,length:i?",
> -        .params     = "[-p] protocol [begin] [length]",
> +        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
> +        .params     = "[-p] filename [begin] [length]",
>          .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",
> -        .user_print = monitor_user_noop,
>          .mhandler.cmd = hmp_dump_guest_memory,
>      },
>  
> @@ -929,8 +928,7 @@ STEXI
>  @findex dump-guest-memory
>  Dump guest memory to @var{protocol}. The file can be processed with crash or
>  gdb.
> -  protocol: destination file(started with "file:") or destination file
> -            descriptor (started with "fd:")
> +  filename: dump file name
>      paging: do paging to get guest's memory mapping
>       begin: the starting physical address. It's optional, and should be
>              specified with length together.
> diff --git a/hmp.c b/hmp.c
> index ba6fbd3..513b40b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>  {
>      Error *errp = NULL;
>      int paging = qdict_get_try_bool(qdict, "paging", 0);
> -    const char *file = qdict_get_str(qdict, "protocol");
> +    const char *file = qdict_get_str(qdict, "filename");
>      bool has_begin = qdict_haskey(qdict, "begin");
>      bool has_length = qdict_haskey(qdict, "length");
>      int64_t begin = 0;
>      int64_t length = 0;
> +    QString *prot;
>  
>      if (has_begin) {
>          begin = qdict_get_int(qdict, "begin");
> @@ -1055,9 +1056,13 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>          length = qdict_get_int(qdict, "length");
>      }
>  
> -    qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
> -                          &errp);
> +    prot = qstring_from_str("file:");
> +    qstring_append(prot, file);

What about prot = g_strconcat("file:", file, NULL) ... g_free(prot) ?

> +
> +    qmp_dump_guest_memory(paging, qstring_get_str(prot), has_begin, begin,
> +                          has_length, length, &errp);
>      hmp_handle_error(mon, &errp);
> +    QDECREF(prot);
>  }
>  
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict)

Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file:

[Luiz Capitulino]

On Tue, 25 Sep 2012 10:48:05 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Today, it's necessary to specify the protocol you want to use
> > when dumping the guest memory, for example:
> >
> >  (qemu) dump-guest-memory file:/tmp/guest-memory
> >
> > This has a few issues:
> >
> >  1. It's cumbersome to type
> >  2. We loose file path autocompletion
> >  3. Being able to specify fd:X in HMP makes little sense for humans
> >
> > Because of these reasons, hardcode the 'protocol' argument to
> > 'file:' in HMP.
> 
> No objection.
> 
> The QMP wart remains: protocol is a string that needs to be parsed.  I
> feel that should be avoided in QMP.  We should have expressed "either
> filename or file descriptor name" in the schema.  Let's avoid such
> mistakes in the future.

I think that the right solution would be to have an URI.

> One remark inline.
> 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hmp-commands.hx |  8 +++-----
> >  hmp.c           | 11 ++++++++---
> >  2 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index ed67e99..0302458 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -914,12 +914,11 @@ ETEXI
> >  #if defined(CONFIG_HAVE_CORE_DUMP)
> >      {
> >          .name       = "dump-guest-memory",
> > -        .args_type  = "paging:-p,protocol:s,begin:i?,length:i?",
> > -        .params     = "[-p] protocol [begin] [length]",
> > +        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
> > +        .params     = "[-p] filename [begin] [length]",
> >          .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",
> > -        .user_print = monitor_user_noop,
> >          .mhandler.cmd = hmp_dump_guest_memory,
> >      },
> >  
> > @@ -929,8 +928,7 @@ STEXI
> >  @findex dump-guest-memory
> >  Dump guest memory to @var{protocol}. The file can be processed with crash or
> >  gdb.
> > -  protocol: destination file(started with "file:") or destination file
> > -            descriptor (started with "fd:")
> > +  filename: dump file name
> >      paging: do paging to get guest's memory mapping
> >       begin: the starting physical address. It's optional, and should be
> >              specified with length together.
> > diff --git a/hmp.c b/hmp.c
> > index ba6fbd3..513b40b 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
> >  {
> >      Error *errp = NULL;
> >      int paging = qdict_get_try_bool(qdict, "paging", 0);
> > -    const char *file = qdict_get_str(qdict, "protocol");
> > +    const char *file = qdict_get_str(qdict, "filename");
> >      bool has_begin = qdict_haskey(qdict, "begin");
> >      bool has_length = qdict_haskey(qdict, "length");
> >      int64_t begin = 0;
> >      int64_t length = 0;
> > +    QString *prot;
> >  
> >      if (has_begin) {
> >          begin = qdict_get_int(qdict, "begin");
> > @@ -1055,9 +1056,13 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
> >          length = qdict_get_int(qdict, "length");
> >      }
> >  
> > -    qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
> > -                          &errp);
> > +    prot = qstring_from_str("file:");
> > +    qstring_append(prot, file);
> 
> What about prot = g_strconcat("file:", file, NULL) ... g_free(prot) ?

Good suggestion.

> 
> > +
> > +    qmp_dump_guest_memory(paging, qstring_get_str(prot), has_begin, begin,
> > +                          has_length, length, &errp);
> >      hmp_handle_error(mon, &errp);
> > +    QDECREF(prot);
> >  }
> >  
> >  void hmp_netdev_add(Monitor *mon, const QDict *qdict)
> 

Re: [Qemu-devel] [PATCH 3/3] hmp: dump-guest-memory: hardcode protocol argument to file:

[Wen Congyang]

At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
> Today, it's necessary to specify the protocol you want to use
> when dumping the guest memory, for example:
> 
>  (qemu) dump-guest-memory file:/tmp/guest-memory
> 
> This has a few issues:
> 
>  1. It's cumbersome to type
>  2. We loose file path autocompletion
>  3. Being able to specify fd:X in HMP makes little sense for humans
> 
> Because of these reasons, hardcode the 'protocol' argument to
> 'file:' in HMP.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

This patch looks fine to me.

Thanks
Wen Congyang

> ---
>  hmp-commands.hx |  8 +++-----
>  hmp.c           | 11 ++++++++---
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ed67e99..0302458 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -914,12 +914,11 @@ ETEXI
>  #if defined(CONFIG_HAVE_CORE_DUMP)
>      {
>          .name       = "dump-guest-memory",
> -        .args_type  = "paging:-p,protocol:s,begin:i?,length:i?",
> -        .params     = "[-p] protocol [begin] [length]",
> +        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
> +        .params     = "[-p] filename [begin] [length]",
>          .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",
> -        .user_print = monitor_user_noop,
>          .mhandler.cmd = hmp_dump_guest_memory,
>      },
>  
> @@ -929,8 +928,7 @@ STEXI
>  @findex dump-guest-memory
>  Dump guest memory to @var{protocol}. The file can be processed with crash or
>  gdb.
> -  protocol: destination file(started with "file:") or destination file
> -            descriptor (started with "fd:")
> +  filename: dump file name
>      paging: do paging to get guest's memory mapping
>       begin: the starting physical address. It's optional, and should be
>              specified with length together.
> diff --git a/hmp.c b/hmp.c
> index ba6fbd3..513b40b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>  {
>      Error *errp = NULL;
>      int paging = qdict_get_try_bool(qdict, "paging", 0);
> -    const char *file = qdict_get_str(qdict, "protocol");
> +    const char *file = qdict_get_str(qdict, "filename");
>      bool has_begin = qdict_haskey(qdict, "begin");
>      bool has_length = qdict_haskey(qdict, "length");
>      int64_t begin = 0;
>      int64_t length = 0;
> +    QString *prot;
>  
>      if (has_begin) {
>          begin = qdict_get_int(qdict, "begin");
> @@ -1055,9 +1056,13 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>          length = qdict_get_int(qdict, "length");
>      }
>  
> -    qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
> -                          &errp);
> +    prot = qstring_from_str("file:");
> +    qstring_append(prot, file);
> +
> +    qmp_dump_guest_memory(paging, qstring_get_str(prot), has_begin, begin,
> +                          has_length, length, &errp);
>      hmp_handle_error(mon, &errp);
> +    QDECREF(prot);
>  }
>  
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict)