Am 21.03.2018 um 14:38 hat Alberto Garcia geschrieben: > When we try to allocate new clusters we first look for available ones > starting from s->free_cluster_index and once we find them we increase > their reference counts. Before we get to call update_refcount() to do > this last step s->free_cluster_index is already pointing to the next > cluster after the ones we are trying to allocate. > > During update_refcount() it may happen however that we also need to > allocate a new refcount block in order to store the refcounts of these > new clusters (and to complicate things further that may also require > us to grow the refcount table). After all this we don't know if the > clusters that we originally tried to allocate are still available, so > we return -EAGAIN to ask the caller to restart the search for free > clusters. > > This is what can happen in a common scenario: > > 1) We want to allocate a new cluster and we see that cluster N is > free. > > 2) We try to increase N's refcount but all refcount blocks are full, > so we allocate a new one at N+1 (where s->free_cluster_index was > pointing at). > > 3) Once we're done we return -EAGAIN to look again for a free > cluster, but now s->free_cluster_index points at N+2, so that's > the one we allocate. Cluster N remains unallocated and we have a > hole in the qcow2 file. > > This can be reproduced easily: > > qemu-img create -f qcow2 -o cluster_size=512 hd.qcow2 1M > qemu-io -c 'write 0 124k' hd.qcow2 > > After this the image has 132608 bytes (256 clusters), and the refcount > block is full. If we write 512 more bytes it should allocate two new > clusters: the data cluster itself and a new refcount block. > > qemu-io -c 'write 124k 512' hd.qcow2 > > However the image has now three new clusters (259 in total), and the > first one of them is empty (and unallocated): > > dd if=hd.qcow2 bs=512c skip=256 count=1 | hexdump -C > > If we write larger amounts of data in the last step instead of the 512 > bytes used in this example we can create larger holes in the qcow2 > file. > > 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. > > The output of iotest 026 also needs to be updated because now that > images have no holes some tests fail at a different point and the > number of leaked clusters is different. > > Signed-off-by: Alberto Garcia <[email protected]> > Reviewed-by: Eric Blake <[email protected]>
Thanks, applied to the block branch. Kevin
