Hi, @pcd1193182 , @ahrens,
I have been working on this problem concurrent with your work presented in this
pull request. I wanted to summarize my comments and ask for your review of how
they were addressed in https://github.com/zfsonlinux/zfs/pull/4024.
One somewhat cosmetic issue is proper logical size setting for L0 holes
mentioned above. In dbuf_read_impl(), the logical size for L0 should indicate
data block size:
```
uint32_t bshift = (db->db_level > 1) ? dn->dn_indblkshift : dn->dn_datablkshift;
BP_SET_LSIZE(bp, (1ULL << bshift));
```
I agree with @pcd1193182 that this issue does not lead to problems since the
size is not really used anywhere in the code.
Another issue is that as I found in my testing, pre-filling Ln holes in
dbuf_read_impl() can result in metadata block ids pointing beyond the
dn_maxblkid. This is similar to what happens with data block ids, and it is
addressed by adjusting dn_maxblkid in dbuf_write_ready().
And also another issue of accounting for holes with non-zero epochs in
dnode_increase_indirection() also mentioned above; the code segment
```
/* check for existing blkptrs in the dnode */
for (i = 0; i < nblkptr; i++)
if (!BP_IS_HOLE(&dn->dn_phys->dn_blkptr[i]))
break;
```
could be adjusted as follows:
```
/* check for existing blkptrs in the dnode */
for (i = 0; i < nblkptr; i++)
if (!BP_IS_HOLE(&dn->dn_phys->dn_blkptr[i]) ||
dn->db_phys->bn_blkptr[i].blk_birth != 0)
break;
```
So, I was wondering if you could review the pull request I mentioned above, and
then perhaps I could port the changes to openzfs as well ?
Best regards.
---
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/46#issuecomment-176463832_______________________________________________
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer
