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

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

[Yasmin Beatriz]

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 | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/dump.c b/dump.c
index e9dfed060a..eb56ff53f6 100644
--- a/dump.c
+++ b/dump.c
@@ -106,6 +106,9 @@ 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) {
+        if (errno == ENOSPC) {
+            return -ENOSPC;
+        }
         return -1;
     }
 
@@ -364,7 +367,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] dump: Show custom message for ENOSPC

[Daniel Henrique Barboza]

Hi Yasmin,

On 02/09/2018 05:31 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 | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/dump.c b/dump.c
> index e9dfed060a..eb56ff53f6 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -106,6 +106,9 @@ 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) {
> +        if (errno == ENOSPC) {
> +            return -ENOSPC;
> +        }
You can do like this:


      if (written_size != size) {
+        return -errno;
+     }


Everyone is checking for a negative "ret" to see if an error occurred in 
qemu_write_full.
There is no negative errno AFAIK, so you can spare one "if" clause there 
and still
check for -ENOSPC down there.

It might be worth checking if this code can't be baked into 
qemu_write_full too.



Thanks,


Daniel

>           return -1;
>       }
>
> @@ -364,7 +367,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] dump: Show custom message for ENOSPC

[Murilo Opsfelder Araujo]

Hi, Yasmin.

Congratulations on your first patch!

On 02/09/2018 05:31 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 | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/dump.c b/dump.c
> index e9dfed060a..eb56ff53f6 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -106,6 +106,9 @@ 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) {
> +        if (errno == ENOSPC) {
> +            return -ENOSPC;
> +        }
>          return -1;
>      }
> 
> @@ -364,7 +367,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");
> +        }

If fd_write_vmcore() returned -errno, as Daniel Barboza suggested, it
could be used in error_setg_errno(). Something like this:

diff --git a/dump.c b/dump.c
index e9dfed060a..313a7460a7 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,7 @@ 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");
+        error_setg_errno(errp, ret, "dump: failed to save memory");
     } else {
         s->written_size += length;
     }

With this, other reasons of errno would also be considered, not only ENOSPC.

Cheers
Murilo

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

[Eric Blake]

On 02/09/2018 01:31 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 | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/dump.c b/dump.c
> index e9dfed060a..eb56ff53f6 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -106,6 +106,9 @@ 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) {
> +        if (errno == ENOSPC) {
> +            return -ENOSPC;
> +        }
>           return -1;

Mixing a return of -1 and negative errno is ugly (as -1 is 
indistinguishable from -EPERM on many, but not all, operating systems). 
If you need to distinguish between different error types, then return 
negative errno in all places.

>       }
>   
> @@ -364,7 +367,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");

Or, if you really want better error messages, it might make sense to 
push errp setting down into fd_write_vmcore() (all callers have to be 
updated).

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