Add subcommand compare for qemu-img

PATCH Superseded [email protected] 2 participants 20 messages Dec 17, 2012 – Jan 14, 2013

lore

Share & tools

lore

[email protected]

author

2012-12-17 14:16 UTC #0 lore raw

v7 cover Add subcommand compare for qemu-img

From: Miroslav Rezanina <[email protected]>

This is seventh 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

v7:
- split patch into pieces
- Quiet mode added for all relevant subcommands
- check non-shared part of disk after shared one
- minor docummentation and naming fixes

v6:
- added handling -?, -h options for compare subcommand

v5 (only minor changes):
- removed redundant comment
- removed dead code (goto after help())
- set final total_sectors on first assignment

v4:
- Fixed various typos
- Added functions for empty sector check and sector-to-bytes offset conversion
- Fixed command-line parameters processing

v3:
- options -f/-F are orthogonal
- documentation updated to new syntax and behavior
- used byte offset instead of sector number for output

v2:
- changed option for second image format to -F
- changed handling 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

Signed-off-by Miroslav Rezanina <[email protected]>

Miroslav Rezanina (4):
block: Add synchronous wrapper for bdrv_co_is_allocated_above
qemu-img: Add "Quiet mode" option
qemu-img: Add compare subcommand
Add qemu-img compare documentation

 block.c          |  50 +++++++-
 block.h          |   4 +-
 blockdev.c       |   6 +-
 qemu-img-cmds.hx |  34 +++--
 qemu-img.c       | 383 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 qemu-img.texi    |  35 +++++
 6 files changed, 460 insertions(+), 52 deletions(-)

Signature · 1 line

1.7.11.7

[email protected]

author

2012-12-17 14:16 UTC #1 lore raw

PATCH v7 1/4 block: Add synchronous wrapper for bdrv_co_is_allocated_above

From: Miroslav Rezanina <[email protected]>

There's no synchronous wrapper for bdrv_co_is_allocated_above function so it's not possible to check for sector allocation in image with backing file.

This patch add missing synchronous wrapper.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 block.c | 39 +++++++++++++++++++++++++++++++++++++++
 block.h |  2 ++
 2 files changed, 41 insertions(+)

block.c +39 -0

index c05875f..24c06ab 100644
@@ -2699,6 +2699,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 
 typedef struct BdrvCoIsAllocatedData {
     BlockDriverState *bs;
+    BlockDriverState *base;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;

@@ -2829,6 +2830,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;
+}
+
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));

block.h +2 -0

index 722c620..2cb8d71 100644
@@ -278,6 +278,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, BlockdevOnError on_read_error,
                        BlockdevOnError on_write_error);

Signature · 1 line

1.7.11.7

[email protected]

author

2012-12-17 14:17 UTC #2 lore raw

PATCH v7 2/4 qemu-img: Add "Quiet mode" option

From: Miroslav Rezanina <[email protected]>

There can be need to turn output to stdout off. This patch adds a -q option that enable "Quiet mode". In Quiet mode, only errors are printed out.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 block.c          |  11 +++---
 block.h          |   2 +-
 blockdev.c       |   6 ++--
 qemu-img-cmds.hx |  28 +++++++--------
 qemu-img.c       | 108 +++++++++++++++++++++++++++++++++++++++++--------------
 qemu-img.texi    |   3 ++
 6 files changed, 108 insertions(+), 50 deletions(-)

block.c +6 -5

index 24c06ab..5450ff9 100644
@@ -4449,7 +4449,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
 
 int bdrv_img_create(const char *filename, const char *fmt,
                     const char *base_filename, const char *base_fmt,
-                    char *options, uint64_t img_size, int flags)
+                    char *options, uint64_t img_size, int flags, bool quiet)
 {
     QEMUOptionParameter *param = NULL, *create_options = NULL;
     QEMUOptionParameter *backing_fmt, *backing_file, *size;

@@ -4565,10 +4565,11 @@ int bdrv_img_create(const char *filename, const char *fmt,
         }
     }
 
-    printf("Formatting '%s', fmt=%s ", filename, fmt);
-    print_option_parameters(param);
-    puts("");
-
+    if (!quiet) {
+        printf("Formatting '%s', fmt=%s ", filename, fmt);
+        print_option_parameters(param);
+        puts("");
+    }
     ret = bdrv_create(drv, filename, param);
 
     if (ret < 0) {

block.h +1 -1

index 2cb8d71..a7e7220 100644
@@ -347,7 +347,7 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
 
 int bdrv_img_create(const char *filename, const char *fmt,
                     const char *base_filename, const char *base_fmt,
-                    char *options, uint64_t img_size, int flags);
+                    char *options, uint64_t img_size, int flags, bool quiet);
 
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);

blockdev.c +3 -3

