From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48463) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QntVI-0006VB-4y for qemu-devel@nongnu.org; Mon, 01 Aug 2011 10:30:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QntVG-0006IF-W3 for qemu-devel@nongnu.org; Mon, 01 Aug 2011 10:30:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:61789) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QntVG-0006IB-O7 for qemu-devel@nongnu.org; Mon, 01 Aug 2011 10:29:58 -0400 Message-ID: <4E36B915.1020804@redhat.com> Date: Mon, 01 Aug 2011 16:32:53 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311914994-20482-1-git-send-email-devin122@gmail.com> <1311914994-20482-19-git-send-email-devin122@gmail.com> In-Reply-To: <1311914994-20482-19-git-send-email-devin122@gmail.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 18/24] qcow2: add qcow2_map List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Devin Nakamura Cc: qemu-devel@nongnu.org Am 29.07.2011 06:49, schrieb Devin Nakamura: > Signed-off-by: Devin Nakamura > --- > block/qcow2-cluster.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > block/qcow2.c | 1 + > block/qcow2.h | 3 +++ > 3 files changed, 53 insertions(+), 0 deletions(-) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index ca56918..848f2ee 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -977,3 +977,52 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset, > > return 0; > } > + > +int qcow2_map(BlockDriverState *bs, uint64_t guest_offset, > + uint64_t host_offset, uint64_t contiguous_bytes) > +{ > + BDRVQcowState *s = bs->opaque; > + unsigned int nelms; > + > + > + if ((s->cluster_size - 1) & guest_offset) { Usually you would have guest_offset first. > + return -EINVAL; > + } > + > + if (contiguous_bytes % s->cluster_size) { > + return -EINVAL; > + } Any reason why the two checks are different? I don't really care if they use & or %, but they should use the same thing. host_offset isn't checked at all? > + > + nelms = s->l2_size / sizeof(uint64_t); s->l2_size is already in elements, not in bytes. > + > + while (contiguous_bytes > 0) { > + unsigned int l1_index, l2_index; > + uint64_t *l2_table; > + int ret; > + l1_index = guest_offset >> (s->l2_bits + s->cluster_bits); > + l2_index = (guest_offset >> s->cluster_bits) & (s->l2_size - 1); > + > + if (!s->l1_table[l1_index]) { > + ret = l2_allocate(bs, l1_index, &l2_table); > + if (ret) { > + return ret; > + } > + } else { > + ret = l2_load(bs, s->l1_table[l1_index] & ~QCOW_OFLAG_COPIED, &l2_table); > + if (ret) { > + return ret; > + } > + } Can't you use get_cluster_table() for this part? > + qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table); > + > + for (; l2_index < nelms && contiguous_bytes > 0; l2_index++) { > + l2_table[l2_index] = cpu_to_be64(host_offset | QCOW_OFLAG_COPIED); You should increase the refcount for host_offset and set QCOW_OFLAG_COPIED only if it becomes 1. Or maybe we should just fail bdrv_map if the old refcount is != 0. > + host_offset += s->cluster_size; > + contiguous_bytes -= s->cluster_size; > + } > + > + qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table); > + > + } > + return 0; > +} Kevin