Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks

[Kevin Wolf <kwolf@redhat.com>]

Am 12.12.2011 03:03, schrieb Lan Tianyu:
> This patch enables allocating new refcount blocks and so then kvm tools
> could expand qcow2 image much larger.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  tools/kvm/disk/qcow.c |  105 +++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 89 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
> index e139fa5..929ba69 100644
> --- a/tools/kvm/disk/qcow.c
> +++ b/tools/kvm/disk/qcow.c
> @@ -12,6 +12,7 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <errno.h>
>  #ifdef CONFIG_HAS_ZLIB
>  #include <zlib.h>
>  #endif
> @@ -20,6 +21,10 @@
>  #include <linux/kernel.h>
>  #include <linux/types.h>
>  
> +static int update_cluster_refcount(struct qcow *q, u64 clust_idx, u16 append);
> +static int qcow_write_refcount_table(struct qcow *q);
> +static u64 qcow_alloc_clusters(struct qcow *q, u64 size, int update_ref);
> +static void  qcow_free_clusters(struct qcow *q, u64 clust_start, u64 size);
>  
>  static inline int qcow_pwrite_sync(int fd,
>  	void *buf, size_t count, off_t offset)
> @@ -657,6 +662,56 @@ static struct qcow_refcount_block *refcount_block_search(struct qcow *q, u64 off
>  	return rfb;
>  }
>  
> +static struct qcow_refcount_block *qcow_grow_refcount_block(struct qcow *q,
> +	u64 clust_idx)
> +{
> +	struct qcow_header *header = q->header;
> +	struct qcow_refcount_table *rft = &q->refcount_table;
> +	struct qcow_refcount_block *rfb;
> +	u64 new_block_offset;
> +	u64 rft_idx;
> +
> +	rft_idx = clust_idx >> (header->cluster_bits -
> +		QCOW_REFCOUNT_BLOCK_SHIFT);
> +
> +	if (rft_idx >= rft->rf_size) {
> +		pr_warning("Don't support grow refcount block table");
> +		return NULL;
> +	}
> +
> +	new_block_offset = qcow_alloc_clusters(q, q->cluster_size, 0);
> +	if (new_block_offset < 0)
> +		return NULL;
> +
> +	rfb = new_refcount_block(q, new_block_offset);
> +	if (!rfb)
> +		return NULL;
> +
> +	memset(rfb->entries, 0x00, q->cluster_size);
> +	rfb->dirty = 1;
> +
> +	/* write refcount block */
> +	if (write_refcount_block(q, rfb) < 0)
> +		goto free_rfb;
> +
> +	if (cache_refcount_block(q, rfb) < 0)
> +		goto free_rfb;
> +
> +	rft->rf_table[rft_idx] = cpu_to_be64(new_block_offset);
> +	if (qcow_write_refcount_table(q) < 0)
> +		goto free_rfb;
> +
> +	if (update_cluster_refcount(q, new_block_offset >>
> +		    header->cluster_bits, 1) < 0)
> +		goto free_rfb;

This order is unsafe, you could end up with a refcount block that is
already in use, but still has a refcount of 0.

> +
> +	return rfb;
> +
> +free_rfb:
> +	free(rfb);
> +	return NULL;
> +}
> +
>  static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 clust_idx)
>  {
>  	struct qcow_header *header = q->header;
> @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64
>  
>  	rft_idx = clust_idx >> (header->cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT);
>  	if (rft_idx >= rft->rf_size)
> -		return NULL;
> +		return (void *)-ENOSPC;

Is this allowed style in kvm-tool? :-/

Kevin