[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation
From: |
Hu Tao |
Subject: |
Re: [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option |
Date: |
Wed, 25 Jun 2014 14:04:25 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sat, Jun 14, 2014 at 09:38:30PM +0200, Max Reitz wrote:
> On 12.06.2014 05:54, Hu Tao wrote:
> >This patch adds a new option preallocation for raw format, and implements
> >full preallocation.
> >
> >Signed-off-by: Hu Tao <address@hidden>
> >---
> > block/raw-posix.c | 59
> > ++++++++++++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 52 insertions(+), 7 deletions(-)
> >
> >diff --git a/block/raw-posix.c b/block/raw-posix.c
> >index 35057f0..73b10cd 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>
> >@@ -1279,6 +1280,8 @@ static int raw_create(const char *filename,
> >QEMUOptionParameter *options,
> > int fd;
> > int result = 0;
> > int64_t total_size = 0;
> >+ PreallocMode prealloc = PREALLOC_MODE_OFF;
> >+ Error *error = NULL;
> > strstart(filename, "file:", &filename);
> >@@ -1286,6 +1289,14 @@ static int raw_create(const char *filename,
> >QEMUOptionParameter *options,
> > while (options && options->name) {
> > if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> > total_size = ROUND_UP(options->value.n, BDRV_SECTOR_SIZE);
> >+ } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> >+ prealloc = qapi_enum_parse(PreallocMode_lookup,
> >options->value.s,
> >+ PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> >+ &error);
> >+ if (error) {
> >+ error_propagate(errp, error);
> >+ return -EINVAL;
>
> Could be "result = -EINVAL; goto out;" instead, but definitely isn't
> wrong this way.
>
> >+ }
> > }
> > options++;
> > }
> >@@ -1295,16 +1306,45 @@ static int raw_create(const char *filename,
> >QEMUOptionParameter *options,
> > if (fd < 0) {
> > result = -errno;
> > error_setg_errno(errp, -result, "Could not create file");
> >- } else {
> >- if (ftruncate(fd, total_size) != 0) {
> >- result = -errno;
> >- error_setg_errno(errp, -result, "Could not resize file");
> >+ goto out;
> >+ }
> >+ if (ftruncate(fd, total_size) != 0) {
> >+ result = -errno;
> >+ error_setg_errno(errp, -result, "Could not resize file");
> >+ goto out_close;
> >+ }
> >+ if (prealloc == PREALLOC_MODE_METADATA) {
> >+ /* 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");
> > }
> >- if (qemu_close(fd) != 0) {
> >- result = -errno;
> >- error_setg_errno(errp, -result, "Could not close the new file");
> >+ } else if (prealloc == PREALLOC_MODE_FULL) {
> >+ char *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;
> >+ }
> >+ left -= num;
> > }
>
> Technically, a raw file does not have any metadata, therefore
> preallocation=metadata is a bit ambiguous. I'd accept both
> allocating nothing and allocating everything; you chose the latter,
> which is fine.
qcow2's behaviour of metadata preallocation depends on raw's, this is
why I chose the latter here.
>
> However, why do you implement the preallocation differently for
> preallocation=full, then? posix_fallocate() does not seem to change
> the contents of the areas which were newly allocated; and the
> ftruncate() before made sure they are read back as zeroes.
> Therefore, using ftruncate() and then posix_fallocate() seems to be
> functionally equivalent to writing zeroes.
The difference is posix_fallocate() reserves space but ftruncate()
doesn't, as said in man page:
After a successful call to posix_fallocate(), subsequent writes
to bytes in the specified range are guaranteed not to fail because
of lack of disk space.
however, posix_fallocate() doesn't guarantee this in other cases like
thin provisioning, this is why preallocation=metadata is different than
preallocation=full.
Hu
- [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full., (continued)
- [Qemu-devel] [PATCH v10 4/6] qapi: introduce PreallocMode and a new PreallocMode full., Hu Tao, 2014/06/11
- [Qemu-devel] [PATCH v10 3/6] rename parse_enum_option to qapi_enum_parse and make it public, Hu Tao, 2014/06/11
- [Qemu-devel] [PATCH v10 2/6] raw, qcow2: don't convert file size to sector size, Hu Tao, 2014/06/11
- [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option, Hu Tao, 2014/06/11
- [Qemu-devel] [PATCH v10 6/6] qcow2: Add full image preallocation option, Hu Tao, 2014/06/11