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

[Vladimir Sementsov-Ogievskiy]

25.05.2017 20:46, Eric Blake wrote:
> On 05/25/2017 12:03 PM, Vladimir Sementsov-Ogievskiy wrote:
> > 25.05.2017 19:32, Eric Blake wrote:
> > > 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.
> > > >
> > > > + * 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.
> > >
> > > I think we still need to do a better job at writing a precise comment.
> > bdrv_get_allocated_file_size is not related to qcow2, as qcow2 doesn't
> > support it. So, it is finally just raw_get_allocated_file_size, which
> > returns st.st_blocks * 512 after fstat(s->fd).
> >
> > It will correspond to qcow2 sparseness if qcow2 discarded corresponding
> > clusters in bs->file and if underlying fs is sparse.
> Okay, that helps.  How about this wording:
>
> Allocation size of the underlying file system area.  Sparseness is taken
> into account (holes do not contribute to this size).  Return < 0 if
> error or unknown.
OK for me, thank you. Will use your wording if no other comments on this.



--
Best regards,
Vladimir