From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33020) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V0R9o-00006z-PD for qemu-devel@nongnu.org; Sat, 20 Jul 2013 03:00:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V0R9n-0003Qq-AM for qemu-devel@nongnu.org; Sat, 20 Jul 2013 03:00:44 -0400 Received: from mail-ee0-x22b.google.com ([2a00:1450:4013:c00::22b]:44220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V0R9n-0003Qb-3T for qemu-devel@nongnu.org; Sat, 20 Jul 2013 03:00:43 -0400 Received: by mail-ee0-f43.google.com with SMTP id l10so2783154eei.30 for ; Sat, 20 Jul 2013 00:00:41 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51EA358A.6040304@redhat.com> Date: Sat, 20 Jul 2013 09:00:26 +0200 From: Paolo Bonzini 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> In-Reply-To: <51E939F2.3020009@kamp.de> 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: Peter Lieven Cc: famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com 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?". So this patch must take those slightly different semantics into account. Paolo