On 21.10.21 12:12, Vladimir Sementsov-Ogievskiy wrote:
Let's detect block-size automatically if not specified by user:
If both files define cluster-size, use minimum to be more precise.
If both files don't specify cluster-size, use default of 64K
If only one file specify cluster-size, just use it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
docs/tools/qemu-img.rst | 7 +++-
qemu-img.c | 71 ++++++++++++++++++++++++++++++++++++-----
qemu-img-cmds.hx | 4 +--
3 files changed, 71 insertions(+), 11 deletions(-)
Looks good, just grammar nits and a request for an assertion below.
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 21164253d4..4b382ca2b0 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -170,6 +170,11 @@ Parameters to compare subcommand:
Block size for comparing with ``--stat``. This doesn't guarantee exact
size of comparing chunks, but that guarantee that data chunks being
compared will never cross aligned block-size boundary.
+ When unspecified the following logic is used:
+
+ - If both files define cluster-size, use minimum to be more precise.
+ - If both files don't specify cluster-size, use default of 64K
+ - If only one file specify cluster-size, just use it.
s/specify/specifies/; and perhaps s/it/that/
[...]
diff --git a/qemu-img.c b/qemu-img.c
index 79a0589167..61e7f470bb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1407,6 +1407,61 @@ static void cmp_stat_print(ImgCmpStat *stat, int64_t
total_bytes)
}
}
+/* Get default value for qemu-img compare --block-size option. */
+static int img_compare_block_size(BlockDriverState *bs1,
+ BlockDriverState *bs2,
+ bool quiet)
+{
+ const int default_block_size = 64 * 1024; /* 64K */
+
+ int ret;
+ BlockDriverInfo bdi;
+ int cluster_size1, cluster_size2, block_size;
+ const char *note = "Note: to alter it, set --block-size option.";
+ const char *fname1 = bs1->filename;
+ const char *fname2 = bs2->filename;
+
+ ret = bdrv_get_info(bs1, &bdi);
+ if (ret < 0 && ret != -ENOTSUP) {
+ error_report("Failed to get info of %s: %s", fname1, strerror(-ret));
+ return ret;
+ }
+ cluster_size1 = ret < 0 ? 0 : bdi.cluster_size;
+
+ ret = bdrv_get_info(bs2, &bdi);
+ if (ret < 0 && ret != -ENOTSUP) {
+ error_report("Failed to get info of %s: %s", fname2, strerror(-ret));
+ return ret;
+ }
+ cluster_size2 = ret < 0 ? 0 : bdi.cluster_size;
+
I’d feel better with an `assert(cluster_size1 >= 0 && cluster_size2 >= 0);`
here.
+ if (cluster_size1 > 0 && cluster_size2 > 0) {
+ if (cluster_size1 == cluster_size2) {
+ block_size = cluster_size1;
+ } else {
+ block_size = MIN(cluster_size1, cluster_size2);
+ qprintf(quiet, "%s and %s has different cluster sizes: %d and %d "
s/has/have/
+ "correspondingly. Use minimum as block-size for "
s/correspondingly/respectively/; s/Use/Using/ (“Use” sounds like an imperative)
+ "accuracy: %d. %s\n",
+ fname1, fname2, cluster_size1,
+ cluster_size2, block_size, note);
+ }
+ } else if (cluster_size1 == 0 && cluster_size2 == 0) {
+ block_size = default_block_size;
+ qprintf(quiet, "Neither of %s and %s has explicit cluster size. Use "
s/has/have an/; s/Use/Using/
+ "default of %d bytes. %s\n", fname1, fname2, block_size, note);
+ } else {
+ block_size = MAX(cluster_size1, cluster_size2);
+ qprintf(quiet, "%s has explicit cluster size of %d and %s "
s/has/has an/
+ "doesn't have one. Use %d as block-size. %s\n",
s/Use/Using/
Hanna
+ cluster_size1 ? fname1 : fname2, block_size,
+ cluster_size1 ? fname2 : fname1,
+ block_size, note);
+ }
+
+ return block_size;
+}
+
/*
* Compares two images. Exit codes:
*