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

[Eric Blake <eblake@redhat.com>]

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

On 07/19/2014 02:35 PM, Max Reitz wrote:
> Many qemu-img subcommands only read the source file(s) once. For these
> use cases, a full write-back cache is unnecessary and mainly clutters
> host cache memory. Though this is generally no concern as cache memory
> is freely available and can be scaled by the host OS, it may become a
> concern with thin provisioning.
> 
> For these cases, it makes sense to allow users to freely specify the
> source cache mode (e.g. use no cache at all).
> 
> This commit adds a new switch (-T) for the qemu-img subcommands check,
> compare, convert and rebase to specify the cache to be used for source
> images (the backing file in case of rebase).

What mnemonic did you have in mind when choosing -T? Or was it just a
universally available letter for the subcommands you were touching?

> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img-cmds.hx | 16 ++++++------
>  qemu-img.c       | 78 ++++++++++++++++++++++++++++++++++++++++++++------------
>  qemu-img.texi    | 14 +++++++---
>  3 files changed, 80 insertions(+), 28 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index d029609..5613628 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -10,9 +10,9 @@ STEXI
>  ETEXI
>  
>  DEF("check", img_check,
> -    "check [-q] [-f fmt] [--output=ofmt]  [-r [leaks | all]] filename")
> +    "check [-q] [-f fmt] [--output=ofmt]  [-r [leaks | all]] [-T src_cache] filename")

Might be nice to fix the unintentional double space before -r while
touching this line.

>  DEF("convert", img_convert,
> -    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
> +    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")

Oh, maybe you just picked -T for source because -t was already picked
for destination?

At any rate, seems reasonable.
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 --]