[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new Pr
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc. |
Date: |
Tue, 09 Sep 2014 06:42:13 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 |
On 09/08/2014 09:54 PM, Hu Tao wrote:
> This patch prepares for the subsequent patches.
>
> Signed-off-by: Hu Tao <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> Reviewed-by: Kevin Wolf <address@hidden>
> ---
> block/qcow2.c | 23 +++++++++++++++--------
> qapi/block-core.json | 17 +++++++++++++++++
> tests/qemu-iotests/049.out | 2 +-
> 3 files changed, 33 insertions(+), 9 deletions(-)
>
> 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 = -EINVAL;
> + error_setg(errp, "Unsupported preallocate mode: %s",
> + PreallocMode_lookup[prealloc]);
> + goto finish;
> + }
I _still_ think this looks weird, and would be better as either:
if (prealloc != PREALLOC_MODE_NONE &&
prealloc != PREALLOC_MODE_METADATA) {
to make it obvious that you are filtering for two acceptable modes, or as:
if (prealloc == PREALLOC_MODE_FALLOC ||
prealloc == PREALLOC_MODE_FULL) {
to make it obvious the modes that you do not support. But my complaint
is not strong enough to prevent this patch, especially if later in the
series revisits this code.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v14 0/5] qcow2, raw: add preallocation=full and preallocation=falloc, Hu Tao, 2014/09/08
- [Qemu-devel] [PATCH v14 1/5] block: round up file size to nearest sector, Hu Tao, 2014/09/08
- [Qemu-devel] [PATCH v14 2/5] block: don't convert file size to sector size, Hu Tao, 2014/09/08
- [Qemu-devel] [PATCH v14 3/5] qapi: introduce PreallocMode and new PreallocModes full and falloc., Hu Tao, 2014/09/08
- [Qemu-devel] [PATCH v14 4/5] raw-posix: Add falloc and full preallocation option, Hu Tao, 2014/09/08
- [Qemu-devel] [PATCH v14 5/5] qcow2: Add falloc and full preallocation option, Hu Tao, 2014/09/08