From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53462) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFhla-0004kD-Br for qemu-devel@nongnu.org; Wed, 13 Mar 2013 05:14:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFhlU-0008CX-7n for qemu-devel@nongnu.org; Wed, 13 Mar 2013 05:14:34 -0400 Date: Wed, 13 Mar 2013 10:14:23 +0100 From: Kevin Wolf Message-ID: <20130313091423.GA2309@dhcp-200-207.str.redhat.com> References: <1362589321-21402-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1362589321-21402-1-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH] qcow2: make is_allocated return true for zero clusters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, stefanha@redhat.com, qemu-stable@nongnu.org Am 06.03.2013 um 18:02 hat Paolo Bonzini geschrieben: > Otherwise, live migration of the top layer will miss zero clusters and > let the backing file show through. This also matches what is done in qed. > > QCOW2_CLUSTER_ZERO clusters are invalid in v2 image files. Check this > directly in qcow2_get_cluster_offset instead of replicating the test > everywhere. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Paolo Bonzini Can you add a test case for this? Also is_allocated() probably is the wrong interface now because it can mean different things. The content of a zero cluster is indeed defined by the image, but it may or may not be fully allocated yet. Have you checked if the callers use it consistently in the former way? > block/qcow2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index 56fccf9..bb04432 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -454,6 +454,9 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, > *cluster_offset &= L2E_COMPRESSED_OFFSET_SIZE_MASK; > break; > case QCOW2_CLUSTER_ZERO: > + if (s->qcow_version < 3) { > + return -EIO; > + } This leaks a cache entry. Kevin