[Gluster-devel] doubts in posix_handle_path and posix_handle_pump

[Xavier Hernandez]

On Tuesday 03 June 2014 15:42:19 Pranith Kumar Karampuri wrote:
> On 06/03/2014 02:42 PM, Xavier Hernandez wrote:
> > The possible problem I see is that in the comments it says that this
> > function returns a path to an IA_IFDIR (it will return IA_IFDIR on an
> > lstat), however if one of the symlinks is missing or anything else fails,
> > it won't return an error *but* it will return a path to an existing file.
> > An lstat on this path will return IA_IFLNK instead of IA_IFDIR. I don't
> > know if this can be a problem in some places.
> 
> This is exactly what I was referring to, I don't see an easy way to find
> out if there is any failure in the function. One needs to do extra lstat
> or a 'path' based syscall like getxattr etc on the returned path to
> check if it returned a good path. So do you think the best thing is to
> ignore the return value of the function call but depend on an lstat or a
> path based syscall of the path?
> 
The only point to consider is for gfid's representing directories. Other types 
of file do not have any problem (the returned path can be considered valid 
even if lstat() fails). For directories there are 3 places where things can 
fail:

At line 360: I think this is not a problem. If lstat() fails (basically 
because it does not exist), the returned path can be considered valid.

At line 367: If posix_handle_pump() fails, it could mean:

* The symlink is not a valid directory symlink:
  * it's a corrupted one: any operation on this file should be denied
  * it's a normal symlink that has lost one of the hard-links: though it's bad 
to have damaged gfid's, the returned path can be considered valid.

* readlink() failed: this woulb be very weird. Access to the file should be 
denied.

At line 374: If lstat() fails, probably it means that the symlink of one of 
the parents of the directory is missing. The returned path won't fail on 
lstat(), but it should because it will return symlink information instead of 
directory information.

I think that it's very hard to determine if something went wrong only by 
inspecting the returned path. I think the best approach would be that 
posix_handle_path() return -1 if posix_handle_pump() or lstat() at line 374 
fail, and each caller decide what to do in case of failure.

However I don't know all the details of the posix xlator, so maybe I'm wrong 
and this is not necessary. Let's see if there is someone else with more 
experience on it to see what he thinks.

Xavi