[Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible

[Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible

[Peter Krempa]

The docs for glfs_init suggest that the function sets errno on every
failure. In fact it doesn't. As other functions such as
qemu_gluster_open() in the gluster block code report their errors based
on this fact we need to make sure that errno is set on each failure.

This fixes a crash of qemu-img/qemu when a gluster brick isn't
accessible from given host while the server serving the volume
description is.

Thread 1 (Thread 0x7ffff7fba740 (LWP 203880)):
 #0  0x00007ffff77673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0
 #1  0x0000555555574a68 in qemu_gluster_getlength ()
 #2  0x0000555555565742 in refresh_total_sectors ()
 #3  0x000055555556914f in bdrv_open_common ()
 #4  0x000055555556e8e8 in bdrv_open ()
 #5  0x000055555556f02f in bdrv_open_image ()
 #6  0x000055555556e5f6 in bdrv_open ()
 #7  0x00005555555c5775 in bdrv_new_open ()
 #8  0x00005555555c5b91 in img_info ()
 #9  0x00007ffff62c9c05 in __libc_start_main () from /lib64/libc.so.6
 #10 0x00005555555648ad in _start ()
---

Notes:
    I'm also going to report a bug in libgfapi too.

 block/gluster.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/gluster.c b/block/gluster.c
index 8836085..d0726ec 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -207,6 +207,11 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename,
                          "volume=%s image=%s transport=%s", gconf->server,
                          gconf->port, gconf->volname, gconf->image,
                          gconf->transport);
+
+        /* glfs_init sometimes doesn't set errno although docs suggest that */
+        if (errno == 0)
+            errno = EINVAL;
+
         goto out;
     }
     return glfs;
@@ -482,7 +487,7 @@ static int qemu_gluster_create(const char *filename,

     glfs = qemu_gluster_init(gconf, filename, errp);
     if (!glfs) {
-        ret = -EINVAL;
+        ret = -errno;
         goto out;
     }

-- 
1.9.2

Re: [Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible

[Stefan Hajnoczi]

On Fri, May 09, 2014 at 12:08:10PM +0200, Peter Krempa wrote:
> The docs for glfs_init suggest that the function sets errno on every
> failure. In fact it doesn't. As other functions such as
> qemu_gluster_open() in the gluster block code report their errors based
> on this fact we need to make sure that errno is set on each failure.
> 
> This fixes a crash of qemu-img/qemu when a gluster brick isn't
> accessible from given host while the server serving the volume
> description is.
> 
> Thread 1 (Thread 0x7ffff7fba740 (LWP 203880)):
>  #0  0x00007ffff77673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0
>  #1  0x0000555555574a68 in qemu_gluster_getlength ()
>  #2  0x0000555555565742 in refresh_total_sectors ()
>  #3  0x000055555556914f in bdrv_open_common ()
>  #4  0x000055555556e8e8 in bdrv_open ()
>  #5  0x000055555556f02f in bdrv_open_image ()
>  #6  0x000055555556e5f6 in bdrv_open ()
>  #7  0x00005555555c5775 in bdrv_new_open ()
>  #8  0x00005555555c5b91 in img_info ()
>  #9  0x00007ffff62c9c05 in __libc_start_main () from /lib64/libc.so.6
>  #10 0x00005555555648ad in _start ()
> ---
> 
> Notes:
>     I'm also going to report a bug in libgfapi too.
> 
>  block/gluster.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Please use scripts/checkpatch.pl to check coding style in the future.  I
added {} around the if statement body.  QEMU always uses curlies even
for 1-statement bodies.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible

[Peter Krempa]

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

On 05/09/14 13:39, Stefan Hajnoczi wrote:
> On Fri, May 09, 2014 at 12:08:10PM +0200, Peter Krempa wrote:

...

> 
> Please use scripts/checkpatch.pl to check coding style in the future.  I
> added {} around the if statement body.  QEMU always uses curlies even
> for 1-statement bodies.

Ah, right, sorry about that. I'm used to libvirt's coding style which
allows it. I'll use the patch checker next time.

> 
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block

Thanks.

> 
> Stefan
> 

Peter


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