[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] qemu-img: Speed up comparing empty/zero images
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [PATCH] qemu-img: Speed up comparing empty/zero images |
Date: |
Wed, 13 Jan 2016 13:29:18 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 |
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 <address@hidden>
> ---
> 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 <address@hidden>
> + 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,
>