* [Qemu-devel] [PATCH] qemu-img: Speed up comparing empty/zero images
@ 2016-01-13 8:37 Fam Zheng
2016-01-13 12:29 ` Paolo Bonzini
2016-01-18 13:32 ` Kevin Wolf
0 siblings, 2 replies; 3+ messages in thread
From: Fam Zheng @ 2016-01-13 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, qemu-block
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 <famz@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)) {
+ 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,
--
2.4.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-img: Speed up comparing empty/zero images
2016-01-13 8:37 [Qemu-devel] [PATCH] qemu-img: Speed up comparing empty/zero images Fam Zheng
@ 2016-01-13 12:29 ` Paolo Bonzini
2016-01-18 13:32 ` Kevin Wolf
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2016-01-13 12:29 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Kevin Wolf, qemu-block
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 <famz@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 <pbonzini@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,
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-img: Speed up comparing empty/zero images
2016-01-13 8:37 [Qemu-devel] [PATCH] qemu-img: Speed up comparing empty/zero images Fam Zheng
2016-01-13 12:29 ` Paolo Bonzini
@ 2016-01-18 13:32 ` Kevin Wolf
1 sibling, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2016-01-18 13:32 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, qemu-block
Am 13.01.2016 um 09:37 hat Fam Zheng geschrieben:
> 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 <famz@redhat.com>
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-18 13:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-13 8:37 [Qemu-devel] [PATCH] qemu-img: Speed up comparing empty/zero images Fam Zheng
2016-01-13 12:29 ` Paolo Bonzini
2016-01-18 13:32 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).