index e73fd6e..b2fb7f7 100644
@@ -789,7 +789,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
             ret = bdrv_img_create(new_image_file, format,
                                   states->old_bs->filename,
                                   states->old_bs->drv->format_name,
-                                  NULL, -1, flags);
+                                  NULL, -1, flags, false);
             if (ret) {
                 error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
                 goto delete_and_fail;

@@ -1264,7 +1264,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         bdrv_get_geometry(bs, &size);
         size *= 512;
         ret = bdrv_img_create(target, format,
-                              NULL, NULL, NULL, size, flags);
+                              NULL, NULL, NULL, size, flags, false);
     } else {
         switch (mode) {
         case NEW_IMAGE_MODE_EXISTING:

@@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device, const char *target,
             ret = bdrv_img_create(target, format,
                                   source->filename,
                                   source->drv->format_name,
-                                  NULL, -1, flags);
+                                  NULL, -1, flags,false);
             break;
         default:
             abort();

qemu-img-cmds.hx +14 ~~-14~~

index a181363..90b93e0 100644
@@ -10,27 +10,27 @@ STEXI
 ETEXI
 
 DEF("check", img_check,
-    "check [-f fmt] [-r [leaks | all]] filename")
+    "check [-q] [-f fmt] [-r [leaks | all]] filename")
 STEXI
-@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
+@item check [-q] [-f @var{fmt}] [-r [leaks | all]] @var{filename}
 ETEXI
 
 DEF("create", img_create,
-    "create [-f fmt] [-o options] filename [size]")
+    "create [-q] [-f fmt] [-o options] filename [size]")
 STEXI
-@item create [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
-    "commit [-f fmt] [-t cache] filename")
+    "commit [-q] [-f fmt] [-t cache] filename")
 STEXI
-@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 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")
+    "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
-@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} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,

@@ -40,20 +40,20 @@ STEXI
 ETEXI
 
 DEF("snapshot", img_snapshot,
-    "snapshot [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+    "snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
 STEXI
-@item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
 ETEXI
 
 DEF("rebase", img_rebase,
-    "rebase [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+    "rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
 STEXI
-@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
 ETEXI
 
 DEF("resize", img_resize,
-    "resize filename [+ | -]size")
+    "resize [-q] filename [+ | -]size")
 STEXI
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [-q] @var{filename} [+ | -]@var{size}
 @end table
 ETEXI

qemu-img.c +81 ~~-27~~

361 lines not on this page · raw message

qemu-img.texi +3 -0

index 00fca8d..bb82a3d 100644
@@ -54,6 +54,9 @@ indicates that target image must be compressed (qcow format only)
 with or without a command shows help and lists the supported formats
 @item -p
 display progress bar (convert and rebase commands only)
+@item -q
+Quiet mode - do not print any output (except errors). There's no progres bar 
+in case both @var{-q} and  @var{-p} options are used.
 @item -S @var{size}
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded

Signature · 1 line

1.7.11.7

[email protected]

author

2012-12-17 14:16 UTC #3 lore raw

PATCH v7 3/4 qemu-img: Add compare subcommand

From: Miroslav Rezanina <[email protected]>

This patch adds new qemu-img subcommand that compare content of two disk images.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 qemu-img-cmds.hx |   6 ++
 qemu-img.c       | 268 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 273 insertions(+), 1 deletion(-)

qemu-img-cmds.hx +6 -0

index 90b93e0..bc1ccfa 100644
@@ -27,6 +27,12 @@ STEXI
 @item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
+DEF("compare", img_compare,
+    "compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2")
+STEXI
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+ETEXI
+
 DEF("convert", img_convert,
     "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI

qemu-img.c +267 -1

280 lines not on this page · raw message

Signature · 1 line

1.7.11.7

[email protected]

author

2012-12-17 14:16 UTC #4 lore raw

PATCH v7 4/4 Add qemu-img compare documentation

From: Miroslav Rezanina <[email protected]>

Adding documentation for new qemu-img subcommand compare.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 qemu-img.c    |  7 ++++++-
 qemu-img.texi | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

qemu-img.c +6 -1

index 8b4f01f..a185e9e 100644
@@ -103,7 +103,12 @@ 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"
+           "\n"
+           "Parameters to compare subcommand:\n"
+           "  '-f' First image format\n"
+           "  '-F' Second image format\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);

qemu-img.texi +32 -0

index bb82a3d..7eb1934 100644
@@ -84,6 +84,18 @@ deletes a snapshot
 lists all snapshots in the given image
 @end table
 
+Parameters to compare subcommand:
+
+@table @option
+
+@item -f
+First image format
+@item -F
+Second image format
+@item -s
+Strict mode - fail on on different image size or sector allocation
+@end table
+
 Command description:
 
 @table @option

@@ -117,6 +129,26 @@ it doesn't need to be specified separately in this case.
 
 Commit the changes recorded in @var{filename} in its base image.
 
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
+
+Check if two images have the same content. You can compare images with
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
+
+By default, images with different size are considered identical if the larger
+image contains only unallocated and/or zeroed sectors in the area after the end
+of the other image. In addition, if any sector is not allocated in one image
+and contains only zero bytes in the second one, it is evaluated as equal. You
+can use Strict mode by specifying the @var{-s} option. When compare runs in
+Strict mode, it fails in case image size differs or a sector is allocated in
+one image and is not allocated in the second one.
+
+By default, compare prints out a result message. This message displays
+information that both images are same or the position of the first different
+byte. In addition, result message can report different image size in case
+Strict mode is used.
+
 @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} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename}

