Re: [Qemu-devel] [PATCH 2/2] qemu-img: Allow cache mode specification for amend

[Eric Blake <eblake@redhat.com>]

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

On 07/19/2014 02:35 PM, Max Reitz wrote:
> qemu-img amend may extensively modify the target image, depending on the
> options to be amended (e.g. conversion to qcow2 compat level 0.10 from
> 1.1 for an image with many unallocated zero clusters). Therefore it
> makes sense to allow the user to specify the cache mode to be used.

Extensive modifications implies long-running operation - the 'amend'
subcommand is a good candidate for the -p progress meter option.  But
that would be a separate patch.

> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img-cmds.hx |  4 ++--
>  qemu-img.c       | 19 +++++++++++++++----
>  qemu-img.texi    |  2 +-
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 

>  
> +    cache = BDRV_DEFAULT_CACHE;
>      for (;;) {
> -        c = getopt(argc, argv, "hqf:o:");
> +        c = getopt(argc, argv, "hqf:t:o:");
>          if (c == -1) {
>              break;
>          }
> @@ -2805,6 +2807,9 @@ static int img_amend(int argc, char **argv)
>              case 'f':
>                  fmt = optarg;
>                  break;
> +            case 't':
> +                cache = optarg;
> +                break;
>              case 'q':


Pre-existing, so I won't hold up review, but I'm a big fan of having the
switch block in the same order as the getopt string (that is, we listed
'q' before 'f' in the string above, so the cases are out-of-order with
respect to that string).  The fix can go either way (reshuffle the case
statements, or reorder the optstring above).  [and for the truly OCD, I
prefer the optstring in case-insensitive alphabetical order "f:ho:qt:",
because then it's easier to scan the string to see what letters are
still available for new options - but that's asking a bit much]

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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