On 06/03/2014 02:42 PM, Xavier Hernandez wrote:
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.
Wonderful explanation! :-). Thanks!. Now I understand why there is that
while loop. I will add this part of the mail as a comment for that
function when I re-submit the code.
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?
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@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
_______________________________________________
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
