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 <pbonz...@redhat.com> 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-sta...@nongnu.org Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> 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