Re: [Qemu-devel] [RFC PATCH v0 3/3] gluster: Add support for creating zero-filled image

[Jeff Cody]

On Fri, Nov 22, 2013 at 12:46:18PM +0530, Bharata B Rao wrote:
> GlusterFS supports creation of zero-filled file on GlusterFS volume
> by means of an API called glfs_zerofill(). Use this API from QEMU to
> create an image that is filled with zeroes by using the preallocation
> option of qemu-img.
> 
> qemu-img create gluster://server/volume/image -o preallocation=full 10G
> 
> The allowed values for preallocation are 'full' and 'off'. By default
> preallocation is off and image is not zero-filled.
> 
> glfs_zerofill() offloads the writing of zeroes to the server and if
> the storage supports SCSI WRITESAME, GlusterFS server can issue
> BLKZEROOUT ioctl to achieve the zeroing.
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  block/gluster.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 15f5dfb..2368997 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -381,6 +381,29 @@ out:
>      qemu_aio_release(acb);
>      return ret;
>  }
> +
> +static inline int gluster_supports_zerofill(void)
> +{
> +    return 1;
> +}
> +
> +static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
> +        int64_t size)
> +{
> +    return glfs_zerofill(fd, offset, size);
> +}
> +
> +#else
> +static inline int gluster_supports_zerofill(void)
> +{
> +    return 0;
> +}
> +
> +static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
> +        int64_t size)
> +{
> +    return 0;
> +}
>  #endif
>  
>  static int qemu_gluster_create(const char *filename,
> @@ -389,6 +412,7 @@ static int qemu_gluster_create(const char *filename,
>      struct glfs *glfs;
>      struct glfs_fd *fd;
>      int ret = 0;
> +    int prealloc = 0;
>      int64_t total_size = 0;
>      GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
>  
> @@ -401,6 +425,18 @@ static int qemu_gluster_create(const char *filename,
>      while (options && options->name) {
>          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>              total_size = options->value.n / BDRV_SECTOR_SIZE;
> +        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> +            if (!options->value.s || !strcmp(options->value.s, "off")) {
> +                prealloc = 0;
> +            } else if (!strcmp(options->value.s, "full") &&
> +                    gluster_supports_zerofill()) {
> +                prealloc = 1;
> +            } else {
> +                error_setg(errp, "Invalid preallocation mode: '%s'"
> +                    " or GlusterFS doesn't support zerofill API",
> +                           options->value.s);
> +                return -EINVAL;

This leaks gconf.  While probably not a huge deal for a .bdrv_create()
implementation, you should still set ret, and perform a 'goto out' here.

> +            }
>          }
>          options++;
>      }
> @@ -413,6 +449,10 @@ static int qemu_gluster_create(const char *filename,
>          if (glfs_ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
>              ret = -errno;
>          }
> +        if (prealloc && qemu_gluster_zerofill(fd, 0,
> +                total_size * BDRV_SECTOR_SIZE)) {
> +            ret = -errno;
> +        }
>          if (glfs_close(fd) != 0) {
>              ret = -errno;
>          }
> @@ -595,6 +635,11 @@ static QEMUOptionParameter qemu_gluster_create_options[] 
> = {
>          .type = OPT_SIZE,
>          .help = "Virtual disk size"
>      },
> +    {
> +        .name = BLOCK_OPT_PREALLOC,
> +        .type = OPT_STRING,
> +        .help = "Preallocation mode (allowed values: off, on)"
> +    },
>      { NULL }
>  };
>  
> -- 
> 1.7.11.7
> 
>