[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 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.

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

diff --git a/dump.c b/dump.c
index 2bf8d8d..5eea015 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;
 
-    /* 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;
+    writen_size = qemu_write_full(s->fd, buf, size);
+    if (writen_size != size) {
+        return -1;
     }
 
     return 0;
-- 
1.7.12.315.g682ce8b

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

[Eric Blake]

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

On 09/21/2012 11:07 AM, Luiz Capitulino wrote:
> 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.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  dump.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 2bf8d8d..5eea015 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;

While you are here, s/writen/written/ in the local variable.

-- 
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 2/3] qmp: dump-guest-memory: don't spin if non-blocking fd would block

[Wen Congyang]

At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
> 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.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  dump.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 2bf8d8d..5eea015 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;
>  
> -    /* 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) {

Hmm, if the fd is a blocking fd, errno can't be EAGAIN. So the
function doesn't spin. What problems do you meet?

Thanks
Wen Congyang

> -            return -1;
> -        }
> -
> -        buf += writen_size;
> -        size -= writen_size;
> +    writen_size = qemu_write_full(s->fd, buf, size);
> +    if (writen_size != size) {
> +        return -1;
>      }
>  
>      return 0;

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

[Luiz Capitulino]

On Mon, 24 Sep 2012 14:27:17 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
> > 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.
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  dump.c | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> > 
> > diff --git a/dump.c b/dump.c
> > index 2bf8d8d..5eea015 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;
> >  
> > -    /* 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) {
> 
> Hmm, if the fd is a blocking fd, errno can't be EAGAIN. So the
> function doesn't spin. What problems do you meet?

The problem is with non-blocking fds, where spinning isn't correct, for
two reasons:

 1. If the fd is non-blocking, that means you don't want to block
    and spinning for a long time will have the same effects

 2. Spinning consumes host resources

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

[Wen Congyang]

At 09/24/2012 09:34 PM, Luiz Capitulino Wrote:
> On Mon, 24 Sep 2012 14:27:17 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
>>> 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.
>>>
>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>>> ---
>>>  dump.c | 13 +++----------
>>>  1 file changed, 3 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/dump.c b/dump.c
>>> index 2bf8d8d..5eea015 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;
>>>  
>>> -    /* 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) {
>>
>> Hmm, if the fd is a blocking fd, errno can't be EAGAIN. So the
>> function doesn't spin. What problems do you meet?
> 
> The problem is with non-blocking fds, where spinning isn't correct, for
> two reasons:

But, If the fd is non-blocking, errno can't be EAGAIN. So it doesn't spin.

Thanks
Wen Congyang

> 
>  1. If the fd is non-blocking, that means you don't want to block
>     and spinning for a long time will have the same effects
> 
>  2. Spinning consumes host resources
> 

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

[Markus Armbruster]

Wen Congyang <wency@cn.fujitsu.com> writes:

> At 09/24/2012 09:34 PM, Luiz Capitulino Wrote:
>> On Mon, 24 Sep 2012 14:27:17 +0800
>> Wen Congyang <wency@cn.fujitsu.com> wrote:
>> 
>>> At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>>>> ---
>>>>  dump.c | 13 +++----------
>>>>  1 file changed, 3 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/dump.c b/dump.c
>>>> index 2bf8d8d..5eea015 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;
>>>>  
>>>> -    /* 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) {
>>>
>>> Hmm, if the fd is a blocking fd, errno can't be EAGAIN. So the
>>> function doesn't spin. What problems do you meet?
>> 
>> The problem is with non-blocking fds, where spinning isn't correct, for
>> two reasons:
>
> But, If the fd is non-blocking, errno can't be EAGAIN. So it doesn't spin.

I'm afraid you're confused.

http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

[EAGAIN] The O_NONBLOCK flag is set for the file descriptor and the
thread would be delayed in the write() operation.

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

[Wen Congyang]

At 09/25/2012 05:01 PM, Markus Armbruster Wrote:
> Wen Congyang <wency@cn.fujitsu.com> writes:
> 
>> At 09/24/2012 09:34 PM, Luiz Capitulino Wrote:
>>> On Mon, 24 Sep 2012 14:27:17 +0800
>>> Wen Congyang <wency@cn.fujitsu.com> wrote:
>>>
>>>> At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>>>>> ---
>>>>>  dump.c | 13 +++----------
>>>>>  1 file changed, 3 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/dump.c b/dump.c
>>>>> index 2bf8d8d..5eea015 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;
>>>>>  
>>>>> -    /* 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) {
>>>>
>>>> Hmm, if the fd is a blocking fd, errno can't be EAGAIN. So the
>>>> function doesn't spin. What problems do you meet?
>>>
>>> The problem is with non-blocking fds, where spinning isn't correct, for
>>> two reasons:
>>
>> But, If the fd is non-blocking, errno can't be EAGAIN. So it doesn't spin.
> 
> I'm afraid you're confused.
> 
> http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html
> 
> [EAGAIN] The O_NONBLOCK flag is set for the file descriptor and the
> thread would be delayed in the write() operation.
> 

Ahh, you are right.

Thanks
Wen Congyang

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

[Wen Congyang]

At 09/24/2012 09:34 PM, Luiz Capitulino Wrote:
> On Mon, 24 Sep 2012 14:27:17 +0800
> Wen Congyang <wency@cn.fujitsu.com> wrote:
> 
>> At 09/22/2012 01:07 AM, Luiz Capitulino Wrote:
>>> 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.
>>>
>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>>> ---
>>>  dump.c | 13 +++----------
>>>  1 file changed, 3 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/dump.c b/dump.c
>>> index 2bf8d8d..5eea015 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;
>>>  
>>> -    /* 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) {
>>
>> Hmm, if the fd is a blocking fd, errno can't be EAGAIN. So the
>> function doesn't spin. What problems do you meet?
> 
> The problem is with non-blocking fds, where spinning isn't correct, for
> two reasons:
> 
>  1. If the fd is non-blocking, that means you don't want to block
>     and spinning for a long time will have the same effects
> 
>  2. Spinning consumes host resources
> 


If so, I agree with it, and the patch looks fine to me

Thanks
Wen Congyang