On 09/08/2014 09:54 PM, Hu Tao wrote: > This patch prepares for the subsequent patches. > > Signed-off-by: Hu Tao <hu...@cn.fujitsu.com> > Reviewed-by: Max Reitz <mre...@redhat.com> > Reviewed-by: Kevin Wolf <kw...@redhat.com> > --- > 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 <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature