diff mbox

[PULL,12/17] gluster: Correctly propagate errors when volume isn't accessible

Message ID 1399662217-31148-13-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi May 9, 2014, 7:03 p.m. UTC 
From: Peter Krempa <pkrempa@redhat.com>

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 ()

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/gluster.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Eric Blake May 12, 2014, 4:07 p.m. UTC | #1 
On 05/09/2014 01:03 PM, Stefan Hajnoczi wrote:
> From: Peter Krempa <pkrempa@redhat.com>
> 
> 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 ()
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

Isn't this missing S-o-b from Peter?
Stefan Hajnoczi May 14, 2014, 12:36 p.m. UTC | #2 
On Mon, May 12, 2014 at 10:07:07AM -0600, Eric Blake wrote:
> On 05/09/2014 01:03 PM, Stefan Hajnoczi wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> > 
> > 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 ()
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> 
> Isn't this missing S-o-b from Peter?

Good catch.  Peter, can you send your Signed-off-by: please?

Stefan
Peter Krempa May 14, 2014, 12:52 p.m. UTC | #3 
On 05/14/14 14:36, Stefan Hajnoczi wrote:
> On Mon, May 12, 2014 at 10:07:07AM -0600, Eric Blake wrote:
>> On 05/09/2014 01:03 PM, Stefan Hajnoczi wrote:
>>> From: Peter Krempa <pkrempa@redhat.com>
>>>
>>> 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 ()
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>
>> Isn't this missing S-o-b from Peter?
> 
> Good catch.  Peter, can you send your Signed-off-by: please?
> 
> Stefan
> 

As this patch was already pulled to master as:

commit 4557117d9eed8cadc360aec23b42fc39a7011864
Author: Peter Krempa <pkrempa@redhat.com>
Date:   Fri May 9 12:08:10 2014 +0200

    gluster: Correctly propagate errors when volume isn't accessible
    
    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 ()

    
    Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

I'm explicitly expressing my signoff here:

Signed-off-by: Peter Krempa <pkrempa@redhat.com>

Sorry for the hassle.

Peter
diff mbox

Patch

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;
     }