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 <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?". 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
