qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Hu Tao
Subject: Re: [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option
Date: Wed, 10 Sep 2014 10:02:56 +0800
User-agent: Mutt/1.5.21 (2010-09-15)

On Tue, Sep 09, 2014 at 03:21:49PM +0200, Benoît Canet wrote:
> 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 <address@hidden>
> > Reviewed-by: Max Reitz <address@hidden>
> > ---
> >  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.

Looks correct, but left wouldn't be decremented since break breaks out
of while directly. I guess you were looking at the wrong closing
brace. I'll change it providing what you heard.

Regards,
Hu

> 
> (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:
> > address@hidden @code
> > address@hidden preallocation
> > +Preallocation mode (allowed values: @code{off}, @code{falloc}, 
> > @code{full}).
> > address@hidden mode preallocates space for image by calling 
> > posix_fallocate().
> > address@hidden mode preallocates space for image by writing zeros to 
> > underlying
> > +storage.
> > address@hidden 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:
> > address@hidden @code
> > address@hidden preallocation
> > +Preallocation mode (allowed values: @code{off}, @code{falloc}, 
> > @code{full}).
> > address@hidden mode preallocates space for image by calling 
> > posix_fallocate().
> > address@hidden mode preallocates space for image by writing zeros to 
> > underlying
> > +storage.
> > address@hidden 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
> > 
> > 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]