[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/3] qemu-img: rebase: Reduce reads on in-cha
From: |
Sam Eiderman |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/3] qemu-img: rebase: Reduce reads on in-chain rebase |
Date: |
Thu, 23 May 2019 19:35:20 +0300 |
Yes, my bad, resending as v5.
Sam
> On 23 May 2019, at 19:14, Max Reitz <address@hidden> 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 <address@hidden>
>> Signed-off-by: Sam Eiderman <address@hidden>
>> Signed-off-by: Eyal Moscovici <address@hidden>
>> ---
>> 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.