In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.

After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense. Drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/vpc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 2d1eade146..555f9d8587 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -606,7 +606,6 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
         bdi->cluster_size = s->block_size;
     }
 
-    bdi->unallocated_blocks_are_zero = true;
     return 0;
 }
 
@@ -745,7 +744,7 @@ static int coroutine_fn vpc_co_block_status(BlockDriverState *bs,
     image_offset = get_image_offset(bs, offset, false, NULL);
     allocated = (image_offset != -1);
     *pnum = 0;
-    ret = 0;
+    ret = BDRV_BLOCK_ZERO;
 
     do {
         /* All sectors in a block are contiguous (without using the bitmap) */
-- 
2.21.0

On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
> In case when get_image_offset() returns -1, we do zero out the
> corresponding chunk of qiov. So, this should be reported as ZERO.
> 
> After block-status update, it never reports 0, so setting
> unallocated_blocks_are_zero doesn't make sense. Drop it.

Same analysis as in patch 1 as to the lone two clients that cared, and 
the fact that we are changing 'qemu-io -c map' output by reporting data 
as allocated now.  But I concur that as there is never a backing file, 
the change is not a regression, but rather a bug fix.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/vpc.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

While the commit message could be improved, the code change itself looks 
correct.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

07.05.2020 0:18, Eric Blake wrote:
> On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
>> In case when get_image_offset() returns -1, we do zero out the
>> corresponding chunk of qiov. So, this should be reported as ZERO.
>>
>> After block-status update, it never reports 0, so setting
>> unallocated_blocks_are_zero doesn't make sense. Drop it.
> 
> Same analysis as in patch 1 as to the lone two clients that cared, and the fact that we are changing 'qemu-io -c map' output by reporting data as allocated now.  But I concur that as there is never a backing file, the change is not a regression, but rather a bug fix.

Note that we have a problem with meaning of unallocated for protocol drivers. For example, iscsi block_status return 0, and it means "unallocated garbage", i.e. not occupying space, read may return any garbage. But vdi and vpc are format drivers, just don't support backing and they would better return ZERO status where appropriate.

> 
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/vpc.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> While the commit message could be improved, the code change itself looks correct.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 


-- 
Best regards,
Vladimir