Re: [Qemu-devel] [PATCH v2 5/7] qcow2: implement lazy refcounts

[Kevin Wolf <kwolf@redhat.com>]

Am 25.07.2012 14:21, schrieb Stefan Hajnoczi:
> Lazy refcounts is a performance optimization for qcow2 that postpones
> refcount metadata updates and instead marks the image dirty.  In the
> case of crash or power failure the image will be left in a dirty state
> and repaired next time it is opened.
> 
> Reducing metadata I/O is important for cache=writethrough and
> cache=directsync because these modes guarantee that data is on disk
> after each write (hence we cannot take advantage of caching updates in
> RAM).  Refcount metadata is not needed for guest->file block address
> translation and therefore does not need to be on-disk at the time of
> write completion - this is the motivation behind the lazy refcount
> optimization.
> 
> The lazy refcount optimization must be enabled at image creation time:
> 
>   qemu-img create -f qcow2 -o compat=1.1,lazy_refcounts=on a.qcow2 10G
>   qemu-system-x86_64 -drive if=virtio,file=a.qcow2,cache=writethrough
> 
> Update qemu-iotests 031 and 036 since the extension header size changes
> when we add feature bit table entries.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block/qcow2-cluster.c      |    5 +++-
>  block/qcow2.c              |   71 +++++++++++++++++++++++++++++++++++++++++---
>  block/qcow2.h              |   13 ++++++++
>  block_int.h                |   26 ++++++++--------
>  tests/qemu-iotests/031.out |   12 ++++----
>  tests/qemu-iotests/036.out |    2 +-
>  6 files changed, 105 insertions(+), 24 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index d7e0e19..e179211 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -662,7 +662,10 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
>          qcow2_cache_depends_on_flush(s->l2_table_cache);
>      }
>  
> -    qcow2_cache_set_dependency(bs, s->l2_table_cache, s->refcount_block_cache);
> +    if (qcow2_need_accurate_refcounts(s)) {
> +        qcow2_cache_set_dependency(bs, s->l2_table_cache,
> +                                   s->refcount_block_cache);
> +    }
>      ret = get_cluster_table(bs, m->offset, &l2_table, &l2_index);
>      if (ret < 0) {
>          goto err;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7fe1567..d48527f7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -215,6 +215,39 @@ static void report_unsupported_feature(BlockDriverState *bs,
>  }
>  
>  /*
> + * Sets the dirty bit and flushes afterwards if necessary.
> + *
> + * The incompatible_features bit is only set if the image file header was
> + * updated successfully.  Therefore it is not required to check the return
> + * value of this function.
> + */
> +static int qcow2_mark_dirty(BlockDriverState *bs)
> +{
> +    BDRVQcowState *s = bs->opaque;
> +    uint64_t val;
> +    int ret;
> +
> +    if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) {
> +        return 0; /* already dirty */
> +    }
> +
> +    val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY);
> +    ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features),
> +                      &val, sizeof(val));

If you respin, I think would be nice to have either an assert(s->version
== 3) before writing to qcow3 header fields, or to use
qcow2_update_header() in the first place.

Kevin