On 02.05.19 15:58, 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 d9b609b3f0..7f20858cb9 100644 > --- a/qemu-img.c > +++ b/qemu-img.c
[...]
> @@ -3422,6 +3428,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);
This will always return true because it definitely is allocated in @bs,
or we wouldn’t be here. (We just checked that with
bdrv_is_allocated().) I think @top should be backing_bs(bs).
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.
>
signature.asc
Description: OpenPGP digital signature
