Re: [Qemu-block] [PATCH 1/7] block: fix comment for bdrv_get_allocated_file_size()

[Eric Blake]

On 05/25/2017 10:26 AM, Vladimir Sementsov-Ogievskiy wrote:
> Current comment is not clear enough: which sparseness is meant, coming
> from sparse image format or from sparse file system?
> 
> For example, if we have qcow2 above raw file on non-sparse file system,
> this function will say nothing about unallocated (by qcow2 layer)
> clusters.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 50ba264143..ba22fc0dfb 100644
> --- a/block.c
> +++ b/block.c
> @@ -3388,8 +3388,8 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> Error **errp)
>  }
>  
>  /**
> - * Length of a allocated file in bytes. Sparse files are counted by actual

Yay for getting rid of bad grammar (s/a /an /)

> - * allocated space. Return < 0 if error or unknown.
> + * Size of allocated in underlying file system area. Sparseness is taken into

Doesn't read well.  Maybe: s/Size of allocated/Allocation size/ ?

> + * account for sparse file systems. Return < 0 if error or unknown.

I still don't get what we are trying to present.

If we have the following 6 qcow2 file clusters backed by the underlying
lseek(SEEK_DATA/HOLE) file system contents:

BDRV_BLOCK_UNALLOCATED   N/A
BDRV_BLOCK_ZERO_PLAIN    N/A
BDRV_BLOCK_ZERO_ALLOC    hole
BDRV_BLOCK_ZERO_ALLOC    data
BDRV_BLOCK_DATA          hole
BDRV_BLOCK_DATA          data

Then is our answer the size of all qcow2 allocations regardless of
underlying status (4, due to clusters 3-6), or the size of only the
clusters that read from the backing file (2, due to clusters 5-6), or
the size of only the clusters that currently occupy space in the file
system (2, due to clusters 4 and 6), or the size of clusters that are
not provably read-as-zero (1, due to cluster 6)?

Does the answer change if you can have underlying holes happen at a
smaller granularity than clusters?

What happens for compressed clusters?

I think we still need to do a better job at writing a precise comment.

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

signature.asc
Description: OpenPGP digital signature