Re: [Qemu-devel] [PATCH v6 22/42] block: Fix bdrv_get_allocated_file_size's fallback

[Max Reitz]

On 10.08.19 18:41, Vladimir Sementsov-Ogievskiy wrote:
> 09.08.2019 19:13, Max Reitz wrote:
>> If the driver does not implement bdrv_get_allocated_file_size(), we
>> should fall back to cumulating the allocated size of all non-COW
>> children instead of just bs->file.
>>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>   block.c | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 1070aa1ba9..6e1ddab056 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -4650,9 +4650,27 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState 
>> *bs)
>>       if (drv->bdrv_get_allocated_file_size) {
>>           return drv->bdrv_get_allocated_file_size(bs);
>>       }
>> -    if (bs->file) {
>> -        return bdrv_get_allocated_file_size(bs->file->bs);
>> +
>> +    if (!QLIST_EMPTY(&bs->children)) {
>> +        BdrvChild *child;
>> +        int64_t child_size, total_size = 0;
>> +
>> +        QLIST_FOREACH(child, &bs->children, next) {
>> +            if (child == bdrv_filtered_cow_child(bs)) {
>> +                /* Ignore COW backing files */
>> +                continue;
>> +            }
>> +
>> +            child_size = bdrv_get_allocated_file_size(child->bs);
>> +            if (child_size < 0) {
>> +                return child_size;
>> +            }
>> +            total_size += child_size;
>> +        }
>> +
>> +        return total_size;
>>       }
>> +
>>       return -ENOTSUP;
>>   }
>>   
>>
> 
> Hmm..
> 
> 1. No children -> -ENOTSUP
> 2. Only cow child -> 0
> 3. Some non-cow children -> SUM
> 
> It's all arguable (the strictest way is -ENOTSUP in either case),
> but if we want to fallback to SUM of non-cow children, 1. and 2. should return
> the same.

I don’t think 2 is possible at all.  If you have a COW child, you need
some other child to COW to.

And in the weird (and probably impossible) case where a node really only
has a COW child, I’d say it’s correct that it has a disk size of 0 –
because it hasn’t COWed anything yet.  (Just like a new qcow2 image with
a backing file only has its metadata as its disk size.)

Max

signature.asc
Description: OpenPGP digital signature