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

[Miroslav Rezanina]

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.

[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.

ii) How to handle images with different size.
If size of images is different and strict mode is not used, addditional size of
bigger image is checked to be zeroed/unallocated. This version do this check
before rest of image is compared. This is done to not compare whole image in
case that one of images is only expanded copy of other.

Paolo Bonzini proposed to do this check after compare shared size of images to
go through image sequentially.

Signed-off-by: Miroslav Rezanina <address@hidden>
---
 block.c          |   39 ++++++++
 block.h          |    3 +-
 qemu-img-cmds.hx |    6 +
 qemu-img.c       |  277 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 qemu-img.texi    |   33 +++++++
 5 files changed, 356 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index b38940b..3558bf9 100644
--- a/block.c
+++ b/block.c
@@ -2284,6 +2284,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 
 typedef struct BdrvCoIsAllocatedData {
     BlockDriverState *bs;
+    BlockDriverState *base;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;
@@ -2414,6 +2415,44 @@ int coroutine_fn 
bdrv_co_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
+/* Coroutine wrapper for bdrv_is_allocated_above() */
+static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
+{
+    BdrvCoIsAllocatedData *data = opaque;
+    BlockDriverState *top = data->bs;
+    BlockDriverState *base = data->base;
+
+    data->ret = bdrv_co_is_allocated_above(top, base, data->sector_num,
+                                           data->nb_sectors, data->pnum);
+    data->done = true;
+}
+
+/*
+ * Synchronous wrapper around bdrv_co_is_allocated_above().
+ *
+ * See bdrv_co_is_allocated_above() for details.
+ */
+int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+                      int64_t sector_num, int nb_sectors, int *pnum)
+{
+    Coroutine *co;
+    BdrvCoIsAllocatedData data = {
+        .bs = top,
+        .base = base,
+        .sector_num = sector_num,
+        .nb_sectors = nb_sectors,
+        .pnum = pnum,
+        .done = false,
+    };
+
+    co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry);
+    qemu_coroutine_enter(co, &data);
+    while (!data.done) {
+        qemu_aio_wait();
+    }
+    return data.ret;
+}
+
 BlockInfoList *qmp_query_block(Error **errp)
 {
     BlockInfoList *head = NULL, *cur_item = NULL;
diff --git a/block.h b/block.h
index c89590d..e520eec 100644
--- a/block.h
+++ b/block.h
@@ -256,7 +256,8 @@ int bdrv_co_discard(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
-
+int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+                            int64_t sector_num, int nb_sectors, int *pnum);
 void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
                        BlockErrorAction on_write_error);
 BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 39419a0..7ee0f69 100644
--- a/qemu-img-cmds.hx
+++ 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")
+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"
+           "  '-q' Quiet mode - do not print any output (except errors)\n"
+           "  '-s' Strict mode - fail on different image size or sector 
allocation\n";
 
     printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
