From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [PATCH] kvm tools, qcow: Add the support for copy-on-write clusters Date: Mon, 21 Nov 2011 09:59:51 +0100 Message-ID: <4ECA1307.2060208@redhat.com> References: <1321859562-21042-1-git-send-email-tianyu.lan@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: penberg@kernel.org, kvm@vger.kernel.org, asias.hejun@gmail.com, levinsasha928@gmail.com, prasadjoshi124@gmail.com To: Lan Tianyu Return-path: Received: from mx1.redhat.com ([209.132.183.28]:49135 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755054Ab1KUI4t (ORCPT ); Mon, 21 Nov 2011 03:56:49 -0500 In-Reply-To: <1321859562-21042-1-git-send-email-tianyu.lan@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: Am 21.11.2011 08:12, schrieb Lan Tianyu: > When meeting request to write the cluster without copied flag, > allocate a new cluster and write original data with modification > to the new cluster. This also adds support for the writing operation > of the qcow2 compressed image. After testing, image file can pass > through "qemu-img check". The performance is needed to be improved. > > Signed-off-by: Lan Tianyu > --- > tools/kvm/disk/qcow.c | 411 +++++++++++++++++++++++++++++------------- > tools/kvm/include/kvm/qcow.h | 2 + > 2 files changed, 285 insertions(+), 128 deletions(-) > @@ -766,122 +872,166 @@ static ssize_t qcow_write_cluster(struct qcow *q, u64 offset, void *buf, u32 src > if (l2t_idx >= l2t_size) > return -1; > > - clust_off = get_cluster_offset(q, offset); > - if (clust_off >= clust_sz) > - return -1; > - > - len = clust_sz - clust_off; > - if (len > src_len) > - len = src_len; > - > - mutex_lock(&q->mutex); > - > l2t_offset = be64_to_cpu(l1t->l1_table[l1t_idx]); > - if (l2t_offset & QCOW2_OFLAG_COMPRESSED) { > - pr_warning("compressed clusters are not supported"); > - goto error; > - } > - if (!(l2t_offset & QCOW2_OFLAG_COPIED)) { > - pr_warning("L2 copy-on-write clusters are not supported"); > - goto error; > - } > - > - l2t_offset &= QCOW2_OFFSET_MASK; > - if (l2t_offset) { > - /* read and cache l2 table */ > + if (l2t_offset & QCOW2_OFLAG_COPIED) { > + l2t_offset &= ~QCOW2_OFLAG_COPIED; > l2t = qcow_read_l2_table(q, l2t_offset); > if (!l2t) > goto error; > } else { > - l2t = new_cache_table(q, l2t_offset); > - if (!l2t) > + l2t_new_offset = qcow_alloc_clusters(q, l2t_size*sizeof(u64)); > + if (l2t_new_offset < 0) > goto error; > > - /* Capture the state of the consistent QCOW image */ > - f_sz = file_size(q->fd); > - if (!f_sz) > - goto free_cache; > + l2t = new_cache_table(q, l2t_new_offset); > + if (!l2t) > + goto free_cluster; > > - /* Write the l2 table of 0's at the end of the file */ > - l2t_offset = qcow_write_l2_table(q, l2t->table); > - if (!l2t_offset) > + if (l2t_offset) { > + l2t = qcow_read_l2_table(q, l2t_offset); > + if (!l2t) > + goto free_cache; > + } else > + memset(l2t->table, 0x00, l2t_size * sizeof(u64)); > + > + /*write l2 table*/ > + l2t->dirty = 1; > + if (qcow_l2_cache_write(q, l2t) < 0) > goto free_cache; > > - if (cache_table(q, l2t) < 0) { > - if (ftruncate(q->fd, f_sz) < 0) > - goto free_cache; > + /*cache l2 table*/ > + cache_table(q, l2t); You're ignoring the return value here. Otherwise I didn't find any obvious problem in a quick scan. Kevin