[Gluster-devel] doubts in posix_handle_path and posix_handle_pump

[Xavier Hernandez]

Hi Pranith,

I'm not Avati but I think I can explain the purpose of these functions.

As I've interpreted it, posix_handle_path() should return the path to a "real" 
entry of the filesystem. It can be called for all kind of files, but it seems 
that basename is != NULL only for entries where gfid refers to a directory. 
That seems good.

If the specified gfid refers to anything other than a directory, since they 
are stored inside .glusterfs as hard links, the returned path should simply be 
<brick_root>/.glusterfs/xx/yy/<gfid>.

For gfid's that specify directories, since they are stored inside .glusterfs 
as symlinks, they need to be dereferenced before returning (to point to a real 
directory). That is the purpose of line 362: only if the lstat succeeded (it 
basically means that the entry exists), the path refers to a symbolic link, 
and it has more than 1 hard link (to differentiate a symlink of a directory, 
with 1 hard link, from a "normal" symlink, with at least 2 hard links) then it 
tries to replace the symlink by its referenced real directory.

This is done using the function posix_handle_pump(). This function reads the 
symlink and replaces it into the path:

  If the original path were: <root>/cd/47/cd473c26-b6d6-42a6-a0a4-28e04de13915
  And cd473c26-b6d6-42a6-a0a4-28e04de13915 points to:
      ../../0e/5d/0e5d04a1-f418-4388-90db-d1d7ebb8a071/dir1
  The result is: <root>/0e/5d/0e5d04a1-f418-4388-90db-d1d7ebb8a071/dir1

The returned path points to a directory, so it should be ok. However since 
this path can be composed by other symlinks (in this case 0e5d04a1-
f418-4388-90db-d1d7ebb8a071 will be another symlink), the system needs to 
resolve them every time it accesses the entry. If there are too many of them 
(for example an entry very deep in the directory hierarchy) the system calls 
will return ELOOP (even if there isn't any symlink loop, the system has a 
limit in the number of symlinks it will resolve for any request). This is what 
is tested in the lstat() call at line 374 and the following test at 375: while 
ELOOP is returned by lstat, posix_handle_pump() is called to remove another 
level of symlink indirection. This guarantees that the returned path points to 
a directory *and* it won't fail on system calls with ELOOP.

It's very easy to check this:

  # cd <mount point>
  # mkdir -p d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d
  # cd <brick root>
  # getfattr -m. -e hex -d d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d
  # file: d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d
  trusted.gfid=0xf6e98d542c4f48259976861b7269a396
  trusted.glusterfs.dht=0x000000010000000000000000ffffffff
  # ls .glusterfs/f6/e9/f6e98d54-2c4f-4825-9976-861b7269a396
  ls: cannot access .glusterfs/f6/e9/f6e98d54-2c4f-4825-9976-861b7269a396: Too 
many levels of symbolic links

The loop on posix_handle_pump() avoids this problem.

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.

Xavi

On Monday 02 June 2014 02:00:30 Pranith Kumar Karampuri wrote:
> hi Avati,
>    Could you please explain why posix_handle_pump fixes ELOOP.
> 
> posix_handle_path seems to return gfid-path when the lstat on the base-gfid
> path fails. I am not sure what the behaviour is supposed to be.
> 
> I added the snippet of code for reference below.
> 
> 346         base_len = (priv->base_path_length + SLEN(GF_HIDDEN_PATH) + 45);
> 347         base_str = alloca (base_len + 1);
> 348         base_len = snprintf (base_str, base_len + 1,
> "%s/%s/%02x/%02x/%s", 349                              priv->base_path,
> GF_HIDDEN_PATH, gfid[0], gfid[1], 350                             
> uuid_str);
> 351
> 352         pfx_len = priv->base_path_length + 1 + SLEN(GF_HIDDEN_PATH) + 1;
> 353
> 354         if (basename) {
> 355                 len = snprintf (buf, maxlen, "%s/%s", base_str,
> basename); 356         } else {
> 357                 len = snprintf (buf, maxlen, "%s", base_str);
> 358         }
> 359
> 360         ret = lstat (base_str, &stat);
> 361
> 362         if (!(ret == 0 && S_ISLNK(stat.st_mode) && stat.st_nlink == 1)) 
>         <<<----- 363                 goto out;
> 
> Pranith.
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel