Yes, my bad, resending as v5. Sam
> On 23 May 2019, at 19:14, Max Reitz <mre...@redhat.com> wrote: > > On 23.05.19 17:44, Sam Eiderman wrote: >> In the following case: >> >> (base) A <- B <- C (tip) >> >> when running: >> >> qemu-img rebase -b A C >> >> QEMU would read all sectors not allocated in the file being rebased (C) >> and compare them to the new base image (A), regardless of whether they >> were changed or even allocated anywhere along the chain between the new >> base and the top image (B). This causes many unneeded reads when >> rebasing an image which represents a small diff of a large disk, as it >> would read most of the disk's sectors. >> >> Instead, use bdrv_is_allocated_above() to reduce the number of >> unnecessary reads. >> >> Reviewed-by: Karl Heubaum <karl.heub...@oracle.com> >> Signed-off-by: Sam Eiderman <shmuel.eider...@oracle.com> >> Signed-off-by: Eyal Moscovici <eyal.moscov...@oracle.com> >> --- >> qemu-img.c | 25 ++++++++++++++++++++++++- >> 1 file changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 9bd0bb1e47..e6fd8e1a98 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c > > [...] > >> @@ -3437,6 +3443,23 @@ static int img_rebase(int argc, char **argv) >> continue; >> } >> >> + if (prefix_chain_bs) { >> + /* >> + * If cluster wasn't changed since prefix_chain, we don't >> need >> + * to take action >> + */ >> + ret = bdrv_is_allocated_above(bs, prefix_chain_bs, >> + offset, n, &n); > > I think you forgot to update the patch...? > > Max > >> + if (ret < 0) { >> + error_report("error while reading image metadata: %s", >> + strerror(-ret)); >> + goto out; >> + } >> + if (!ret) { >> + continue; >> + } >> + } >> + >> /* >> * Read old and new backing file and take into consideration that >> * backing files may be smaller than the COW image.