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



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1200,13 +1200,24 @@ public FileStatus getFileStatus(Path f) throws 
IOException {
         INode<FileSystem> inode = iEntry.getValue();
         if (inode.isLink()) {
           INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
-
-          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));
+          try {
+            FileStatus status = link.getTargetFileSystem()
+                .getFileStatus(new Path("/"));

Review comment:
       Why are we getting status on "/"? In practical scenario it should work. 
However what if the target uri is a file?
   Should we simply use link.getTargetFileSystem().getUri() ?
   Could you please check scenario? If this works, I have no other changes.
   Thanks for update.

##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1211,13 +1211,29 @@ public FileStatus getFileStatus(Path f) throws 
IOException {
         INode<FileSystem> inode = iEntry.getValue();
         if (inode.isLink()) {
           INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
-
-          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));
+          // For MERGE or NFLY links, the first target link is considered
+          // for fetching the FileStatus with an assumption that the permission
+          // and the owner will be the same for all the target directories.
+          Path linkedPath = new Path(link.targetDirLinkList[0].toString());
+          ChRootedFileSystem linkedFs = (ChRootedFileSystem)
+              link.getTargetFileSystem();
+          try {
+            FileStatus status = linkedFs.getMyFs().getFileStatus(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));
+          } 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));

Review comment:
       IMO, FileNotFoundException is fine. Please see my comment below 
https://github.com/apache/hadoop/pull/2019/files#r430611396
   @chliang71 , What do you say?




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to