[adding Carlos who was looking into this if I remember correctly] On Fri, Aug 31, 2018 at 08:36:40AM -0400, Brian Foster wrote: > So basically ioctl_fibmap() either prioritizes ->fiemap() or looks for > some special combination of (fiemap && !bmap) to translate the call.. I'd always use fiemap if present, but if there is any good reason to prefer ->bmap if present that should be ok. Hopefully we can just kill of pure ->bmap implementations in the long run.. > > > Granted, grub's blocklist code doesn't seem to check for shared blocks > > > when it writes grubenv.... yuck, though TBH I don't have the eye budget > > > to spend on digging through grub2. Frankly I think FIBMAP comes verrry > > > close to "this API is unfixably stupid and shouldn't be enabled for new > > > use cases and should go away some day". > > > > .. and that policy should be: always return an error for the slightest > > unusual file layout (shared, encrypted, inline, etc). > > ... and then return some error if the associate extent is in some state > that cannot be described by fibmap..? That sounds like a nice option to > me. Carlos..? Yes. > Maybe it's too late for this, but I think even dropping ->bmap > completely for the time being on XFS reflink=1 filesystems is preferable > to the current behavior where we return a perfectly valid result and > pretend that somehow represents an error to userspace. Note that a 0 return on ->bmap has traditionally been treated as an error, including in userspace as block 0 is usually not a valid block for user data. (this isn't intended as support for this interface, just stating history)