[v2] dump: Show custom message for ENOSPC

[Qemu-devel] [PATCH v2] dump: Show custom message for ENOSPC

Posted by Yasmin Beatriz 6 years, 2 months ago

This patch intends to make a more specific message for when
the system has not enough space to save guest memory.

Reported-by: yilzhang@redhat.com
Cc: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Signed-off-by: Yasmin Beatriz <yasmins@linux.vnet.ibm.com>
---
 dump.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/dump.c b/dump.c
index e9dfed060a..12e0c779ee 100644
--- a/dump.c
+++ b/dump.c
@@ -106,7 +106,7 @@ static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
 
     written_size = qemu_write_full(s->fd, buf, size);
     if (written_size != size) {
-        return -1;
+        return -errno;
     }
 
     return 0;
@@ -364,7 +364,11 @@ 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");
+        if (ret == -ENOSPC) {
+            error_setg(errp, "dump: not enough space to save memory");
+        } else {
+            error_setg(errp, "dump: failed to save memory");
+        }
     } else {
         s->written_size += length;
     }
-- 
2.14.1

Re: [Qemu-devel] [PATCH v2] dump: Show custom message for ENOSPC

Posted by Daniel Henrique Barboza 6 years, 2 months ago


On 02/09/2018 06:26 PM, Yasmin Beatriz wrote:
> This patch intends to make a more specific message for when
> the system has not enough space to save guest memory.
>
> Reported-by: yilzhang@redhat.com
> Cc: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> Signed-off-by: Yasmin Beatriz <yasmins@linux.vnet.ibm.com>
> ---
>   dump.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>

> diff --git a/dump.c b/dump.c
> index e9dfed060a..12e0c779ee 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -106,7 +106,7 @@ static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
>
>       written_size = qemu_write_full(s->fd, buf, size);
>       if (written_size != size) {
> -        return -1;
> +        return -errno;
>       }
>
>       return 0;
> @@ -364,7 +364,11 @@ 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");
> +        if (ret == -ENOSPC) {
> +            error_setg(errp, "dump: not enough space to save memory");
> +        } else {
> +            error_setg(errp, "dump: failed to save memory");
> +        }
>       } else {
>           s->written_size += length;
>       }

Re: [Qemu-devel] [PATCH v2] dump: Show custom message for ENOSPC

Posted by Eric Blake 6 years, 2 months ago

On 02/09/2018 02:26 PM, Yasmin Beatriz wrote:
> This patch intends to make a more specific message for when
> the system has not enough space to save guest memory.
> 
> Reported-by: yilzhang@redhat.com
> Cc: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> Signed-off-by: Yasmin Beatriz <yasmins@linux.vnet.ibm.com>
> ---

> @@ -364,7 +364,11 @@ 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");
> +        if (ret == -ENOSPC) {
> +            error_setg(errp, "dump: not enough space to save memory");
> +        } else {
> +            error_setg(errp, "dump: failed to save memory");
> +        }

Why is this caller the only one in the file that gets distinguished 
error messages?  And why not use error_setg_errno() instead of 
open-coding just one special errno value?  If you're changing the return 
value of fd_write_vmcore() anyways, I'd suggest that ALL callers in the 
file be updated to use error_setg_errno().

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2] dump: Show custom message for ENOSPC

Posted by joserz@linux.vnet.ibm.com 6 years, 2 months ago

On Fri, Feb 09, 2018 at 02:57:33PM -0600, Eric Blake wrote:
> On 02/09/2018 02:26 PM, Yasmin Beatriz wrote:
> > This patch intends to make a more specific message for when
> > the system has not enough space to save guest memory.
> > 
> > Reported-by: yilzhang@redhat.com
> > Cc: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> > Signed-off-by: Yasmin Beatriz <yasmins@linux.vnet.ibm.com>
> > ---
> 
> > @@ -364,7 +364,11 @@ 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");
> > +        if (ret == -ENOSPC) {
> > +            error_setg(errp, "dump: not enough space to save memory");
> > +        } else {
> > +            error_setg(errp, "dump: failed to save memory");
> > +        }
> 
> Why is this caller the only one in the file that gets distinguished error
> messages?  And why not use error_setg_errno() instead of open-coding just
> one special errno value?  If you're changing the return value of
> fd_write_vmcore() anyways, I'd suggest that ALL callers in the file be
> updated to use error_setg_errno().

Hello Eric, thanks for reviewing it! :)

Yes, only this part has this distiguished error handling because
the whole big amount of data is managed in here, as far as we can tell
it.

It's triggered when you have one of those huge memory guests that
crashes and want to dump its memory into a host that can't offer all
these requirements. AFAIK, other callers doesn't handle so much data,
so we decided to handle only in this part to be the less intrusive
possible.

Your suggestions will be applied then!

> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>