[Gluster-devel] Report ESTALE as ENOENT

[Michael Kerrisk (man-pages)]

On 10/11/2017 04:02 PM, J. Bruce Fields wrote:
> On Wed, Oct 11, 2017 at 04:11:51PM +0530, Raghavendra G wrote:
>> On Thu, Mar 31, 2016 at 1:22 AM, J. Bruce Fields <bfields at fieldses.org>
>> wrote:
>>
>>> On Mon, Mar 28, 2016 at 04:21:00PM -0400, Vijay Bellur wrote:
>>>> I would prefer to:
>>>>
>>>> 1. Return ENOENT for all system calls that operate on a path.
>>>>
>>>> 2. ESTALE might be ok for file descriptor based operations.
>>>
>>> Note that operations which operate on paths can fail with ESTALE when
>>> they attempt to look up a component within a directory that no longer
>>> exists.
>>>
>>
>> But, "man 2 rmdir"  or "man 2 unlink" doesn't list ESTALE as a valid error.
> 
> In fact, almost no man pages list ESTALE as a valid error:
> 
> 	[bfields at patate man-pages]$ git grep ESTALE
> 	Changes.old:        Change description for ESTALE
> 	man2/open_by_handle_at.2:.B ESTALE
> 	man2/open_by_handle_at.2:.B ESTALE
> 	man3/errno.3:.B ESTALE
> 
> Cc'ing Michael Kerrisk for advice.  Is there some reason for that, or
> can we fix those man pages?

Sure. I'd need help or a patch, to know which pages need fixing though.

Cheers,

Michael

>> Also rm doesn't seem to handle ESTALE too [3]
>>
>> [4] https://github.com/coreutils/coreutils/blob/master/src/remove.c#L305
> 
> I *think* that code is just deciding whether a given error should be
> silently ignored in the rm -f case.  I don't think -ESTALE (indicating
> the directory is bad) is such an error, so I think this code is correct.
> But my understanding may be wrong.
> 
>>> Maybe non-creating open("./foo") returning ENOENT would be reasonable in
>>> this case since that's what you'd get in the local filesystem case, but
>>> creat("./foo") returning ENOENT, for example, isn't something
>>> applications will be written to handle.
>>>
>>> The Linux VFS will retry ESTALE on path-based systemcalls *one* time, to
>>> reduce the chance of ESTALE in those cases.
>>
>>
>> I should've anticipated bug [2] due to this comment. My mistake. Bug [2] is
>> indeed due to kernel not retrying open on receiving an ENOENT error.
>> Glusterfs sent ENOENT because file's inode-number/nodeid changed but same
>> path exists. The correct error would've been ESTALE, but due to our
>> conversion of ESTALE to ENOENT, the latter was sent back to kernel.
>>
>> Looking through kernel VFS code, only open *seems* to retry
>> (do_filep_open). I couldn't find similar logic to other path based syscalls
>> like rmdir, unlink, stat, chmod etc
> 
> I believe there is a retry in those cases, but I'm not sure exactly
> where it is.  Looking around.... See the retry_estale() checks sprinkled
> around namei.c, which were added by Jeff Layton a few years ago.
> 
> --b.
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/