Full summary What drivers returns?
Filters and raw are just broken together with their BDRV_BLOCK_RAW. Format drivers behaves as follows (except for backing-not-supporting, which should be fixed): 0 - go to backing (not-backing supporting will never return it, backing-supporting will return it even if there is not backing file, but read is guaranteed to return zeroes, if there is no backing file at the moment) ZERO - zero at this level DATA - data at this level DATA | ZERO actually never returned. Protocol drivers 0 - fs-unallocated, read may return any garbage (null-co, iscsi) from SCSI: Logical Block Provisioning Read Zeros (LBPRZ) bit 1 If the logical block provisioning read zeros (LBPRZ) bit is set to one, then, for an unmapped LBA specified by a read operation, the deviceserver shall send user data with all bits set to zero to the data-in buffer. 0 If the TPRZ bit is set to zero, then, for an unmapped LBA specified by a read operation, the device server may send user data with all bitsset to any value to the data-in buffer. so, yes, this 0 actually have significant meaning. And null-co matches it too. And nbd should be fixed to match it, I think. ZERO - fs-unallocated, reads as zero DATA - fs-allocated data or safe default ZERO | DATA - only nbd may return it, and it seems wrong. let's fix it. So, seems, we may can with it as is. The only thing to be documented is meaning of zero status returned by driver: for backing-supporting it means go-to-backing, with a guarantee to read zeroes if there is no backing file, for backing-not-supporting it means most probably not occupy disk space, read returns garbage. And format drivers without backing support should never return 0. =================== How it is used? And here we definitely fail. As there are a lot of places where 0 return of drivers is misused: we consider it fs-unallocated, but it may be just go-to-backing, we consider it go-to-backing, but it may be fs-unallocated. let's go again through our public API = bdrv_block_status = img-convert and bdrv_make_zero wants only zero and go-to-backing information img-map is better to distinguish fs-unallocated and go-to-backing and report them in different way. = bdrv_block_status_above = most callers needs only zero and go-to-backing information, others are doing wrong things and should be rewritten anyway = bdrv_is_allocated = most callers need only go-to-backing information. io-alloc io-map - are reporting utilities, and they probably want to show fs-unallocated as well... but we never documented, how actually img-map, io-map and io-alloc should work and what they are trying to say :). Anyway, they may use same interface as img-map = bdrv_is_allocated_above = callers only need go-to-backing information OK, sounds good. So, for most things we only need zero/data/go-to-backing information. fs-unallocated should be treated as DATA, it's garbage, but on read we will directly read this garbage, so we should consider it DATA. And only for reporting through img-map, io-alloc and io-map we may want fs-unallocated information. Do we actually need it? Basing on this, I think that there should be four block-status types: ZERO: unrelated to backing, reads as zero from this layer DATA: unrelated to backing, reads from this layer, may be non-zero BACKING: go to backing for information, reads from backing as well RAW: I'm a filter or 'raw' driver, I don't know what to do. I give you my child and offset in it, take care of it. Be careful: it may be backing child or not, don't break your backing-chain loop on me! So we may require that at most one of DATA, ZERO, RAW is set. And if nothing is set it means BACKING. And if we want to report fs-unallocated things, we just need additional flag for it, like BDRV_FS_UNALLOCATED_GARBAGE, which may be combined with DATA type chunks. Or, may be even better, to split type from flags: block_status will return type, which is one of these four types, and other things (RECURSE, EOF, OFFSET_VALID, FS_UNALLOCATED_GARBAGE) goes to additional flags out-argument. Note also, that the only user of @map and @file parameters is img-map. And all other callers have to pass NULL, NULL. I think, this definitely should be refactored. -- Best regards, Vladimir