Patchwork [3/6] qcow2: flush caches in qcow2_alloc_bytes()

login
register
mail settings
Submitter Stefan Hajnoczi
Date Feb. 19, 2013, 3:45 p.m.
Message ID <1361288706-13929-4-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/221719/
State New
Headers show

Comments

Stefan Hajnoczi - Feb. 19, 2013, 3:45 p.m.
It is not completely clear to me what is being flushed in
qcow2_alloc_bytes() but bdrv_flush(bs->file) is probably wrong.  At
least the refcount cache should be flushed since this function calls
update_cluster_refcount().

To be on the safe side, call the full bdrv_flush(bs).  This flushes all
caches and the underlying file itself.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Kevin Wolf - Feb. 20, 2013, 10:08 a.m.
On Tue, Feb 19, 2013 at 04:45:03PM +0100, Stefan Hajnoczi wrote:
> It is not completely clear to me what is being flushed in
> qcow2_alloc_bytes() but bdrv_flush(bs->file) is probably wrong.  At
> least the refcount cache should be flushed since this function calls
> update_cluster_refcount().
> 
> To be on the safe side, call the full bdrv_flush(bs).  This flushes all
> caches and the underlying file itself.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

To me it looks as if qcow2_alloc_compressed_cluster_offset() would need
a call to qcow2_cache_set_dependency() like there is already for the
uncompressed case in qcow2_alloc_cluster_link_l2(). Once we have that,
the unconditional flush can be removed. I guess this will be a major
performance improvement for converting to compressed qcow2.

The bdrv_flush() ended up here only because I pushed the flush from
qcow2_alloc_clusters() to its callers so I could remove it from the
common fast path. All surviving instances from this still need review.

Kevin

Patch

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 05b5ec9..4ef3dac 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -663,7 +663,7 @@  int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
         }
     }
 
-    bdrv_flush(bs->file);
+    bdrv_flush(bs);
     return offset;
 }