Re: [Qemu-devel] [PATCH v5 22/42] block: Use CAFs in bdrv_get_allocated_file_size()

[Max Reitz]

On 14.06.19 17:41, Vladimir Sementsov-Ogievskiy wrote:
> 13.06.2019 1:09, Max Reitz wrote:
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>   block.c | 26 ++++++++++++++++++++++++--
>>   1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 11b7ba8cf6..856d9b58be 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -4511,15 +4511,37 @@ exit:
>>   int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>>   {
>>       BlockDriver *drv = bs->drv;
>> +    BlockDriverState *storage_bs, *metadata_bs;
>> +
>>       if (!drv) {
>>           return -ENOMEDIUM;
>>       }
>> +
>>       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);
>> +
>> +    storage_bs = bdrv_storage_bs(bs);
>> +    metadata_bs = bdrv_metadata_bs(bs);
>> +
>> +    if (storage_bs) {
>> +        int64_t data_size, metadata_size = 0;
>> +
>> +        data_size = bdrv_get_allocated_file_size(storage_bs);
>> +        if (data_size < 0) {
>> +            return data_size;
>> +        }
>> +
>> +        if (storage_bs != metadata_bs) {
>> +            metadata_size = bdrv_get_allocated_file_size(metadata_bs);
>> +            if (metadata_size < 0) {
>> +                return metadata_size;
>> +            }
>> +        }
>> +
>> +        return data_size + metadata_size;
>>       }
>> +
>>       return -ENOTSUP;
>>   }
>>   
>>
> 
> Again, I dislike nailing down new fresh feature about separate metadata and 
> storage child
> to the generic block layer, as it's simple to imagine a driver which needs 
> three or more
> children to store all its data and metadata..

Yes, we have that, it’s VMDK.

> Isn't it better by default loop through all children and sum all their 
> allocated sizes?
> 
> Hmm, but we want exclude backing, yes? Still we may ignore it while iterating.

I want to object in that there could be drivers that have children that
should not count towards their allocated size other than COW backing
files.  But I actually cannot imagine a reasonable scenario.  (The only
reason why COW backing files should be excluded is because they are
generally listed separately.)

So, yes, that sounds good.

Max

signature.asc
Description: OpenPGP digital signature