On Wed, Mar 25, 2015 at 6:07 PM, Boris <[email protected]> wrote:

> LGTM for code
>
> Nit pick: dmu.c:1828
> /* Given the ZFS object...
>

Thanks, fixed.


>
> But an api design question: why return separately the block size and
> number of holes instead of the actual number of bytes which seems a better
> match for file abstraction ?
>

I could see arguments either way.  Certainly you could get the same info by
returning the "hole bytes" and blocksize, rather than "hole blocks" and
blocksize.

But the hole concept doesn't really match the "file is a stream of bytes"
concept, because it's inherently operating on chunks of bytes (blocks).
For example, if you write 10 bytes at offset 20, it does not create a hole
from bytes 0-20.  But if the blocksize is 32K and you write to offset 64K,
it will create a hole from bytes 0-64K.

--matt


> Typos courtesy of my iPhone
>
> On Mar 25, 2015, at 11:00 AM, Matthew Ahrens <[email protected]> wrote:
>
>   This is an automatically generated e-mail. To reply, visit:
> https://reviews.csiden.org/r/170/
>   Review request for OpenZFS Developer Mailing List and Christopher Siden.
> By Matthew Ahrens.
>  *Bugs: * 5692
> <https://www.illumos.org/projects/illumos-gate//issues/5692>
>  *Repository: * illumos-gate
> Description
>
> 5692 expose the number of hole blocks in a file
> Reviewed by: Adam Leventhal <[email protected]\>
> Reviewed by: Matthew Ahrens <[email protected]\>
>
> Original author: Max Grossman
>
>   Testing
>
> zfs test suite
> ztest
> wrote a program to query this, tested with files w/various number and 
> position of holes in them.
>
> http://jenkins/job/zfs-precommit/1911/
>
>   Diffs
>
>    - usr/src/uts/common/sys/filio.h
>    (cbd98af97eb1a0b1abdb6fab7937cabda15c8765)
>    - usr/src/uts/common/fs/zfs/zfs_vnops.c
>    (5060412afd9b3e3bd2572fa71397426d12bc3cb3)
>    - usr/src/uts/common/fs/zfs/sys/dmu.h
>    (6e49ae3d09260c33339e17c868976f5f94b1f50e)
>    - usr/src/uts/common/fs/zfs/dmu.c
>    (c541e04d54297243c9d6a342b7a8df742e8d7ed2)
>    - usr/src/lib/libzfs/common/mapfile-vers
>    (65f266996e168c55d600e26895a12e0ab4cad460)
>    - usr/src/lib/libzfs/common/libzfs_util.c
>    (414d74ee20660a067cf224b91677c09658a05854)
>    - usr/src/lib/libzfs/common/libzfs.h
>    (9aa5ba59056455d7ecc510189546a1349d65eaa5)
>
> View Diff <https://reviews.csiden.org/r/170/diff/>
>
> _______________________________________________
> developer mailing list
> [email protected]
> http://lists.open-zfs.org/mailman/listinfo/developer
>
>
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer

Reply via email to