Re: [Qemu-devel] [PATCH RFC 2/3] qemu-img: Add --unsafe-read option to subcommands

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

On 03/13/2017 09:39 PM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  qemu-img.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 114 insertions(+), 34 deletions(-)
> 

> @@ -2711,9 +2751,10 @@ static int img_map(int argc, char **argv)
>              {"output", required_argument, 0, OPTION_OUTPUT},
>              {"object", required_argument, 0, OPTION_OBJECT},
>              {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> +            {"unsafe-read", no_argument, 0, 'U'},
>              {0, 0, 0, 0}
>          };
> -        c = getopt_long(argc, argv, "f:h",
> +        c = getopt_long(argc, argv, "f:hU",

Yay for 'U' being an unambiguous mnemonic that we could use on all the
commands.  You got lucky (it gets ever harder to add new options, as our
letters get thinner ;)

> @@ -2970,6 +3020,7 @@ static int img_rebase(int argc, char **argv)
>      int c, flags, src_flags, ret;
>      bool writethrough, src_writethrough;
>      int unsafe = 0;
> +    int unsafe_read = 0;

bool, please.  (Just because existing code is lousy with using an int
for a bool doesn't mean we should perpetuate it)


> @@ -3033,6 +3085,9 @@ static int img_rebase(int argc, char **argv)
>          case OPTION_IMAGE_OPTS:
>              image_opts = true;
>              break;
> +        case 'U':
> +            unsafe_read = 1;

s/1/true/

Good patch, but missing documentation updates to --help/man page.  So,
as encouragement to add that when you drop the RFC, I'll withhold my R-b
until then. ;)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]