On 13/01/2016 09:37, Fam Zheng wrote: > Two empty raw files are always compared by actually reading data even if > there is no data, because BDRV_BLOCK_ZERO is considered "allocated" in > bdrv_is_allocated_above(). That is inefficient. > > Use bdrv_get_block_status_above() for more information, and skip the > consecutive zero sectors. > > This brings a huge speed up in comparing sparse/empty raw images: > > $ qemu-img create a 1G > > $ time ~/build/master/bin/qemu-img compare a a > Images are identical. > > real 0m6.583s > user 0m0.191s > sys 0m6.367s > > $ time qemu-img compare a a > Images are identical. > > real 0m0.033s > user 0m0.003s > sys 0m0.031s > > Signed-off-by: Fam Zheng <f...@redhat.com> > --- > qemu-img.c | 45 ++++++++++++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 15 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index 3d48b4f..82f704f 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1074,28 +1074,50 @@ static int img_compare(int argc, char **argv) > } > > for (;;) { > + int64_t status1, status2; > 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); > - if (allocated1 < 0) { > + status1 = bdrv_get_block_status_above(bs1, NULL, sector_num, > + total_sectors1 - sector_num, > + &pnum1); > + if (status1 < 0) { > ret = 3; > error_report("Sector allocation test failed for %s", filename1); > goto out; > } > + allocated1 = status1 & BDRV_BLOCK_ALLOCATED; > > - allocated2 = bdrv_is_allocated_above(bs2, NULL, sector_num, > nb_sectors, > - &pnum2); > - if (allocated2 < 0) { > + status2 = bdrv_get_block_status_above(bs2, NULL, sector_num, > + total_sectors2 - sector_num, > + &pnum2); > + if (status2 < 0) { > ret = 3; > error_report("Sector allocation test failed for %s", filename2); > goto out; > } > - nb_sectors = MIN(pnum1, pnum2); > + allocated2 = status2 & BDRV_BLOCK_ALLOCATED; > + if (pnum1) { > + nb_sectors = MIN(nb_sectors, pnum1); > + } > + if (pnum2) { > + nb_sectors = MIN(nb_sectors, pnum2); > + } > > - if (allocated1 == allocated2) { > + if (strict) { > + if ((status1 & ~BDRV_BLOCK_OFFSET_MASK) != > + (status2 & ~BDRV_BLOCK_OFFSET_MASK)) {
This is not exactly the same definition as before, but if that's okay: Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> > + ret = 1; > + qprintf(quiet, "Strict mode: Offset %" PRId64 > + " block status mismatch!\n", > + sectors_to_bytes(sector_num)); > + goto out; > + } > + } > + if ((status1 & BDRV_BLOCK_ZERO) && (status2 & BDRV_BLOCK_ZERO)) { > + nb_sectors = MIN(pnum1, pnum2); > + } else if (allocated1 == allocated2) { > if (allocated1) { > ret = blk_read(blk1, sector_num, buf1, nb_sectors); > if (ret < 0) { > @@ -1123,13 +1145,6 @@ static int img_compare(int argc, char **argv) > } > } > } else { > - if (strict) { > - ret = 1; > - qprintf(quiet, "Strict mode: Offset %" PRId64 > - " allocation mismatch!\n", > - sectors_to_bytes(sector_num)); > - goto out; > - } > > if (allocated1) { > ret = check_empty_sectors(blk1, sector_num, nb_sectors, >