qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]