Re: [Qemu-devel] [PATCH v14 11/14] qemu-img: Specify backing file for commit

[Eric Blake]

On 10/24/2014 07:57 AM, Max Reitz wrote:
> Introduce a new parameter for qemu-img commit which may be used to
> explicitly specify the backing file into which an image should be
> committed if the backing chain has more than a single layer.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  qemu-img-cmds.hx |  4 ++--
>  qemu-img.c       | 32 +++++++++++++++++++++++---------
>  qemu-img.texi    | 12 +++++++++++-
>  3 files changed, 36 insertions(+), 12 deletions(-)
> 

> +If the backing chain of the given image file @var{filename} has more than one
> +layer, the backing file into which the changes will be committed may be
> +specified as @var{base} (which has to be part of @var{filename}'s backing
> +chain). If @var{base} is not specified, the immediate backing file of the top
> +image (which is @var{filename}) will be used. For reasons of consistency,
> +explicitly specifying @var{base} will always imply @code{-d} (otherwise, an
> +image could be committed in an indirect backing file and emptying it might 
> lead
> +to different data being read from it because the intermediate backing chain
> +overrules the commit target).

I wonder if there is any better wording to make it obvious that both
instances of 'it' in the (comment) refer to the further reference of the
top image, and not the closer reference to the indirect backing file. Maybe:

...always imply -d (since emptying an image after committing to an
indirect backing file would lead to different data being read from the
image due to content in the intermediate backing chain overruling the
commit target)

But I can live with the wording as you proposed it, so:

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature