| Message ID | 1361288706-13929-4-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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; }
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(-)