umamaheswararao commented on a change in pull request #2084:
URL: https://github.com/apache/hadoop/pull/2084#discussion_r443060391



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1216,37 +1222,50 @@ public FileStatus getFileStatus(Path f) throws 
IOException {
       for (Entry<String, INode<FileSystem>> iEntry :
           theInternalDir.getChildren().entrySet()) {
         INode<FileSystem> inode = iEntry.getValue();
+        Path path = new Path(inode.fullPath).makeQualified(myUri, null);
         if (inode.isLink()) {
           INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
+
+          if (showMountLinksAsSymlinks) {
+            // To maintain backward compatibility, with default option(showing
+            // mount links as symlinks), we will represent target link as
+            // symlink and rest other properties are belongs to mount link 
only.
+            result[i++] =
+                new FileStatus(0, false, 0, 0, creationTime, creationTime,
+                    PERMISSION_555, ugi.getShortUserName(),
+                    ugi.getPrimaryGroupName(), link.getTargetLink(),
+                    path);
+            continue;
+          }
+
+          //  We will represent as non-symlinks. Here it will show target
+          //  directory/file properties like permissions, isDirectory etc on
+          //  mount path. The path will be a mount link path and isDirectory is
+          //  true if target is dir, otherwise false.
+          String linkedPath = link.getTargetFileSystem().getUri().getPath();
+          if ("".equals(linkedPath)) {
+            linkedPath = "/";
+          }
           try {
-            String linkedPath = link.getTargetFileSystem().getUri().getPath();
-            if("".equals(linkedPath)) {
-              linkedPath = "/";
-            }
             FileStatus status =
                 ((ChRootedFileSystem)link.getTargetFileSystem())
                 .getMyFs().getFileStatus(new Path(linkedPath));
-            result[i++] = new FileStatus(status.getLen(), false,
-              status.getReplication(), status.getBlockSize(),
-              status.getModificationTime(), status.getAccessTime(),
-              status.getPermission(), status.getOwner(), status.getGroup(),
-              link.getTargetLink(),
-              new Path(inode.fullPath).makeQualified(
-                  myUri, null));
+            result[i++] = new FileStatus(status.getLen(), status.isDirectory(),
+                status.getReplication(), status.getBlockSize(),
+                status.getModificationTime(), status.getAccessTime(),
+                status.getPermission(), status.getOwner(), status.getGroup(),
+                null, path);
           } catch (FileNotFoundException ex) {
-            result[i++] = new FileStatus(0, false, 0, 0,
-              creationTime, creationTime, PERMISSION_555,
-              ugi.getShortUserName(), ugi.getPrimaryGroupName(),
-              link.getTargetLink(),
-              new Path(inode.fullPath).makeQualified(
-                  myUri, null));
+            LOG.warn("Cannot get one of the children's(" + path
+                + ")  target path(" + link.getTargetFileSystem().getUri()
+                + ") file status.", ex);
+            throw ex;

Review comment:
       The problem here is, rounter has state, but in viewFS is simply light, 
state can be build from config only, so no real permissions notion. All mounts 
will have same perms bit. Just showing that really may not help. Most of the fs 
what we mount are high available. SO, don't  expect this case should trigger ( 
even I see this point in you comment "this won't happen general"). Even if it 
triggered, application will anyway starts seeing failures on further ops on 
that directory. And one more point I am having is, restricting first and 
relaxing later is safer than relaxing and later restricting is tough. So, I 
feel Let's throw exception. If this really turning out problems, then we can 
always relax (that should be safe as we are having general IOException). When 
coming to symlink notion, you always have an identity other than file/dir, that 
is link. So, I have no issues. But non-symlinks case, I am not really show some 
temp values there. With that values anyway app is going to fail if he crawls 
one more level.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to