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

2014-05-09 Thread 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 0x77fba740 (LWP 203880)):
 #0  0x777673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0
 #1  0x55574a68 in qemu_gluster_getlength ()
 #2  0x55565742 in refresh_total_sectors ()
 #3  0x5556914f in bdrv_open_common ()
 #4  0x5556e8e8 in bdrv_open ()
 #5  0x5556f02f in bdrv_open_image ()
 #6  0x5556e5f6 in bdrv_open ()
 #7  0x555c5775 in bdrv_new_open ()
 #8  0x555c5b91 in img_info ()
 #9  0x762c9c05 in __libc_start_main () from /lib64/libc.so.6
 #10 0x555648ad 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

2014-05-09 Thread 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 0x77fba740 (LWP 203880)):
  #0  0x777673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0
  #1  0x55574a68 in qemu_gluster_getlength ()
  #2  0x55565742 in refresh_total_sectors ()
  #3  0x5556914f in bdrv_open_common ()
  #4  0x5556e8e8 in bdrv_open ()
  #5  0x5556f02f in bdrv_open_image ()
  #6  0x5556e5f6 in bdrv_open ()
  #7  0x555c5775 in bdrv_new_open ()
  #8  0x555c5b91 in img_info ()
  #9  0x762c9c05 in __libc_start_main () from /lib64/libc.so.6
  #10 0x555648ad 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

2014-05-09 Thread Peter Krempa 
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



signature.asc
Description: OpenPGP digital signature