umamaheswararao commented on a change in pull request #2061:
URL: https://github.com/apache/hadoop/pull/2061#discussion_r436490143
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1202,19 +1202,18 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
try {
String linkedPath = link.getTargetFileSystem().getUri().getPath();
- if("".equals(linkedPath)) {
+ 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));
+ ((ChRootedFileSystem) link.getTargetFileSystem()).getMyFs()
+ .getFileStatus(new Path(linkedPath));
+ result[i++] = new FileStatus(status.getLen(), status.isDirectory(),
+ status.getReplication(), status.getBlockSize(),
+ status.getModificationTime(), status.getAccessTime(),
+ status.getPermission(), status.getOwner(), status.getGroup(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(myUri, null));
Review comment:
@ayushtkn , Thanks for the review!
Here ViewFS mount points seems to be designed to show them as symlinks. This
can be figured out by looking at
ViewFsBaseTest.java#testListOnInternalDirsOfMountTable (which was way old test)
and also I had discussion with Sanjay on that.
So, I don't think it's reasonable to change that assumption/behavior now.
>Any idea where the link.getTargetLink() in the FileStatus can be or is used?
FileStatus has getSymlink API. Also Filestatus path represents link path.
and symlink path represents target path. This seems to be correct.
Coming to permissions bit I just realized symlink showing its own
permissions instead of target in macos also. ( but we just changed the behavior
in HADOOP-17029 where we display target dir's permissions and group. cc:
@abhishekdas99, as per linux/macos the older behavior seems to be ok, but
ViewFS perspective it seems to be good to show target permissions instead of
showing some bogus permissions. I think in ViewFS case the in-memory
InternalViewFSDirFS was created by fs but we can not really cary who created
what mount, we will just configure in xml that link and would not have any info
who added that, all should be at admins permission control only ). I assume in
linux mount link dir will be created with the permission to the current user
who created right? Since we don't have that link creation logic dynamically at
ViewFS, I don't think we can behave exactly same as Linux here?
My another original question was why we should not have symlink on dir?
is it because HDFS does not support that? If that because of hdfs, should we
move that assert under HdfsNamedFileStatus? I don't know what else will break
here
----------------------------------------------------------------
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]
