qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/4] qemu-img: implement compare --stat


From: Hanna Reitz
Subject: Re: [PATCH v3 1/4] qemu-img: implement compare --stat
Date: Tue, 2 Nov 2021 13:40:56 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

On 28.10.21 12:24, Vladimir Sementsov-Ogievskiy wrote:
With new option qemu-img compare will not stop at first mismatch, but
instead calculate statistics: how many clusters with different data,
how many clusters with equal data, how many clusters were unallocated
but become data and so on.

We compare images chunk by chunk. Chunk size depends on what
block_status returns for both images. It may return less than cluster
(remember about qcow2 subclusters), it may return more than cluster (if
several consecutive clusters share same status). Finally images may
have different cluster sizes. This all leads to ambiguity in how to
finally compare the data.

What we can say for sure is that, when we compare two qcow2 images with
same cluster size, we should compare clusters with data separately.
Otherwise, if we for example compare 10 consecutive clusters of data
where only one byte differs we'll report 10 different clusters.
Expected result in this case is 1 different cluster and 9 equal ones.

So, to serve this case and just to have some defined rule let's do the
following:

1. Select some block-size for compare procedure. In this commit it must
    be specified by user, next commit will add some automatic logic and
    make --block-size optional.

2. Go chunk-by-chunk using block_status as we do now with only one
    differency:
    If block_status() returns DATA region that intersects block-size
    aligned boundary, crop this region at this boundary.

This way it's still possible to compare less than cluster and report
subcluster-level accuracy, but we newer compare more than one cluster
of data.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
  docs/tools/qemu-img.rst |  18 +++-
  qemu-img.c              | 210 +++++++++++++++++++++++++++++++++++++---
  qemu-img-cmds.hx        |   4 +-
  3 files changed, 216 insertions(+), 16 deletions(-)

[...]

diff --git a/qemu-img.c b/qemu-img.c
index f036a1d428..0cb7cebe91 100644
--- a/qemu-img.c
+++ b/qemu-img.c

[...]

@@ -1465,7 +1602,7 @@ static int img_compare(int argc, char **argv)
      }
while (offset < total_size) {
-        int status1, status2;
+        block_end = QEMU_ALIGN_UP(offset + 1, block_size);

Without --stat, `block_size` is 0, and then this is a division by zero.

My compiler seems clever enough to skip this division if `stat == NULL`, but when I add a `printf("%li\n", block_size);` after this line and do a compare without --stat, qemu-img aborts.  I don’t think we should rely on the compiler optimization working here.

(Sorry I didn’t notice this in v2, I just noticed it because I was trying to find out whether `block_size` really needs to be a power of two as Eric proposed...  The good news is that I don’t think it needs to be a power of two (still might make sense to require it), but, well, the bad news is that I found this.)

Hanna




reply via email to

[Prev in Thread] Current Thread [Next in Thread]