Signature · 1 line

1.7.11.7

Eric Blake

2012-12-19 18:15 UTC #5 lore raw

Re: [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation

Quoted 15 lines · On 12/17/2012 06:39 AM, [email protected] wrote:

From: Miroslav Rezanina <[email protected]>
Adding documentation for new qemu-img subcommand compare.
Signed-off-by Miroslav Rezanina <[email protected]>
---
 qemu-img.c    |  7 ++++++-
 qemu-img.texi | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)
qemu-img.c +0 -0
index 8b4f01f..a185e9e 100644

qemu-img.c static void help(void) · line 111

Quoted 10 lines

qemu-img.c +6 -1

@@ -103,7 +103,12 @@ 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"
+           "\n"
+           "Parameters to compare subcommand:\n"
+           "  '-f' First image format\n"
+           "  '-F' Second image format\n"
+           "  '-s' Strict mode - fail on different image size or sector allocation\n";

s/First/first/; s/Second/second/; s/Strict/strict/ for consistent appearance of starting description with lower case

qemu-img.c it doesn't need to be specified separately in this case. · line 137

Quoted 11 lines

qemu-img.c +6 -0

 @table @option

@@ -117,6 +129,26 @@ it doesn't need to be specified separately in this case.
 
 Commit the changes recorded in @var{filename} in its base image.
 
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
+
+Check if two images have the same content. You can compare images with
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.

Wrap this long line.

Signature · 2 lines

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

Miroslav Rezanina

author

2012-12-20 19:46 UTC #6 lore raw