@@ -652,6 +656,277 @@ static int compare_sectors(const uint8_t *buf1, const 
uint8_t *buf2, int n,
 
 #define IO_BUF_SIZE (2 * 1024 * 1024)
 
+/*
+ * Get number of sectors that can be stored in IO buffer.
+ */
+
+static int64_t sectors_to_process(int64_t total, int64_t from)
+{
+    int64_t rv = total - from;
+
+    if (rv > (IO_BUF_SIZE >> BDRV_SECTOR_BITS)) {
+        return IO_BUF_SIZE >> BDRV_SECTOR_BITS;
+    }
+
+    return rv;
+}
+
+/*
+ * Compares two images. Exit codes:
+ *
+ * 0 - Images are identical
+ * 1 - Images differ
+ * 2 - Error occured
+ */
+
+static int img_compare(int argc, char **argv)
+{
+    const char *fmt1 = NULL, *fmt2 = NULL, *filename1, *filename2;
+    BlockDriverState *bs1, *bs2;
+    int64_t total_sectors1, total_sectors2;
+    uint8_t *buf1 = NULL, *buf2 = NULL;
+    int pnum1, pnum2;
+    int allocated1, allocated2;
+    int flags = BDRV_O_FLAGS;
+    int ret = 0; /* return value - 0 Ident, 1 Different, 2 Error */
+    int progress = 0, quiet = 0, strict = 0;
+    int64_t total_sectors;
+    int64_t sector_num = 0;
+    int64_t nb_sectors;
+    int c, rv, pnum;
+    uint64_t bs_sectors;
+    uint64_t progress_base;
+
+
+    for (;;) {
+        c = getopt(argc, argv, "pf:F:sq");
+        if (c == -1) {
+            break;
+        }
+        switch (c) {
+        case 'f':
+            fmt1 = optarg;
+            if (fmt2 == NULL) {
+                fmt2 = optarg;
+            }
+            break;
+        case 'F':
+            fmt2 = optarg;
+            if (fmt1 == NULL) {
+                fmt2 = optarg;
+            }
+            break;
+        case 'p':
+            progress = (quiet == 0) ? 1 : 0;
+            break;
+        case 'q':
+            quiet = 1;
+            if (progress == 1) {
+                progress = 0;
+            }
+            break;
+        case 's':
+            strict = 1;
+            break;
+        }
+    }
+    if (optind >= argc) {
+        help();
+        goto out3;
+    }
+    filename1 = argv[optind++];
+    filename2 = argv[optind++];
+
+    /* Initialize before goto out */
+    qemu_progress_init(progress, 2.0);
+
+    bs1 = bdrv_new_open(filename1, fmt1, flags);
+    if (!bs1) {
+        error_report("Can't open file %s", filename1);
+        ret = 2;
+        goto out3;
+    }
+
+    bs2 = bdrv_new_open(filename2, fmt2, flags);
+    if (!bs2) {
+        error_report("Can't open file %s:", filename2);
+        ret = 2;
+        goto out2;
+    }
+
+    buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
+    buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
+    bdrv_get_geometry(bs1, &bs_sectors);
+    total_sectors1 = bs_sectors;
+    bdrv_get_geometry(bs2, &bs_sectors);
+    total_sectors2 = bs_sectors;
+    total_sectors = total_sectors1;
+    progress_base = total_sectors;
+
+    qemu_progress_print(0, 100);
+
+    if (total_sectors1 != total_sectors2) {
+        BlockDriverState *bsover;
+        int64_t lo_total_sectors, lo_sector_num;
+        const char *filename_over;
+
+        if (strict) {
+            ret = 1;
+            if (!quiet) {
+                printf("Strict mode: Image size mismatch!");
+            }
+            goto out;
+        } else if (!quiet) {
+            printf("Warning: Image size mismatch!\n");
+        }
+
+        if (total_sectors1 > total_sectors2) {
+            total_sectors = total_sectors2;
+            lo_total_sectors = total_sectors1;
+            lo_sector_num = total_sectors2;
+            bsover = bs1;
+            filename_over = filename1;
+        } else {
+            total_sectors = total_sectors1;
+            lo_total_sectors = total_sectors2;
+            lo_sector_num = total_sectors1;
+            bsover = bs2;
+            filename_over = filename2;
+        }
+
+        progress_base = lo_total_sectors;
+
+        for (;;) {
+            nb_sectors = sectors_to_process(lo_total_sectors, lo_sector_num);
+            if (nb_sectors <= 0) {
+                break;
+            }
+            rv = bdrv_is_allocated(bsover, lo_sector_num, nb_sectors, &pnum);
+            nb_sectors = pnum;
+            if (rv) {
+                rv = bdrv_read(bsover, lo_sector_num, buf1, nb_sectors);
+                if (rv < 0) {
+                    error_report("error while reading sector %" PRId64
+                                 " of %s: %s", lo_sector_num, filename_over,
+                                 strerror(-rv));
+                    ret = 2;
+                    goto out;
+                }
+                rv = is_allocated_sectors(buf1, nb_sectors, &pnum);
+                if (rv || pnum != nb_sectors) {
+                    ret = 1;
+                    if (!quiet) {
+                        printf("Content mismatch - Sector %" PRId64
+                               " not available in both images!\n",
+                               rv ? lo_sector_num : lo_sector_num + pnum);
+                    }
+                    goto out;
+                }
+            }
+            lo_sector_num += nb_sectors;
+            qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+        }
+    }
+
+
+    for (;;) {
+        nb_sectors = sectors_to_process(total_sectors, sector_num);
+        if (nb_sectors <= 0) {
+            break;
+        }
+        allocated1 = bdrv_is_allocated_above(bs1, NULL, sector_num, nb_sectors,
+                                             &pnum1);
+        allocated2 = bdrv_is_allocated_above(bs2, NULL, sector_num, nb_sectors,
+                                             &pnum2);
+        if (pnum1 < pnum2) {
+            nb_sectors = pnum1;
+        } else {
+            nb_sectors = pnum2;
+        }
+
+        if (allocated1 == allocated2) {
+            if (allocated1) {
+                rv = bdrv_read(bs1, sector_num, buf1, nb_sectors);
+                if (rv < 0) {
+                    ret = 2;
+                    error_report("error while reading sector %" PRId64 " of 
%s:"
+                                 " %s", sector_num, filename1, strerror(-rv));
+                    goto out;
+                }
+                rv = bdrv_read(bs2, sector_num, buf2, nb_sectors);
+                if (rv < 0) {
+                    ret = 2;
+                    error_report("error while reading sector %" PRId64
+                                 " of %s: %s", sector_num, filename2,
+                                 strerror(-rv));
+                    goto out;
+                }
+                rv = compare_sectors(buf1, buf2, nb_sectors, &pnum);
+                if (rv || pnum != nb_sectors) {
+                    ret = 1;
+                    if (!quiet) {
+                        printf("Content mismatch at sector %" PRId64 "!\n",
+                               rv ? sector_num : sector_num + pnum);
+                    }
+                    goto out;
+                }
+            }
+        } else {
+            BlockDriverState *bstmp;
+            const char *filenametmp;
+
+            if (strict) {
+                ret = 1;
+                if (!quiet) {
+                    printf("Strict mode: Sector %" PRId64
+                           " allocation mismatch!",
+                           sector_num);
+                }
+                goto out;
+            }
+
+            if (allocated1) {
+                bstmp = bs1;
+                filenametmp = filename1;
+            } else {
+                bstmp = bs2;
+                filenametmp = filename2;
+            }
+            rv = bdrv_read(bstmp, sector_num, buf1, nb_sectors);
+            if (rv < 0) {
+                ret = 2;
+                error_report("error while reading sector %" PRId64 " of %s: 
%s",
+                                sector_num, filenametmp, strerror(-rv));
+                goto out;
+            }
+            rv = is_allocated_sectors(buf1, nb_sectors, &pnum);
+            if (rv || pnum != nb_sectors) {
+                ret = 1;
+                if (!quiet) {
+                    printf("Content mismatch at sector %" PRId64 "!\n",
+                           rv ? sector_num : sector_num + pnum);
+                }
+                goto out;
+            }
+        }
+        sector_num += nb_sectors;
+        qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+    }
+    if (!quiet) {
+        printf("Images are identical.\n");
+    }
+
+out:
+    bdrv_delete(bs2);
+    qemu_vfree(buf1);
+    qemu_vfree(buf2);
+out2:
+    bdrv_delete(bs1);
+out3:
+    qemu_progress_end();
+    return ret;
+}
+
 static int img_convert(int argc, char **argv)
 {
     int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
diff --git a/qemu-img.texi b/qemu-img.texi
index 77c6d0b..2972118 100644
--- a/qemu-img.texi
+++ 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)
address@hidden -q
+Quiet mode - do not print any output (except errors)
address@hidden -s
+Strict mode - fail on on different image size or sector allocation
address@hidden table
+
 Command description:
 
 @table @option
@@ -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
+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.
+
 @item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] 
[-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] 
@var{filename} address@hidden [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to 
disk image @var{output_filename}
--