Re: [Qemu-devel] [PATCH 2/2] qcow2: truncate the tail of the image file after shrinking the image

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2365 bytes --]

On 2017-09-21 18:16, Pavel Butsykin wrote:
> On 21.09.2017 18:28, Max Reitz wrote:
>> On 2017-09-20 15:58, Pavel Butsykin wrote:
>>> Now after shrinking the image, at the end of the image file, there
>>> might be a
>>> tail that probably will never be used. So we can find the last used
>>> cluster and
>>> cut the tail.
>>>
>>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>>> ---
>>>   block/qcow2-refcount.c | 21 +++++++++++++++++++++
>>>   block/qcow2.c          | 19 +++++++++++++++++++
>>>   block/qcow2.h          |  1 +
>>>   3 files changed, 41 insertions(+)
>>>
>>> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
>>> index 88d5a3f1ad..5e221a166c 100644
>>> --- a/block/qcow2-refcount.c
>>> +++ b/block/qcow2-refcount.c
>>> @@ -3181,3 +3181,24 @@ out:
>>>       g_free(reftable_tmp);
>>>       return ret;
>>>   }
>>> +
>>> +int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size)
>>> +{
>>> +    BDRVQcow2State *s = bs->opaque;
>>> +    int64_t i, last_cluster, nb_clusters = size_to_clusters(s, size);
>>> +    uint64_t refcount;
>>> +
>>> +    for (i = 0, last_cluster = 0; i < nb_clusters; i++) {
>>> +        int ret = qcow2_get_refcount(bs, i, &refcount);
>>> +        if (ret < 0) {
>>> +            fprintf(stderr, "Can't get refcount for cluster %"
>>> PRId64 ": %s\n",
>>> +                    i, strerror(-ret));
>>> +            continue;
>>> +        }
>>> +
>>> +        if (refcount > 0) {
>>> +            last_cluster = i;
>>> +        }
>>> +    }
>>> +    return last_cluster;
>>> +}
>>
>> Wouldn't it make more sense to start from the end of the image?
> 
> If this will reduce the iterations, then yes. But it will depend on the
> situation. If you truncate the image more than half, it can increase the
> number of iterations. But intuitively it seems that to start from the
> end would be better :)

That's one thing (also, I think usually the end should coincide with
some allocated cluster, but yes, that's just intuition :-)); but also,
this would simplify things a bit because we would no longer need the
last_cluster variable (the loop would just stop at the first allocated
cluster).

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]