The function should return actually used by top-level format driver
space (in it's .file). It differs from bdrv_get_allocated_file_size,
which returns allocated on fs file size.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c                   | 15 +++++++++++++++
 include/block/block.h     |  1 +
 include/block/block_int.h |  1 +
 3 files changed, 17 insertions(+)

diff --git a/block.c b/block.c
index ba22fc0dfb..6e1a435490 100644
--- a/block.c
+++ b/block.c
@@ -3407,6 +3407,21 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
 }
 
 /**
+ * Actually used by top-level format driver space (in it's .file) in bytes
+ */
+int64_t bdrv_get_format_allocated_size(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv) {
+        return -ENOMEDIUM;
+    }
+    if (drv->bdrv_get_format_allocated_size) {
+        return drv->bdrv_get_format_allocated_size(bs);
+    }
+    return -ENOTSUP;
+}
+
+/**
  * Return number of sectors on success, -errno on error.
  */
 int64_t bdrv_nb_sectors(BlockDriverState *bs)
diff --git a/include/block/block.h b/include/block/block.h
index 9b355e92d8..c36b9ccae4 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -304,6 +304,7 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp);
 int64_t bdrv_nb_sectors(BlockDriverState *bs);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
+int64_t bdrv_get_format_allocated_size(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
 void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
 int bdrv_commit(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8d3724cce6..e562fa7a5a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -208,6 +208,7 @@ struct BlockDriver {
     int64_t (*bdrv_getlength)(BlockDriverState *bs);
     bool has_variable_length;
     int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+    int64_t (*bdrv_get_format_allocated_size)(BlockDriverState *bs);
 
     int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
-- 
2.11.1

On 05/25/2017 10:26 AM, Vladimir Sementsov-Ogievskiy wrote:
> The function should return actually used by top-level format driver

s/actually/the allocation actually/

> space (in it's .file). It differs from bdrv_get_allocated_file_size,

s/it's/its/ (remember, "it's" is only appropriate if "it is" can be used
in its place)

> which returns allocated on fs file size.

s/allocated on fs file size/the disk usage of the underlying file/

So this is a measure of how many clusters a qcow2 image is currently
using (including metadata clusters, and regardless of whether those
clusters happen to point to a hole in the underlying protocol layer)?

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block.c                   | 15 +++++++++++++++
>  include/block/block.h     |  1 +
>  include/block/block_int.h |  1 +
>  3 files changed, 17 insertions(+)
> 
> diff --git a/block.c b/block.c
> index ba22fc0dfb..6e1a435490 100644
> --- a/block.c
> +++ b/block.c
> @@ -3407,6 +3407,21 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>  }
>  
>  /**
> + * Actually used by top-level format driver space (in it's .file) in bytes

s/it's/its/

I think we can do better.  How about:

Return the amount of space allocated by the format driver (including
metadata) in bytes, even if some of that space currently maps to holes
in the underlying protocol file.

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

25.05.2017 20:54, Eric Blake wrote:
> On 05/25/2017 10:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>> The function should return actually used by top-level format driver
> s/actually/the allocation actually/
>
>> space (in it's .file). It differs from bdrv_get_allocated_file_size,
> s/it's/its/ (remember, "it's" is only appropriate if "it is" can be used
> in its place)
>
>> which returns allocated on fs file size.
> s/allocated on fs file size/the disk usage of the underlying file/
>
> So this is a measure of how many clusters a qcow2 image is currently
> using (including metadata clusters, and regardless of whether those
> clusters happen to point to a hole in the underlying protocol layer)?

Hmm, interesting remark. Now it is realized exactly as you write.. But 
this leads to the situation, when qcow2 allocates data amount > 
bdrv_getlength(bs->file->bs) and we will have negative size of 
unallocated area =). Denis what do you think? (Actually, we needed this 
feature for non-sparce file system and this question was not considered).

>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block.c                   | 15 +++++++++++++++
>>   include/block/block.h     |  1 +
>>   include/block/block_int.h |  1 +
>>   3 files changed, 17 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index ba22fc0dfb..6e1a435490 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3407,6 +3407,21 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>>   }
>>   
>>   /**
>> + * Actually used by top-level format driver space (in it's .file) in bytes
> s/it's/its/
>
> I think we can do better.  How about:
>
> Return the amount of space allocated by the format driver (including
> metadata) in bytes, even if some of that space currently maps to holes
> in the underlying protocol file.
>

-- 
Best regards,
Vladimir

25.05.2017 21:07, Vladimir Sementsov-Ogievskiy wrote:
> 25.05.2017 20:54, Eric Blake wrote:
>> On 05/25/2017 10:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> The function should return actually used by top-level format driver
>> s/actually/the allocation actually/
>>
>>> space (in it's .file). It differs from bdrv_get_allocated_file_size,
>> s/it's/its/ (remember, "it's" is only appropriate if "it is" can be used
>> in its place)
>>
>>> which returns allocated on fs file size.
>> s/allocated on fs file size/the disk usage of the underlying file/
>>
>> So this is a measure of how many clusters a qcow2 image is currently
>> using (including metadata clusters, and regardless of whether those
>> clusters happen to point to a hole in the underlying protocol layer)?
>
> Hmm, interesting remark. Now it is realized exactly as you write.. But 
> this leads to the situation, when qcow2 allocates data amount > 
> bdrv_getlength(bs->file->bs) and we will have negative size of 
> unallocated area =). Denis what do you think? (Actually, we needed 
> this feature for non-sparce file system and this question was not 
> considered).

No, it will not be negative, all OK. I've mistaken. length of 
bs->file->bs will be positive anyway.

Actually, current approach is the following:

a. clusters allocated in qcow2 and after the end of bs->file: unallocated
b. clusters allocated in qcow2 and which are holes bs->file: allocated

(a) look inconsistent with b and with commit message and other things. 
But if I just account (a) clusters, it will lead to negative size of 
unallocated and will spoil the whole stat.. Something should be 
adjusted, at least at comment/commit-message level.


>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
>>> ---
>>>   block.c                   | 15 +++++++++++++++
>>>   include/block/block.h     |  1 +
>>>   include/block/block_int.h |  1 +
>>>   3 files changed, 17 insertions(+)
>>>
>>> diff --git a/block.c b/block.c
>>> index ba22fc0dfb..6e1a435490 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -3407,6 +3407,21 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>>>   }
>>>   
>>>   /**
>>> + * Actually used by top-level format driver space (in it's .file) in bytes
>> s/it's/its/
>>
>> I think we can do better.  How about:
>>
>> Return the amount of space allocated by the format driver (including
>> metadata) in bytes, even if some of that space currently maps to holes
>> in the underlying protocol file.
>>
>
> -- 
> Best regards,
> Vladimir


-- 
Best regards,
Vladimir

On 05/25/2017 01:19 PM, Vladimir Sementsov-Ogievskiy wrote:

> No, it will not be negative, all OK. I've mistaken. length of
> bs->file->bs will be positive anyway.
> 
> Actually, current approach is the following:
> 
> a. clusters allocated in qcow2 and after the end of bs->file: unallocated

How often do we actually end up in this situation? I know there are a
few cases where it makes sense (for example, if your image is large
enough to have L1 occupy two clusters, but the guest hasn't written
anything in the second half of the guest-visible area, then the second
cluster of the L1 table can be all zeroes - and depending on allocation
patterns, it is feasible that the L1 table can occupy the end of the
image, where the second cluster of the L1 table thus points beyond
end-of-file of the underlying image while still being a well-formed
qcow2 image).  It may also be a common transitory state in how we expand
a qcow2 image; if the expansion is interrupted in the middle, we could
easily have the refcount table saying a cluster has been allocated while
we have not yet actually written to the host file.

> b. clusters allocated in qcow2 and which are holes bs->file: allocated

This may be more common; a typical example is if we have a cluster
marked QCOW2_CLUSTER_NORMAL but which maps to a hole (and therefore
bdrv_get_block_status will report it as BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO).

> 
> (a) look inconsistent with b and with commit message and other things.
> But if I just account (a) clusters, it will lead to negative size of
> unallocated and will spoil the whole stat.. Something should be
> adjusted, at least at comment/commit-message level.

Exposing the stat as a difference may not make as much sense as
reporting actual numbers (this image requires an allocation of this many
bytes to fully represent current guest- and meta-data, even if some of
the allocation is currently pointing to holes or beyond the end of the
underlying file).

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