Re: [Qemu-devel] [PATCH v2][RFC] Add compare subcommand for qemu-img

[Eric Blake]

On 08/03/2012 12:45 AM, Miroslav Rezanina wrote:
> This is second version of  patch adding compare subcommand that compares two
> images. Compare has following criteria:
>  - only data part is compared
>  - unallocated sectors are not read
>  - in case of different image size, exceeding part of bigger disk has
>    to be zeroed/unallocated to compare rest
>  - qemu-img returns:
>     - 0 if images are identical
>     - 1 if images differ
>     - 2 on error
> 
> v2:
>  - changed option for second image format to -F
>  - changed handlig of -f and -F [1]
>  - added strict mode (-s)
>  - added quiet mode (-q)
>  - improved output messages [2]
>  - rename variables for larger image handling
>  - added man page content
> 
> [1] Original patch handling was as following:
>  i)   neither -f nor -F  - both images probed for type
>  ii)  -f only            - both images use specified type
>  iii) -F only            - first image probed, second image use specified type
>  iii) -f and -F          - first image use -f type, second use -F type
> 
> This patch change behavior in way that case ii) and iii) has same efect - we
> use specified value for both images.

I still think orthogonality is better than applying one option to both
files.  Probing is sometimes useful, and you have left no way to probe
one file but not the other.

> 
> [2] When we hit different sector we print its number out.
> 
> Points to dicuss:
> 
> i) Handling -f/-F options.
> Currently we have three scenarios - no option
> specified - probe all, one of options specified - use it for both, both option
> specified - use each value for related image. This behavior is based on idea
> that we can use format probing for all images or specify format for all 
> images.
> This preserve state when -f fmt specify input image format (compare is only
> subcomand with more than one input image except convert that uses multiple
> images without possibility to specify different format for each image).
> 
> However, there's one more behavior to be considered - to use -f/-F for one
> image only - when only one option is provided, only appropriate image use 
> specified
> format, second one is probed.

I would prefer this, as it would let me compare against a file of
unknown type.


> +++ b/qemu-img-cmds.hx
> @@ -27,6 +27,12 @@ STEXI
>  @item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
>  ETEXI
>  
> +DEF("compare", img_compare,
> +    "compare [-f fmt] [-g fmt] [-p] filename1 filename2")

Out of date with the rest of your patch.

> +STEXI
> address@hidden compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] 
> @var{filename1} @var{filename2}
> +ETEXI
> +
>  DEF("convert", img_convert,
>      "convert [-c] [-p] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s 
> snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
>  STEXI
> diff --git a/qemu-img.c b/qemu-img.c
> index 80cfb9b..6722fa0 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -96,7 +96,11 @@ static void help(void)
>             "  '-a' applies a snapshot (revert disk to saved state)\n"
>             "  '-c' creates a snapshot\n"
>             "  '-d' deletes a snapshot\n"
> -           "  '-l' lists all snapshots in the given image\n";
> +           "  '-l' lists all snapshots in the given image\n"
> +           "Parameters to compare subcommand:\n"
> +           "  '-F' Second image format (in case it differs from first 
> image)\n"

If you make -f and -F orthogonal, applying to one image each, this might
be better worded as:

'-F' Second image format (-f applies only to first image)\n

or even just

'-F' Second image format


> +/*
> + * Compares two images. Exit codes:
> + *
> + * 0 - Images are identical
> + * 1 - Images differ
> + * 2 - Error occured

s/occured/occurred/


> +++ b/qemu-img.texi
> @@ -67,6 +67,18 @@ deletes a snapshot
>  lists all snapshots in the given image
>  @end table
>  
> +Parameters to compare subcommand:
> +
> address@hidden @option
> +
> address@hidden -F
> +Second image format (in case it differs from first image)

Another instance of wording to be careful of.

> @@ -100,6 +112,27 @@ it doesn't need to be specified separately in this case.
>  
>  Commit the changes recorded in @var{filename} in its base image.
>  
> address@hidden compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] 
> @var{filename1} @var{filename2}
> +
> +Compare content of two images. You can compare images with different format 
> or
> +settings.
> +
> +Format is probed unless you specify it by @var{-f} and/or @var{-F} option.
> +If only one of these options is specified, it is used for both images.
> +If both options are specfied, @var{-f} is used for @var{filename1} and
> address@hidden for @var{filename2}.
> +
> +By default, compare evaluate as identical images with different size where

s/evaluate/evaluates/

> +bigger image contains only unallocated and/or zeroed sectors in area above
> +second image size. In addition, if any sector is not allocated in one image
> +and contains only zero bytes in second, it is evaluated as equal. You can use
> +Strict mode by specifying @var{-s} option. When compare runs in Strict mode,
> +it fails in case image size differs or sector is allocated in one image and
> +is not allocated in second.
> +
> +In case you want to suppress any non-error output, you can use Quiet mode by
> +specifying @var{-q} option.

When -q is not in use, what gets output?  Is it like cmp(1), where
output is silent on the same, and lists the location of the first
differing byte on different?

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature