Re: [Qemu-block] [PATCH 2/7] block: add bdrv_get_format_allocated_size format interface

[Vladimir Sementsov-Ogievskiy]

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 <address@hidden>
---
 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