Re: [Qemu-devel] [PATCH 1/2] block: gluster - code movements, state storage changes

["Benoît Canet" <benoit.canet@irqsave.net>]

Le Tuesday 04 Feb 2014 à 14:26:58 (-0500), Jeff Cody a écrit :
> In preparation for supporting reopen on gluster, move flag
> parsing out to a function.  Also, store open_flags and filename
> in the gluster state storage struct, and add a NULL check in the
> gconf cleanup.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/gluster.c | 48 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index a009b15..79af3fd 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -30,6 +30,8 @@ typedef struct GlusterAIOCB {
>  typedef struct BDRVGlusterState {
>      struct glfs *glfs;
>      struct glfs_fd *fd;
> +    int open_flags;
> +    char *filename;
>  } BDRVGlusterState;
>  
>  #define GLUSTER_FD_READ  0
> @@ -45,11 +47,13 @@ typedef struct GlusterConf {
>  
>  static void qemu_gluster_gconf_free(GlusterConf *gconf)
>  {
> -    g_free(gconf->server);
> -    g_free(gconf->volname);
> -    g_free(gconf->image);
> -    g_free(gconf->transport);
> -    g_free(gconf);
> +    if (gconf) {
> +        g_free(gconf->server);
> +        g_free(gconf->volname);
> +        g_free(gconf->image);
> +        g_free(gconf->transport);
> +        g_free(gconf);
> +    }
>  }
>  
>  static int parse_volume_options(GlusterConf *gconf, char *path)
> @@ -269,11 +273,27 @@ static QemuOptsList runtime_opts = {
>      },
>  };
>  
> +static void qemu_gluster_parse_flags(int bdrv_flags, int *open_flags)
> +{
> +    assert(open_flags != NULL);
> +
> +    *open_flags |= O_BINARY;
> +
> +    if (bdrv_flags & BDRV_O_RDWR) {
> +        *open_flags |= O_RDWR;
> +    } else {
> +        *open_flags |= O_RDONLY;
> +    }
> +
> +    if ((bdrv_flags & BDRV_O_NOCACHE)) {
> +        *open_flags |= O_DIRECT;
> +    }
> +}

I saw the enable-O_SYNC option here.
http://www.gluster.org/community/documentation/index.php/Translators/performance
Why the gluster driver does not allow to enable O_SYNC ?

> +
>  static int qemu_gluster_open(BlockDriverState *bs,  QDict *options,
>                               int bdrv_flags, Error **errp)
>  {
>      BDRVGlusterState *s = bs->opaque;
> -    int open_flags = O_BINARY;
>      int ret = 0;
>      GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
>      QemuOpts *opts;
> @@ -291,23 +311,17 @@ static int qemu_gluster_open(BlockDriverState *bs,  QDict *options,
>  
>      filename = qemu_opt_get(opts, "filename");
>  
> +    s->filename = g_strdup(filename);
> +
>      s->glfs = qemu_gluster_init(gconf, filename);
>      if (!s->glfs) {
>          ret = -errno;
>          goto out;
>      }
>  
> -    if (bdrv_flags & BDRV_O_RDWR) {
> -        open_flags |= O_RDWR;
> -    } else {
> -        open_flags |= O_RDONLY;
> -    }
> -
> -    if ((bdrv_flags & BDRV_O_NOCACHE)) {
> -        open_flags |= O_DIRECT;
> -    }
> +    qemu_gluster_parse_flags(bdrv_flags, &s->open_flags);
>  
> -    s->fd = glfs_open(s->glfs, gconf->image, open_flags);
> +    s->fd = glfs_open(s->glfs, gconf->image, s->open_flags);
>      if (!s->fd) {
>          ret = -errno;
>      }
> @@ -324,6 +338,7 @@ out:
>      if (s->glfs) {
>          glfs_fini(s->glfs);
>      }
> +    g_free(s->filename);
>      return ret;
>  }
>  
> @@ -589,6 +604,7 @@ static void qemu_gluster_close(BlockDriverState *bs)
>          s->fd = NULL;
>      }
>      glfs_fini(s->glfs);
> +    g_free(s->filename);
>  }
>  
>  static int qemu_gluster_has_zero_init(BlockDriverState *bs)
> -- 
> 1.8.3.1
> 
>