[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.12 v2] qcow2: Reset free_cluster_index whe
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH for-2.12 v2] qcow2: Reset free_cluster_index when allocating a new refcount block |
Date: |
Wed, 21 Mar 2018 15:10:01 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 21 Mar 2018 02:52:38 PM CET, Kevin Wolf wrote:
>> qemu-img create -f qcow2 -o cluster_size=512 hd.qcow2 1M
>> qemu-io -c 'write 0 124k' hd.qcow2
>> qemu-io -c 'write 124k 512' hd.qcow2
>
> So if I understand correctly, this is what we get:
>
> 0x20000: free
> 0x20200: refcount block
> 0x20400: data cluster
> 0x20600: potential next data cluster
Yes.
>> What this patch does is reset s->free_cluster_index to its previous
>> value when alloc_refcount_block() returns -EAGAIN. This way the caller
>> will try to allocate again the original clusters if they are still
>> free.
>
> This is an improvement, because now we should avoid the unused cluster:
>
> 0x20000: data cluster
> 0x20200: refcount block
> 0x20400: potential next data cluster
That's correct.
> But now the data clusters are fragmented. Should we try to change the
> logic so that already the refcount block allocation can make use of
> the free space? That is:
>
> 0x20000: refcount block
> 0x20200: data cluster
> 0x20400: contiguous potential next data cluster
Well, the clusters before 0x20000 are also data clusters so there's
going to be fragmentation anyway. Also, if you're writing more than 1
data cluster in a single request when the refcount block allocation
happens all those new data clusters are still going to be contiguous
(before the new refcount block).
Another possibility that I considered was to make alloc_clusters_noref()
return the offset but let free_cluster_index untouched until the
refcounts are correct, but this requires more code and I fear that
keeping free_cluster_index pointing to the clusters we're trying to
allocate can lead to unexpected surprises.
The solution I chose is -I think- the simplest and easiest to audit.
On a related note, I'm using a script that I wrote myself to debug qcow2
images. It prints the contents of the header and lists all host
clusters, indicating the type of each one. I find it useful to debug
problems like this one. If there's nothing similar already existing I
can post it and we could put it in the scripts/ directory.
Berto
Re: [Qemu-devel] [PATCH for-2.12 v2] qcow2: Reset free_cluster_index when allocating a new refcount block, Kevin Wolf, 2018/03/21