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 <hu...@cn.fujitsu.com>
> > Reviewed-by: Max Reitz <mre...@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.
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:
> > +@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
> >
> >
