On Wed, Feb 06, 2013 at 01:32:06PM +0100, Benoît Canet wrote: > +static int qcow2_dedup_alloc(BlockDriverState *bs) > +{ > + BDRVQcowState *s = bs->opaque; > + int ret; > + > + ret = qcow2_do_table_init(bs, > + &s->dedup_table, > + s->dedup_table_offset, > + s->dedup_table_size, > + false); > + > + if (ret < 0) { > + return ret; > + } > + > + s->dedup_tree_by_hash = g_tree_new_full(qcow2_dedup_compare_by_hash, > NULL, > + NULL, > + > qcow2_dedup_destroy_qcow_hash_node); > + s->dedup_tree_by_sect = g_tree_new_full(qcow2_dedup_compare_by_offset, > + NULL, NULL, NULL); > + > + s->dedup_cluster_cache = qcow2_cache_create(bs, DEDUP_CACHE_SIZE, > + s->hash_block_size);
I don't remember any patches that set a dependency on other caches. This suggests that the write ordering for metadata updates hasn't been considered yet. The cache dependencies allow ordering to be expressed between caches. This way, the refcounts will always be written before the L2 table updates, for example. > + > + return 0; > +} > + > +static void qcow2_dedup_free(BlockDriverState *bs) > +{ > + BDRVQcowState *s = bs->opaque; > + g_free(s->dedup_table); > + > + qcow2_cache_flush(bs, s->dedup_cluster_cache); > + qcow2_cache_destroy(bs, s->dedup_cluster_cache); > + g_tree_destroy(s->dedup_tree_by_sect); > + g_tree_destroy(s->dedup_tree_by_hash); > +} > + > +int qcow2_dedup_init(BlockDriverState *bs) > +{ > + BDRVQcowState *s = bs->opaque; > + int ret = 0; > + > + s->has_dedup = true; > + > + ret = qcow2_dedup_alloc(bs); > + > + if (ret < 0) { > + return ret; > + } > + > + /* if we are read-only we don't load the deduplication table */ > + if (bs->read_only) { > + return 0; Might be worth mentioning that has_dedup remains set to true here so that .bdrv_check() still looks into the dedup tables for read-only images (qemu-img check is read-only by default).