[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v13 4/6] qapi: introduce PreallocMode and a new
From: |
Hu Tao |
Subject: |
Re: [Qemu-devel] [PATCH v13 4/6] qapi: introduce PreallocMode and a new PreallocMode full. |
Date: |
Wed, 3 Sep 2014 09:31:22 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Sep 02, 2014 at 11:32:50PM +0200, Max Reitz wrote:
> On 29.08.2014 10:33, Hu Tao wrote:
> >This patch prepares for the subsequent patches.
> >
> >Signed-off-by: Hu Tao <address@hidden>
> >---
> > block/qcow2.c | 23 +++++++++++++++--------
> > qapi/block-core.json | 16 ++++++++++++++++
> > tests/qemu-iotests/049.out | 2 +-
> > 3 files changed, 32 insertions(+), 9 deletions(-)
> >
> >diff --git a/block/qcow2.c b/block/qcow2.c
> >index cf27c3f..95fb240 100644
> >--- a/block/qcow2.c
> >+++ b/block/qcow2.c
> >@@ -30,6 +30,7 @@
> > #include "qemu/error-report.h"
> > #include "qapi/qmp/qerror.h"
> > #include "qapi/qmp/qbool.h"
> >+#include "qapi/util.h"
> > #include "trace.h"
> > #include "qemu/option_int.h"
> >@@ -1738,7 +1739,7 @@ static int preallocate(BlockDriverState *bs)
> > static int qcow2_create2(const char *filename, int64_t total_size,
> > const char *backing_file, const char
> > *backing_format,
> >- int flags, size_t cluster_size, int prealloc,
> >+ int flags, size_t cluster_size, PreallocMode
> >prealloc,
> > QemuOpts *opts, int version,
> > Error **errp)
> > {
> >@@ -1915,7 +1916,7 @@ static int qcow2_create(const char *filename, QemuOpts
> >*opts, Error **errp)
> > uint64_t size = 0;
> > int flags = 0;
> > size_t cluster_size = DEFAULT_CLUSTER_SIZE;
> >- int prealloc = 0;
> >+ PreallocMode prealloc = PREALLOC_MODE_OFF;
> > int version = 3;
> > Error *local_err = NULL;
> > int ret;
> >@@ -1931,12 +1932,11 @@ static int qcow2_create(const char *filename,
> >QemuOpts *opts, Error **errp)
> > cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
> > DEFAULT_CLUSTER_SIZE);
> > buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> >- if (!buf || !strcmp(buf, "off")) {
> >- prealloc = 0;
> >- } else if (!strcmp(buf, "metadata")) {
> >- prealloc = 1;
> >- } else {
> >- error_setg(errp, "Invalid preallocation mode: '%s'", buf);
> >+ prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> >+ PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
> >+ &local_err);
> >+ if (local_err) {
> >+ error_propagate(errp, local_err);
> > ret = -EINVAL;
> > goto finish;
> > }
> >@@ -1958,6 +1958,13 @@ static int qcow2_create(const char *filename,
> >QemuOpts *opts, Error **errp)
> > flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
> > }
> >+ if (prealloc && prealloc != PREALLOC_MODE_METADATA) {
> >+ ret = -1;
>
> Since the return value is expected to be -errno, I'd propose "ret =
> -EINVAL;" here. With that fixed (or a good explanation why we want
> -1 here):
Good.
>
> Reviewed-by: Max Reitz <address@hidden>
>
> >+ error_setg(errp, "Unsupported preallocate mode: %s",
> >+ PreallocMode_lookup[prealloc]);
> >+ goto finish;
> >+ }
> >+
> > if (backing_file && prealloc) {
> > error_setg(errp, "Backing file and preallocation cannot be used at
> > "
> > "the same time");
> >diff --git a/qapi/block-core.json b/qapi/block-core.json
> >index fb74c56..543b00b 100644
> >--- a/qapi/block-core.json
> >+++ b/qapi/block-core.json
> >@@ -1679,3 +1679,19 @@
> > 'len' : 'int',
> > 'offset': 'int',
> > 'speed' : 'int' } }
> >+
> >+# @PreallocMode
> >+#
> >+# Preallocation mode of QEMU image file
> >+#
> >+# @off: no preallocation
> >+# @metadata: preallocate only for metadata
> >+# @full: preallocate all data by calling posix_fallocate() if it is
> >+# available, otherwise by writing zeros to device to ensure disk
> >+# space is really available. @full preallocation also sets up
> >+# metadata correctly.
> >+#
> >+# Since 2.2
> >+##
> >+{ 'enum': 'PreallocMode',
> >+ 'data': [ 'off', 'metadata', 'full' ] }
> >diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
> >index 71ca44d..09ca0ae 100644
> >--- a/tests/qemu-iotests/049.out
> >+++ b/tests/qemu-iotests/049.out
> >@@ -179,7 +179,7 @@ qemu-img create -f qcow2 -o preallocation=metadata
> >TEST_DIR/t.qcow2 64M
> > Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off
> > cluster_size=65536 preallocation='metadata' lazy_refcounts=off
> > qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M
> >-qemu-img: TEST_DIR/t.qcow2: Invalid preallocation mode: '1234'
> >+qemu-img: TEST_DIR/t.qcow2: invalid parameter value: 1234
> > Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off
> > cluster_size=65536 preallocation='1234' lazy_refcounts=off
> > == Check encryption option ==