[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 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..3d93ebe 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 allocates 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

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

[Eric Blake]

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

On 09/21/2012 11:07 AM, Luiz Capitulino wrote:
>  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..3d93ebe 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.

You've got a TAB in there, that made it hard to see what the intended
indentation really is.  The rest of the file doesn't have TABs, so you
need to respin this.

> +# 	   IMPORTANT: this option can make QEMU allocates several gigabytes

s/allocates/allocate/

-- 
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 --]