Re: [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option

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

The Tuesday 09 Sep 2014 à 11:54:30 (+0800), Hu Tao wrote :
> This patch adds a new option preallocation for raw format, and implements
> falloc and full preallocation.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-posix.c | 93 +++++++++++++++++++++++++++++++++++++++++++------------
>  qemu-doc.texi     |  9 ++++++
>  qemu-img.texi     |  9 ++++++
>  3 files changed, 92 insertions(+), 19 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 7208c05..27c854c 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -30,6 +30,7 @@
>  #include "block/thread-pool.h"
>  #include "qemu/iov.h"
>  #include "raw-aio.h"
> +#include "qapi/util.h"
>  
>  #if defined(__APPLE__) && (__MACH__)
>  #include <paths.h>
> @@ -1365,6 +1366,9 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      int result = 0;
>      int64_t total_size = 0;
>      bool nocow = false;
> +    PreallocMode prealloc;
> +    char *buf = NULL;
> +    Error *local_err = NULL;
>  
>      strstart(filename, "file:", &filename);
>  
> @@ -1372,37 +1376,83 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>      total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
>                            BDRV_SECTOR_SIZE);
>      nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false);
> +    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> +    prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> +                               PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> +                               &local_err);
> +    g_free(buf);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        result = -EINVAL;
> +        goto out;
> +    }
>  
>      fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
>                     0644);
>      if (fd < 0) {
>          result = -errno;
>          error_setg_errno(errp, -result, "Could not create file");
> -    } else {
> -        if (nocow) {
> +        goto out;
> +    }
> +
> +    if (nocow) {
>  #ifdef __linux__
> -            /* Set NOCOW flag to solve performance issue on fs like btrfs.
> -             * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
> -             * will be ignored since any failure of this operation should not
> -             * block the left work.
> -             */
> -            int attr;
> -            if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> -                attr |= FS_NOCOW_FL;
> -                ioctl(fd, FS_IOC_SETFLAGS, &attr);
> -            }
> -#endif
> +        /* Set NOCOW flag to solve performance issue on fs like btrfs.
> +         * This is an optimisation. The FS_IOC_SETFLAGS ioctl return value
> +         * will be ignored since any failure of this operation should not
> +         * block the left work.
> +         */
> +        int attr;
> +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> +            attr |= FS_NOCOW_FL;
> +            ioctl(fd, FS_IOC_SETFLAGS, &attr);
>          }
> +#endif
> +    }
>  
> -        if (ftruncate(fd, total_size) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not resize file");
> +    if (ftruncate(fd, total_size) != 0) {
> +        result = -errno;
> +        error_setg_errno(errp, -result, "Could not resize file");
> +        goto out_close;
> +    }
> +
> +    if (prealloc == PREALLOC_MODE_FALLOC) {
> +        /* posix_fallocate() doesn't set errno. */
> +        result = -posix_fallocate(fd, 0, total_size);
> +        if (result != 0) {
> +            error_setg_errno(errp, -result,
> +                             "Could not preallocate data for the new file");

Here you choose not to goto out_close but let the code flow lead to it.

>          }

> -        if (qemu_close(fd) != 0) {
> -            result = -errno;
> -            error_setg_errno(errp, -result, "Could not close the new file");
> +    } else if (prealloc == PREALLOC_MODE_FULL) {
> +        buf = g_malloc0(65536);
> +        int64_t num = 0, left = total_size;
> +
> +        while (left > 0) {
> +            num = MIN(left, 65536);
> +            result = write(fd, buf, num);
> +            if (result < 0) {
> +                result = -errno;
> +                error_setg_errno(errp, -result,
> +                                 "Could not write to the new file");

> +                g_free(buf);
> +                goto out_close;

As a consequence you could replace the two previous line by:

+                break;

The while loop would gently exit left would be decremented and fsync would
be done but we don't care these sides effects and buf would be freed.
Then the code would exit the if branch and reach out_close.

(I heard some people don't like exiting a loop with a goto).


> +            }
> +            left -= num;
>          }
> +        fsync(fd);
> +        g_free(buf);
> +    } else if (prealloc != PREALLOC_MODE_OFF) {
> +        result = -EINVAL;
> +        error_setg(errp, "Unsupported preallocation mode: %s",
> +                   PreallocMode_lookup[prealloc]);
>      }
> +
> +out_close:
> +    if (qemu_close(fd) != 0 && result == 0) {
> +        result = -errno;
> +        error_setg_errno(errp, -result, "Could not close the new file");
> +    }
> +out:
>      return result;
>  }
>  
> @@ -1585,6 +1635,11 @@ static QemuOptsList raw_create_opts = {
>              .type = QEMU_OPT_BOOL,
>              .help = "Turn off copy-on-write (valid only on btrfs)"
>          },
> +        {
> +            .name = BLOCK_OPT_PREALLOC,
> +            .type = QEMU_OPT_STRING,
> +            .help = "Preallocation mode (allowed values: off, falloc, full)"
> +        },
>          { /* end of list */ }
>      }
>  };
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 2b232ae..1f289d6 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -527,6 +527,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
>  space. Use @code{qemu-img info} to know the real size used by the
>  image or @code{ls -ls} on Unix/Linux.
>  
> +Supported options:
> +@table @code
> +@item preallocation
> +Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
> +@code{falloc} mode preallocates space for image by calling posix_fallocate().
> +@code{full} mode preallocates space for image by writing zeros to underlying
> +storage.
> +@end table
> +
>  @item qcow2
>  QEMU image format, the most versatile format. Use it to have smaller
>  images (useful if your filesystem does not supports holes, for example
> diff --git a/qemu-img.texi b/qemu-img.texi
> index cc4668e..d64d05e 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -419,6 +419,15 @@ Linux or NTFS on Windows), then only the written sectors will reserve
>  space. Use @code{qemu-img info} to know the real size used by the
>  image or @code{ls -ls} on Unix/Linux.
>  
> +Supported options:
> +@table @code
> +@item preallocation
> +Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).
> +@code{falloc} mode preallocates space for image by calling posix_fallocate().
> +@code{full} mode preallocates space for image by writing zeros to underlying
> +storage.
> +@end table
> +
>  @item qcow2
>  QEMU image format, the most versatile format. Use it to have smaller
>  images (useful if your filesystem does not supports holes, for example
> -- 
> 1.9.3
> 
>