Re: [PATCH, RFC] xfs: re-enable FIBMAP on reflink; disable for swap

[Eric Sandeen <sandeen@sandeen.net>]

On 8/30/18 2:39 PM, Brian Foster wrote:
>>> 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".

Backing up, the interface isn't /that/ dumb, other than being limited to
32 bits.  Q: "Where is this logical file block physically located?"  A: "It is here."

Inefficient, sure.

>> So instead if anyone asks we'll just give them a successful response which
>> is indistinguishable from a hole.  :(
>>
> ... but this seems to be the crux of the matter (IMO, at least). If we
> can return -ENOTSUPP or whatever, then it can be made obvious that the
> user either needs to use fiemap or avoid using reflinked files. ISTM
> that what we do now is essentially report an incorrect bmap, which leads
> to these subtle bug reports.
> 
> I haven't dug into the fibmap code.. does something prevent returning a
> legitimate error code?

so ->bmap() returns a sector_t, which is a u64 and doesn't really
leave room for a negative error return.  But the ioctl interface 
stuffs that return into an int, and returns it to the user.
(note to self, wonder why it doesn't return -EOVERFLOW as needed).

I suppose ->bmap() could be changed to take a u64 in/out pointer like the
user interface does, and return 0 or -errno.  *Shrug* this seems like
a lot of work to avoid simply giving the user what they asked
for.  ;)

-Eric