hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-5658) Implement ACL as a INode feature
Date Thu, 19 Dec 2013 00:08:08 GMT

    [ https://issues.apache.org/jira/browse/HDFS-5658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852358#comment-13852358
] 

Chris Nauroth commented on HDFS-5658:
-------------------------------------

Hi, Haohui.  This patch looks good.  A couple of comments:

# In {{FSDirectory#getAclStatus}}, can we take care of sticky bit in this patch by calling
{{node.getFsPermission().getStickyBit()}}?
# Minor: In {{resolveINodeWithAdditionalField}}, the throw statement is indented 4 spaces
instead of 2.

I think that's all that needs to be addressed before commit to the feature branch.

Here are a couple of other comments that I don't think need to be addressed in this patch.
 These are just some things for us to keep in mind.  I think they're going to be easier to
address in other patches.

# We'll want to add more tests in {{TestNameNodeAcl}}.  I think this will make more sense
in a separate patch to finish out the wiring of all methods through {{NameNodeRpcServer}}
and {{FSNamesystem}}.
# Right now, {{getAclStatus}} returns an empty entry list for inodes that don't have an ACL.
 We'll need to change this so that it returns a minimal ACL: exactly 3 ACL entries for owner,
group and other, inferred from the {{FsPermission}} bits.  I think this will make more sense
in a separate patch to finish out the interactions with classic permission bits and chmod.
# Regarding the {{FileNotFoundException}} thrown from {{resolveINodeWithAdditionalField}},
I think the only kind of {{INode}} that won't be an {{INodeWithAdditionalFields}} is an {{INodeReference}}
(used inside a snapshot).  At this point, the code should never see one of those, because
{{INodeDirectory#getINodesInPath4Write}} throws a {{SnapshotAccessControlException}} for attempts
to modify a snapshot path.  I think the only way we'd hit this code path is if there was a
bug, so {{FileNotFoundException}} might not be the right choice.  I think this decision is
easier to finalize as part of HDFS-5614, which tracks interaction of ACLs with snapshots.


> Implement ACL as a INode feature
> --------------------------------
>
>                 Key: HDFS-5658
>                 URL: https://issues.apache.org/jira/browse/HDFS-5658
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: hdfs-client, namenode, security
>            Reporter: Haohui Mai
>            Assignee: Haohui Mai
>         Attachments: HDFS-5658.000.patch, HDFS-5658.001.patch
>
>
> HDFS-5284 introduces features as generic abstractions to extend the functionality of
the inodes. The implementation of ACL should leverage the new abstractions.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)

Mime
View raw message