Re: [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation

----- Original Message -----

Quoted 29 lines

From: "Eric Blake" <[email protected]>
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Sent: Wednesday, December 19, 2012 7:15:11 PM
Subject: Re: [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation
Quoted 22 lines · On 12/17/2012 06:39 AM, [email protected] wrote:
From: Miroslav Rezanina <[email protected]>
Adding documentation for new qemu-img subcommand compare.
Signed-off-by Miroslav Rezanina <[email protected]>
---
 qemu-img.c    |  7 ++++++-
 qemu-img.texi | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)
qemu-img.c +2 -1
index 8b4f01f..a185e9e 100644
@@ -103,7 +103,12 @@ 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"
+           "\n"

Quoted 8 lines

qemu-img.c static void help(void)
Quoted 5 lines
qemu-img.c +4 -0
+           "Parameters to compare subcommand:\n"
+           "  '-f' First image format\n"
+           "  '-F' Second image format\n"
+           "  '-s' Strict mode - fail on different image size or
 sector allocation\n";
s/First/first/; s/Second/second/; s/Strict/strict/ for consistent appearance of starting description with lower case

Yes for First and Second, but Strict mode is same case as Quiet mode in patch 02 - it's always capital S not because it's start of description.

Quoted 7 lines

Quoted 7 lines
qemu-img.c +0 -0
 @table @option
@@ -117,6 +129,26 @@ it doesn't need to be specified separately in
this case.
Commit the changes recorded in @var{filename} in its base image.

Quoted 12 lines

Quoted 10 lines
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
qemu-img.c +2 -0
+
+Check if two images have the same content. You can compare images
with
qemu-img.c +3 -0
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for
@var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
Wrap this long line.

None of the long lines is wraped in qemu-img.c in this part of code so I did not wrap it intentionally.

Quoted 6 lines

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

Miroslav Rezanina

Eric Blake

2012-12-19 18:12 UTC #7 lore raw

Re: [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand

In reply to [email protected]

Quoted 3 lines · On 12/17/2012 06:39 AM, [email protected] wrote:

From: Miroslav Rezanina <[email protected]>
This patch adds new qemu-img subcommand that compare content of two disk

s/compare/compares/

qemu-img.c static int img_commit(int argc, char **argv) · line 590

Quoted 10 lines

images.

Signed-off-by Miroslav Rezanina <[email protected]>

---

qemu-img.c +1 -1

@@ -587,7 +587,7 @@ static int img_commit(int argc, char **argv)
 }
 
 /*
- * Returns true iff the first sector pointed to by 'buf' contains at least
+ * Returns true if the first sector pointed to by 'buf' contains at least

Spurious change. 'iff' is correct here, for its mathematical meaning of if-and-only-if.

qemu-img.c static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n, · line 693

Quoted 10 lines

qemu-img.c +3 -0

  * a non-NUL byte.
  *
  * 'pnum' is set to the number of sectors (including and immediately following

@@ -688,6 +688,272 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
 
 #define IO_BUF_SIZE (2 * 1024 * 1024)
 
+static int64_t sectors_to_bytes(int64_t sectors)
+{
+    return sectors << BDRV_SECTOR_BITS;

Worth checking for overflow?

qemu-img.c static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n, · line 731

Quoted 12 lines

qemu-img.c +12 -0

+static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
+                               int sect_count, const char *filename,
+                               uint8_t *buffer, bool quiet)
+{
+    int pnum, ret = 0;
+    ret = bdrv_read(bs, sect_num, buffer, sect_count);
+    if (ret < 0) {
+        error_report("Error while reading offset %" PRId64 " of %s: %s",
+                     sectors_to_bytes(sect_num), filename, strerror(-ret));
+        return ret;
+    }
+    ret = is_allocated_sectors(buffer, sect_count, &pnum);

Is this logic backwards? Isn't it wasteful to read a sector prior to seeing if it was actually allocated?

Signature · 2 lines

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

Miroslav Rezanina

author

2012-12-20 19:44 UTC #8 lore raw

Re: [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand

----- Original Message -----

Quoted 21 lines

From: "Eric Blake" <[email protected]>
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Sent: Wednesday, December 19, 2012 7:12:49 PM
Subject: Re: [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand
Quoted 4 lines · On 12/17/2012 06:39 AM, [email protected] wrote:
From: Miroslav Rezanina <[email protected]>
This patch adds new qemu-img subcommand that compare content of two disk
s/compare/compares/
Quoted 7 lines
images.
Signed-off-by Miroslav Rezanina <[email protected]>
---
(quoted diff) +0 -0
@@ -587,7 +587,7 @@ static int img_commit(int argc, char **argv)
 }
 

Quoted 9 lines

Quoted 5 lines
(quoted diff) +0 -1
 /*
- * Returns true iff the first sector pointed to by 'buf' contains
at least
+ * Returns true if the first sector pointed to by 'buf' contains at least
Spurious change. 'iff' is correct here, for its mathematical meaning of
if-and-only-if.

You're right. I probably just see iff and change it to if without checking the reason.

Quoted 8 lines

Quoted 8 lines
* a non-NUL byte.
*
* 'pnum' is set to the number of sectors (including and immediately following
(quoted diff) +0 -0
@@ -688,6 +688,272 @@ static int compare_sectors(const uint8_t
 *buf1, const uint8_t *buf2, int n,
 

Quoted 8 lines

qemu-img.c static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n, · line 693
Quoted 5 lines
qemu-img.c +3 -0
 #define IO_BUF_SIZE (2 * 1024 * 1024)
 
+static int64_t sectors_to_bytes(int64_t sectors)
+{
+    return sectors << BDRV_SECTOR_BITS;
Worth checking for overflow?

I think it's not. This should be save as block driver should not allow sectors to cause overflow.

Quoted 11 lines

qemu-img.c +8 -0

+static int check_empty_sectors(BlockDriverState *bs, int64_t
 sect_num,
+                               int sect_count, const char
 *filename,
+                               uint8_t *buffer, bool quiet)
+{
+    int pnum, ret = 0;
+    ret = bdrv_read(bs, sect_num, buffer, sect_count);
+    if (ret < 0) {
+        error_report("Error while reading offset %" PRId64 " of

%s: %s",

Quoted 9 lines

qemu-img.c static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n, · line 731
Quoted 5 lines
+ sectors_to_bytes(sect_num), filename,
strerror(-ret));
qemu-img.c +3 -0
+        return ret;
+    }
+    ret = is_allocated_sectors(buffer, sect_count, &pnum);
Is this logic backwards? Isn't it wasteful to read a sector prior to seeing if it was actually allocated?

This is correct order. Function is_allocated_sector test if sectors contain any non-zero byte. We know that sector is "physically" allocated in the image, we test if it contains any data.

Quoted 5 lines

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

Miroslav Rezanina

Eric Blake

2012-12-19 18:06 UTC #9 lore raw

Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option

In reply to [email protected]

Quoted 1 line · On 12/17/2012 06:39 AM, [email protected] wrote:

From: Miroslav Rezanina <[email protected]>

Your git send-email settings threaded each message as a reply to the other, rather than the more typical setting of threading messages only as a reply to the cover letter. You may want to do 'git config format.thread shallow' rather than your current deep approach.

Quoted 2 lines

There can be need to turn output to stdout off. This patch adds a -q option that

s/be need/be a need/

Quoted 13 lines

enable "Quiet mode". In Quiet mode, only errors are printed out.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 block.c          |  11 +++---
 block.h          |   2 +-
 blockdev.c       |   6 ++--
 qemu-img-cmds.hx |  28 +++++++--------
 qemu-img.c       | 108 +++++++++++++++++++++++++++++++++++++++++--------------
 qemu-img.texi    |   3 ++
 6 files changed, 108 insertions(+), 50 deletions(-)

blockdev.c void qmp_drive_mirror(const char *device, const char *target, · line 1278

Quoted 5 lines

blockdev.c +1 -1

@@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device, const char *target,
             ret = bdrv_img_create(target, format,
                                   source->filename,
                                   source->drv->format_name,
-                                  NULL, -1, flags);
+                                  NULL, -1, flags,false);

Space after comma.

qemu-img-cmds.hx STEXI · line 13

Quoted 6 lines

qemu-img-cmds.hx +1 -1

@@ -10,27 +10,27 @@ STEXI
 ETEXI
 
 DEF("check", img_check,
-    "check [-f fmt] [-r [leaks | all]] filename")
+    "check [-q] [-f fmt] [-r [leaks | all]] filename")

Is it really better to list [-q] to all of the sub-commands, or would it be easier to simply declare that -q is a universal option that can appear before sub-commands, as in:
qemu-img [-q] command [command options]

qemu-img.c static void help(void) · line 90

Quoted 7 lines

qemu-img.c +1 -0

 #ifdef _WIN32
 #include <windows.h>

@@ -86,6 +87,7 @@ static void help(void)
            "       rebasing in this case (useful for renaming the backing file)\n"
            "  '-h' with or without a command shows this help and lists the supported formats\n"
            "  '-p' show progress of command (only certain commands)\n"
+           "  '-q' Quiet mode - do not print any output (except errors)\n"

s/Quiet/quiet/ for consistency with the nearby lines not starting with a capital.

qemu-img.texi indicates that target image must be compressed (qcow format only) · line 58

Quoted 7 lines

qemu-img.texi +3 -0

+++ b/qemu-img.texi

@@ -54,6 +54,9 @@ indicates that target image must be compressed (qcow format only)
 with or without a command shows help and lists the supported formats
 @item -p
 display progress bar (convert and rebase commands only)
+@item -q
+Quiet mode - do not print any output (except errors). There's no progres bar

s/progres/progress/

qemu-img.texi indicates that target image must be compressed (qcow format only) · line 59

Quoted 1 line

qemu-img.texi +1 -0

+in case both @var{-q} and  @var{-p} options are used.

Should it be a hard error if both -q and -p are given? Otherwise, when dealing with conflicting options, it's more typical to have the semantic of last one wins: 'qemu-img -q -p' prints progress, and only 'qemu-img -p -q' is quiet.

Signature · 2 lines

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

Miroslav Rezanina

author

2012-12-20 19:31 UTC #10 lore raw

Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option

Hi Eric,
thanks for review, reply inline.

----- Original Message -----

Quoted 6 lines

From: "Eric Blake" <[email protected]>
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Sent: Wednesday, December 19, 2012 7:06:35 PM
Subject: Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option

Quoted 8 lines

Quoted 1 line · On 12/17/2012 06:39 AM, [email protected] wrote:
From: Miroslav Rezanina <[email protected]>
Your git send-email settings threaded each message as a reply to the other, rather than the more typical setting of threading messages only
as a reply to the cover letter. You may want to do 'git config format.thread shallow' rather than your current deep approach.

I use temporary machine for sending patches as my usual one was not usable for me in this case, so setting can be incorrect. I will take better care of this next time.

Quoted 33 lines

Quoted 4 lines

There can be need to turn output to stdout off. This patch adds a -q option that

s/be need/be a need/

Quoted 14 lines

enable "Quiet mode". In Quiet mode, only errors are printed out.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 block.c          |  11 +++---
 block.h          |   2 +-
 blockdev.c       |   6 ++--
 qemu-img-cmds.hx |  28 +++++++--------
 qemu-img.c       | 108
 +++++++++++++++++++++++++++++++++++++++++--------------
 qemu-img.texi    |   3 ++
 6 files changed, 108 insertions(+), 50 deletions(-)

blockdev.c void qmp_drive_mirror(const char *device, · line 1279

Quoted 6 lines

blockdev.c +1 -1

@@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device,
 const char *target,
             ret = bdrv_img_create(target, format,
                                   source->filename,
                                   source->drv->format_name,
-                                  NULL, -1, flags);
+                                  NULL, -1, flags,false);

Space after comma.

Quoted 3 lines

(quoted diff) +0 -0

@@ -10,27 +10,27 @@ STEXI
 ETEXI

Quoted 10 lines

qemu-img-cmds.hx STEXI · line 13
Quoted 3 lines
qemu-img-cmds.hx +1 -1
 DEF("check", img_check,
-    "check [-f fmt] [-r [leaks | all]] filename")
+    "check [-q] [-f fmt] [-r [leaks | all]] filename")
Is it really better to list [-q] to all of the sub-commands, or would it
be easier to simply declare that -q is a universal option that can appear before sub-commands, as in:
qemu-img [-q] command [command options]

-q is not useable for all commands so it is listed this ways as it is same for other options.

Quoted 7 lines

qemu-img.c +0 -0

 #ifdef _WIN32
 #include <windows.h>

@@ -86,6 +87,7 @@ static void help(void)
            "       rebasing in this case (useful for renaming the
            backing file)\n"
            "  '-h' with or without a command shows this help and
            lists the supported formats\n"

Quoted 8 lines

Quoted 4 lines
(quoted diff) +1 -0
            "  '-p' show progress of command (only certain
            commands)\n"
+           "  '-q' Quiet mode - do not print any output (except
errors)\n"
s/Quiet/quiet/ for consistency with the nearby lines not starting with a
capital.

I use Quiet mode as name with capital Q, so I have to choose what consistency to break - I choose the start of the description. I have no problem with using quiet instead of Quiet.

Quoted 16 lines

Quoted 11 lines
+++ b/qemu-img.texi
(quoted diff) +2 -0
@@ -54,6 +54,9 @@ indicates that target image must be compressed
 (qcow format only)
 with or without a command shows help and lists the supported
 formats
 @item -p
 display progress bar (convert and rebase commands only)
+@item -q
+Quiet mode - do not print any output (except errors). There's no
progres bar
s/progres/progress/
Quoted 1 line
+in case both @var{-q} and @var{-p} options are used.

Quoted 8 lines

Should it be a hard error if both -q and -p are given? Otherwise, when
dealing with conflicting options, it's more typical to have the semantic
of last one wins: 'qemu-img -q -p' prints progress, and only 'qemu-img
-p -q' is quiet.

Depends on point of view - There's no conflict for -p and -q as -p adds progress bar to output and -q suppress output. You can imagine (in theory) -q as redirecting stdout to /dev/null.

Quoted 5 lines

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

Eric Blake

2012-12-19 17:54 UTC #11 lore raw

Re: [Qemu-devel] [PATCH v7 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above

In reply to [email protected]

Quoted 4 lines · On 12/17/2012 06:39 AM, [email protected] wrote:

From: Miroslav Rezanina <[email protected]>
There's no synchronous wrapper for bdrv_co_is_allocated_above function so it's not possible to check for sector allocation in image with

s/in image with/in an image with a/

Quoted 3 lines

backing file.
This patch add missing synchronous wrapper.

s/add/adds the/

Quoted 9 lines

Signed-off-by Miroslav Rezanina <[email protected]>
---
 block.c | 39 +++++++++++++++++++++++++++++++++++++++
 block.h |  2 ++
 2 files changed, 41 insertions(+)
block.c +0 -0
index c05875f..24c06ab 100644

block.c int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top, · line 2851

Quoted 8 lines

block.c +7 -1

--- a/block.c
+/*
+ * 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)

Indentation looks off.

Signature · 2 lines

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

Miroslav Rezanina

author

2013-01-14 10:26 UTC #12 lore raw

PATCH v8 1/4 block: Add synchronous wrapper for bdrv_co_is_allocated_above

In reply to [email protected]

There's no synchronous wrapper for bdrv_co_is_allocated_above function so it's not possible to check for sector allocation in in mage with a backing file.

This patch adds the missing synchronous wrapper.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 block.c               |   39 +++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    2 ++
 2 files changed, 41 insertions(+), 0 deletions(-)

block.c +39 -0

index 60873ea..8ea8bb7 100644
@@ -2716,6 +2716,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 
 typedef struct BdrvCoIsAllocatedData {
     BlockDriverState *bs;
+    BlockDriverState *base;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;

@@ -2846,6 +2847,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;
+}
+
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));

include/block/block.h +2 -0

index 0719339..ec62d92 100644
@@ -278,6 +278,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, BlockdevOnError on_read_error,
                        BlockdevOnError on_write_error);

Signature · 1 line

1.7.1

Eric Blake

Reviewed-by on v8 1/4

2013-01-14 21:35 UTC #13 lore raw

Re: [Qemu-devel] [PATCH v8 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above

Quoted 2 lines · Miroslav Rezanina

There's no synchronous wrapper for bdrv_co_is_allocated_above function so it's not possible to check for sector allocation in in mage with

s/in in mage/in an image/

Quoted 9 lines

a backing file.
This patch adds the missing synchronous wrapper.
Signed-off-by Miroslav Rezanina <[email protected]>
---
 block.c               |   39 +++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    2 ++
 2 files changed, 41 insertions(+), 0 deletions(-)

Reviewed-by Eric Blake <[email protected]>

Signature · 2 lines

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

Miroslav Rezanina

author

2013-01-14 10:26 UTC #14 lore raw

v8 cover Add subcommand compare for qemu-img

In reply to [email protected]

This is 8th 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

v8:
- minor grammar fixes (no behavior change)

v7:
- split patch into pieces
- Quiet mode added for all relevant subcommands
- check non-shared part of disk after shared one
- minor docummentation and naming fixes

v6:
- added handling -?, -h options for compare subcommand

v5 (only minor changes):
- removed redundant comment
- removed dead code (goto after help())
- set final total_sectors on first assignment

v4:
- Fixed various typos
- Added functions for empty sector check and sector-to-bytes offset conversion
- Fixed command-line parameters processing

v3:
- options -f/-F are orthogonal
- documentation updated to new syntax and behavior
- used byte offset instead of sector number for output

Signed-off-by Miroslav Rezanina <[email protected]>

Miroslav Rezanina (4):
block: Add synchronous wrapper for bdrv_co_is_allocated_above
qemu-img: Add "Quiet mode" option
qemu-img: Add compare subcommand
Add qemu-img compare documentation

 block.c               |   51 ++++++-
 blockdev.c            |    6 +-
 include/block/block.h |    5 +-
 qemu-img-cmds.hx      |   34 +++--
 qemu-img.c            |  381 +++++++++++++++++++++++++++++++++++++++++++++----
 qemu-img.texi         |   35 +++++
 6 files changed, 461 insertions(+), 51 deletions(-)

Miroslav Rezanina

author

2013-01-14 10:26 UTC #15 lore raw

PATCH v8 2/4 qemu-img: Add "Quiet mode" option

In reply to [email protected]

There can be a need to turn output to stdout off. This patch adds a -q option that enable "Quiet mode". In Quiet mode, only errors are printed out.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 block.c               |   12 +++--
 blockdev.c            |    6 +-
 include/block/block.h |    3 +-
 qemu-img-cmds.hx      |   28 ++++++------
 qemu-img.c            |  108 ++++++++++++++++++++++++++++++++++++------------
 qemu-img.texi         |    3 +
 6 files changed, 110 insertions(+), 50 deletions(-)

block.c +7 -5

index 8ea8bb7..e563ca8 100644
@@ -4512,7 +4512,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
 
 void bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
-                     char *options, uint64_t img_size, int flags, Error **errp)
+                     char *options, uint64_t img_size, int flags,
+                     Error **errp, bool quiet)
 {
     QEMUOptionParameter *param = NULL, *create_options = NULL;
     QEMUOptionParameter *backing_fmt, *backing_file, *size;

@@ -4621,10 +4622,11 @@ void bdrv_img_create(const char *filename, const char *fmt,
         }
     }
 
-    printf("Formatting '%s', fmt=%s ", filename, fmt);
-    print_option_parameters(param);
-    puts("");
-
+    if (!quiet) {
+        printf("Formatting '%s', fmt=%s ", filename, fmt);
+        print_option_parameters(param);
+        puts("");
+    }
     ret = bdrv_create(drv, filename, param);
     if (ret < 0) {
         if (ret == -ENOTSUP) {

blockdev.c +3 -3

index d724e2d..3e377f9 100644
@@ -790,7 +790,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
             bdrv_img_create(new_image_file, format,
                             states->old_bs->filename,
                             states->old_bs->drv->format_name,
-                            NULL, -1, flags, &local_err);
+                            NULL, -1, flags, &local_err, false);
             if (error_is_set(&local_err)) {
                 error_propagate(errp, local_err);
                 goto delete_and_fail;

@@ -1265,7 +1265,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         bdrv_get_geometry(bs, &size);
         size *= 512;
         bdrv_img_create(target, format,
-                        NULL, NULL, NULL, size, flags, &local_err);
+                        NULL, NULL, NULL, size, flags, &local_err, false);
     } else {
         switch (mode) {
         case NEW_IMAGE_MODE_EXISTING:

@@ -1276,7 +1276,7 @@ void qmp_drive_mirror(const char *device, const char *target,
             bdrv_img_create(target, format,
                             source->filename,
                             source->drv->format_name,
-                            NULL, -1, flags, &local_err);
+                            NULL, -1, flags, &local_err, false);
             break;
         default:
             abort();

include/block/block.h +2 -1

index ec62d92..03948c6 100644
@@ -347,7 +347,8 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
 
 void bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
-                     char *options, uint64_t img_size, int flags, Error **errp);
+                     char *options, uint64_t img_size, int flags,
+                     Error **errp, bool quiet);
 
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);

qemu-img-cmds.hx +14 ~~-14~~

index a181363..90b93e0 100644
@@ -10,27 +10,27 @@ STEXI
 ETEXI
 
 DEF("check", img_check,
-    "check [-f fmt] [-r [leaks | all]] filename")
+    "check [-q] [-f fmt] [-r [leaks | all]] filename")
 STEXI
-@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
+@item check [-q] [-f @var{fmt}] [-r [leaks | all]] @var{filename}
 ETEXI
 
 DEF("create", img_create,
-    "create [-f fmt] [-o options] filename [size]")
+    "create [-q] [-f fmt] [-o options] filename [size]")
 STEXI
-@item create [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
-    "commit [-f fmt] [-t cache] filename")
+    "commit [-q] [-f fmt] [-t cache] filename")
 STEXI
-@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 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")
+    "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
-@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} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,

@@ -40,20 +40,20 @@ STEXI
 ETEXI
 
 DEF("snapshot", img_snapshot,
-    "snapshot [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+    "snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
 STEXI
-@item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
 ETEXI
 
 DEF("rebase", img_rebase,
-    "rebase [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+    "rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
 STEXI
-@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
 ETEXI
 
 DEF("resize", img_resize,
-    "resize filename [+ | -]size")
+    "resize [-q] filename [+ | -]size")
 STEXI
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [-q] @var{filename} [+ | -]@var{size}
 @end table
 ETEXI

qemu-img.c +81 ~~-27~~

361 lines not on this page · raw message

qemu-img.texi +3 -0

index 00fca8d..4fdb19a 100644
@@ -54,6 +54,9 @@ indicates that target image must be compressed (qcow format only)
 with or without a command shows help and lists the supported formats
 @item -p
 display progress bar (convert and rebase commands only)
+@item -q
+Quiet mode - do not print any output (except errors). There's no progress bar 
+in case both @var{-q} and  @var{-p} options are used.
 @item -S @var{size}
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded

Signature · 1 line

1.7.1

Eric Blake

2013-01-14 23:01 UTC #16 lore raw

Re: [Qemu-devel] [PATCH v8 2/4] qemu-img: Add "Quiet mode" option

qemu-img.c static int img_check(int argc, char **argv) · line 466

Quoted 12 lines · Miroslav Rezanina

There can be a need to turn output to stdout off. This patch adds a -q option that enable "Quiet mode". In Quiet mode, only errors are printed out.
Signed-off-by Miroslav Rezanina <[email protected]>
---
     if (result.corruptions_fixed || result.leaks_fixed) {
qemu-img.c +2 -1
-        printf("The following inconsistencies were found and repaired:\n\n"
+        qprintf(quiet, 
+               "The following inconsistencies were found and repaired:\n\n"
                "    %d leaked clusters\n"
                "    %d corruptions\n\n"

Here, and in other multi-line conversions, you failed to reindent the subsequent lines, so indentation is now off.

Signature · 2 lines

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

Miroslav Rezanina

author

2013-01-14 10:26 UTC #17 lore raw

PATCH v8 3/4 qemu-img: Add compare subcommand

In reply to [email protected]

This patch adds new qemu-img subcommand that compares content of two disk images.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 qemu-img-cmds.hx |    6 ++
 qemu-img.c       |  266 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 272 insertions(+), 0 deletions(-)

qemu-img-cmds.hx +6 -0

index 90b93e0..bc1ccfa 100644
@@ -27,6 +27,12 @@ STEXI
 @item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
+DEF("compare", img_compare,
+    "compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2")
+STEXI
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+ETEXI
+
 DEF("convert", img_convert,
     "convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI

qemu-img.c +266 -0

272 lines not on this page · raw message

Signature · 1 line

1.7.1

Miroslav Rezanina

author

2013-01-14 10:26 UTC #18 lore raw

PATCH v8 4/4 Add qemu-img compare documentation

In reply to [email protected]

Adding documentation for new qemu-img subcommand compare.

Signed-off-by Miroslav Rezanina <[email protected]>

---
 qemu-img.c    |    7 ++++++-
 qemu-img.texi |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletions(-)

qemu-img.c +6 -1

index 0c12692..6aebdc3 100644
@@ -103,7 +103,12 @@ 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"
+           "\n"
+           "Parameters to compare subcommand:\n"
+           "  '-f' first image format\n"
+           "  '-F' second image format\n"
+           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
 
     printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);

qemu-img.texi +32 -0

index 4fdb19a..90f581a 100644
@@ -84,6 +84,18 @@ deletes a snapshot
 lists all snapshots in the given image
 @end table
 
+Parameters to compare subcommand:
+
+@table @option
+
+@item -f
+First image format
+@item -F
+Second image format
+@item -s
+Strict mode - fail on on different image size or sector allocation
+@end table
+
 Command description:
 
 @table @option

@@ -117,6 +129,26 @@ it doesn't need to be specified separately in this case.
 
 Commit the changes recorded in @var{filename} in its base image.
 
+@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2}
+
+Check if two images have the same content. You can compare images with
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
+
+By default, images with different size are considered identical if the larger
+image contains only unallocated and/or zeroed sectors in the area after the end
+of the other image. In addition, if any sector is not allocated in one image
+and contains only zero bytes in the second one, it is evaluated as equal. You
+can use Strict mode by specifying the @var{-s} option. When compare runs in
+Strict mode, it fails in case image size differs or a sector is allocated in
+one image and is not allocated in the second one.
+
+By default, compare prints out a result message. This message displays
+information that both images are same or the position of the first different
+byte. In addition, result message can report different image size in case
+Strict mode is used.
+
 @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} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename}

Signature · 1 line

1.7.1

Eric Blake

2013-01-14 23:24 UTC #19 lore raw

Re: [Qemu-devel] [PATCH v8 4/4] Add qemu-img compare documentation

Quoted 7 lines · Miroslav Rezanina

Adding documentation for new qemu-img subcommand compare.
Signed-off-by Miroslav Rezanina <[email protected]>
---
 qemu-img.c    |    7 ++++++-
 qemu-img.texi |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletions(-)

Your series is lacking a test case; it would be useful to test both identical images, images that are identical only when not in strict mode, and differing images.

qemu-img.texi it doesn't need to be specified separately in this case. · line 137

Quoted 4 lines

qemu-img.texi +4 -0

+Check if two images have the same content. You can compare images with
+different format or settings.
+
+The format is probed unless you specify it by @var{-f} (used for @var{filename1}) and/or @var{-F} (used for @var{filename2}) option.

Long line; worth wrapping.

Signature · 2 lines

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