[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/6] qemu-img convert: Support multiple -o op
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/6] qemu-img convert: Support multiple -o options |
Date: |
Thu, 20 Feb 2014 14:33:30 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, Feb 20, 2014 at 03:57:20PM +0100, Kevin Wolf wrote:
> Instead of ignoring all option values but the last one, multiple -o
> options now have the same meaning as having a single option with all
> settings in the order of their respective -o options.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> qemu-img.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 770e4e6..ba6e82d 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1160,6 +1160,9 @@ static int img_convert(int argc, char **argv)
> Error *local_err = NULL;
> QemuOpts *sn_opts = NULL;
>
> + /* Initialize before goto out */
> + qemu_progress_init(progress, 1.0);
> +
> fmt = NULL;
> out_fmt = "raw";
> cache = "unsafe";
> @@ -1191,13 +1194,21 @@ static int img_convert(int argc, char **argv)
> case 'e':
> error_report("option -e is deprecated, please use \'-o "
> "encryption\' instead!");
> - return 1;
> + ret = -1;
> + goto out;
> case '6':
> error_report("option -6 is deprecated, please use \'-o "
> "compat6\' instead!");
> - return 1;
> + ret = -1;
> + goto out;
> case 'o':
> - options = optarg;
> + if (!options) {
> + options = g_strdup(optarg);
> + } else {
> + char *old_options = options;
> + options = g_strdup_printf("%s,%s", options, optarg);
> + g_free(old_options);
> + }
> break;
> case 's':
> snapshot_name = optarg;
> @@ -1208,7 +1219,8 @@ static int img_convert(int argc, char **argv)
> if (!sn_opts) {
> error_report("Failed in parsing snapshot param '%s'",
> optarg);
> - return 1;
> + ret = -1;
> + goto out;
> }
> } else {
> snapshot_name = optarg;
> @@ -1221,7 +1233,8 @@ static int img_convert(int argc, char **argv)
> sval = strtosz_suffix(optarg, &end, STRTOSZ_DEFSUFFIX_B);
> if (sval < 0 || *end) {
> error_report("Invalid minimum zero buffer size for sparse
> output specified");
> - return 1;
> + ret = -1;
> + goto out;
> }
There is a 'return -1' that is missed, that I think needs to be 'goto
out;' for cleanup. It is right after the call to
bdrv_parse_cache_flags() in the img_convert() function:
ret = bdrv_parse_cache_flags(cache, &flags);
if (ret < 0) {
error_report("Invalid cache option: %s", cache);
return -1;
}
Looks like this has been this way for a while, though. And this is an
instance (the only instance) where img_convert returns -1 instead of 0
or 1. I'm not sure the implications if that changed, and became '1',
so we'd probably want to preserve the return value.
Since this doesn't have anything to do with this patch (although
leaking 'options' now, in addition to the other stuff leaked), I'll go
ahead and give my reviewed-by, and we can fix this in another
follow-up patch:
Reviewed-by: Jeff Cody <address@hidden>
>
> min_sparse = sval / BDRV_SECTOR_SIZE;
> @@ -1253,10 +1266,7 @@ static int img_convert(int argc, char **argv)
>
> out_filename = argv[argc - 1];
>
> - /* Initialize before goto out */
> - qemu_progress_init(progress, 1.0);
> -
> - if (options && is_help_option(options)) {
> + if (options && has_help_option(options)) {
> ret = print_block_option_help(out_filename, out_fmt);
> goto out;
> }
> @@ -1649,6 +1659,7 @@ out:
> free_option_parameters(create_options);
> free_option_parameters(param);
> qemu_vfree(buf);
> + g_free(options);
> if (sn_opts) {
> qemu_opts_del(sn_opts);
> }
> --
> 1.8.1.4
>
>
- [Qemu-devel] [PATCH v2 0/6] qemu-img: Support multiple -o options, Kevin Wolf, 2014/02/20
- [Qemu-devel] [PATCH v2 1/6] qemu-option: Introduce has_help_option(), Kevin Wolf, 2014/02/20
- [Qemu-devel] [PATCH v2 2/6] qemu-img create: Support multiple -o options, Kevin Wolf, 2014/02/20
- [Qemu-devel] [PATCH v2 3/6] qemu-img convert: Support multiple -o options, Kevin Wolf, 2014/02/20
- [Qemu-devel] [PATCH v2 4/6] qemu-img amend: Support multiple -o options, Kevin Wolf, 2014/02/20
- [Qemu-devel] [PATCH v2 6/6] qemu-iotests: Check qemu-img command line parsing, Kevin Wolf, 2014/02/20
- [Qemu-devel] [PATCH v2 5/6] qemu-img: Allow -o help with incomplete argument list, Kevin Wolf, 2014/02/20