From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47597) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1jyU-0001fG-Gv for qemu-devel@nongnu.org; Tue, 23 Jul 2013 17:18:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V1jyQ-00060h-SM for qemu-devel@nongnu.org; Tue, 23 Jul 2013 17:18:26 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:60033 helo=mx01.kamp.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1V1jyQ-0005zz-Is for qemu-devel@nongnu.org; Tue, 23 Jul 2013 17:18:22 -0400 Message-ID: <51EEF31A.90202@kamp.de> Date: Tue, 23 Jul 2013 23:18:18 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1373992168-26043-1-git-send-email-pbonzini@redhat.com> <1373992168-26043-11-git-send-email-pbonzini@redhat.com> <51E8E124.3050500@kamp.de> <51E90DD3.8030600@redhat.com> <51E90F28.9050409@kamp.de> <51E92D5B.6020600@redhat.com> <51E939F2.3020009@kamp.de> <51EA358A.6040304@redhat.com> In-Reply-To: <51EA358A.6040304@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Am 20.07.2013 09:00, schrieb Paolo Bonzini: > Il 19/07/2013 15:06, Peter Lieven ha scritto: >>>>> Il 19/07/2013 08:48, Peter Lieven ha scritto: >>>>>>> - return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum); >>>>>>> + int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, >>>>>>> pnum); >>>>>>> + return >>>>>>> + (ret & BDRV_BLOCK_DATA) || >>>>>>> + ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs)); >>>>>> i do also not understand the "((ret & BDRV_BLOCK_ZERO) && >>>>>> !bdrv_has_zero_init(bs))"; >>>>>> if a block is unallocated and reads as zero, but the device lacks zero >>>>>> init, it is declared as allocated with this, isn't it? >> If it is zero and allocated the API should return only BDRV_BLOCK_DATA >> and if it is zero and unallocated only BDRV_BLOCK_ZERO or not? >> >> What I mean is the new API shouldn't change the behaviour of the old >> bdrv_is_allocated(). >> It would have returned >> >> (ret & BDRV_BLOCK_DATA) regardless if BDRV_BLOCK_ZERO or not. > bdrv_is_allocated must return true for some zero clusters, even > if BDRV_BLOCK_DATA = 0. See > > commit 381b487d54ba18c73df9db8452028a330058c505 > Author: Paolo Bonzini > Date: Wed Mar 6 18:02:01 2013 +0100 > > qcow2: make is_allocated return true for zero clusters > > 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 > Signed-off-by: Stefan Hajnoczi > > I think the source of the confusion is that SCSI "GET LBA STATUS" > does not have to deal with backing files, bdrv_is_allocated must. > If bs->backing_hd != NULL, bdrv_is_allocated is not about allocation > of blocks in the SCSI sense; it's a query for "does the block come > from this BDS or rather from its backing file?". Ok, this makes it clearer. Thanks for pointing that out. the bdrv_get_block_status() will add the possibility to check for the allocation status which is a good thing. Peter