open() and ESTALE error

[Terry Lambert]

Don Lewis wrote:
> > +again:
> >       error = vn_open(&nd, flags, cmode);
> >       if (error) {
> > +             /*
> > +              * if the underlying filesystem returns ESTALE
> > +              * we must have used a cached file handle.
> > +              */
> > +             if (error == ESTALE && stale++ == 0)
> > +                     goto again;
> >               /*
> >                * release our own reference
> >                */
[ ... ]
> If the name of the file are you attempting to open is relative to your
> current working directory, and your current working directory is nuked
> on the server, vn_open will return ESTALE, and your patch above will
> loop forever.

No, actually he thought of that (I read the code this way
the first time too, but was lucky enough to read it through
a couple of times while looking at the NFS sources in another
window, so I caught myself before sending anything).

Specifically, see the underline part of:

> > +             if (error == ESTALE && stale++ == 0)
                                      ---------------

...he exits it after retrying it fails, and falls into the
standard ESTALE return case.

If this gets committed (which I think it shouldn't because I
can see a genuinely bad handle getting converted to a good one
in a couple of cases), that line should probably be rewritten
to be more obvious (e.g. move the "stale++" before the "if"
statment and adjust the compare to compensate for the difference
so no one else reads it the way we did).

Your other comments are good, though (see other post).

-- Terry