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

Charles Lamb commented on HDFS-6258:
------------------------------------

Hi Yi,

Here are a few minor things that I picked up on as well as some javadoc fixups.

Charles

Index: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java
===================================================================
--- 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java   
    (revision 0)
+++ 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java   
    (working copy)
@@ -0,0 +1,138 @@
+/**
+ * XAttr is POSIX Extended Attribute model, similar to the one in traditional 
Operating Systems.
+ * Extended Attribute consists of a name and associated data, and 4 namespaces 
are defined: user, 
+ * trusted, security and system.
+ *   1). USER namespace extended attribute may be assigned for storing 
arbitrary additional 
+ *   information, and its access permissions are defined by file/directory 
permission bits.
+ *   2). TRUSTED namespace extended attribute are visible and accessible only 
to privilege user 
+ *   (file/directory owner or fs admin), and it is available from both user 
space (filesystem 
+ *   API) and fs kernel.
+ *   3). SYSTEM namespace extended attribute is used by fs kernel to store 
system objects, 
+ *   and only available in fs kernel. It's not visible to users.
+ *   4). SECURITY namespace extended attribute is used by fs kernel for 
security features, and 
+ *   it's not visible to users.

XAttr is the POSIX Extended Attribute model similar to that found in
traditional Operating Systems.  Extended Attributes consist of one
or more name/value pairs associated with a file or directory. Four
namespaces are defined: user, trusted, security and system.
  1) USER namespace attributes may be used by any user to store
  arbitrary information. Access permissions in this namespace are
  defined by a file directory's permission bits.
  2) TRUSTED namespace attributes are only visible and accessible to
  privileged users (a file or directory's owner or the fs
  admin). This namespace is available from both user space
  (filesystem API) and fs kernel.
  3) SYSTEM namespace attributes are used by the fs kernel to store
  system objects.  This namespace is only available in the fs
  kernel. It is not visible to users.
  4) SECURITY namespace attributes are used by the fs kernel for
  security features. It is not visible to users.

Index: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
===================================================================
--- 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
 (revision 1589028)
+++ 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
 (working copy)
@@ -109,6 +109,8 @@
 import org.apache.hadoop.fs.MD5MD5CRC32FileChecksum;
 import org.apache.hadoop.fs.MD5MD5CRC32GzipFileChecksum;
 import org.apache.hadoop.fs.Options;
+import org.apache.hadoop.fs.XAttr;
+import org.apache.hadoop.fs.XAttrSetFlag;
 import org.apache.hadoop.fs.Options.ChecksumOpt;
 import org.apache.hadoop.fs.ParentNotDirectoryException;
 import org.apache.hadoop.fs.Path;
@@ -191,6 +193,8 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.common.net.InetAddresses;
 
 /********************************************************
@@ -2757,6 +2761,127 @@
                                      UnresolvedPathException.class);
     }
   }
+  
+  XAttr constructXAttr(String name, byte[] value) {
+    if (name == null) {
+      throw new NullPointerException("XAttr name can not be null.");
+    }
+    
+    int prefixIndex = name.indexOf(".");
+    if (prefixIndex == -1) {
+      throw new IllegalArgumentException("XAttr name must be prefixed with 
user/trusted/security/system which followed by '.'");

s/which/and/

+    }
+    
+    XAttr.NameSpace ns;
+    String prefix = name.substring(0, prefixIndex).toUpperCase();
+    if (prefix.equals(XAttr.NameSpace.USER.toString())) {
+      ns = XAttr.NameSpace.USER;
+    } else if (prefix.equals(XAttr.NameSpace.TRUSTED.toString())) {
+      ns = XAttr.NameSpace.TRUSTED;
+    } else if (prefix.equals(XAttr.NameSpace.SECURITY.toString())) {
+      ns = XAttr.NameSpace.SECURITY;
+    } else if (prefix.equals(XAttr.NameSpace.SYSTEM.toString())) {
+      ns = XAttr.NameSpace.SYSTEM;
+    } else {
+      throw new IllegalArgumentException("XAttr name must be prefixed with 
user/trusted/security/system which followed by '.'");

s/which/and/

I'm unclear as to whether namespaces are case-sensitive or insensitive
(I believe they are case-insensitive). The check above is
case-sensitive, but constructXAttrMap below uses toLowerCase(). All of
the javadoc should clarify this.

+    }
+    XAttr xAttr = (new 
XAttr.Builder()).setNameSpace(ns).setName(name.substring(prefixIndex + 
1)).setValue(value).build();
+    
+    return xAttr;
+  }
+  
+  public void setXAttr(String src, String name, byte[] value, 
EnumSet<XAttrSetFlag> flag) throws IOException {
+    checkOpen();
+    try {
+      namenode.setXAttr(src, constructXAttr(name, value), flag);
+    } catch (RemoteException re) {
+      throw re.unwrapRemoteException(AccessControlException.class,
+                                     AclException.class,
+                                     FileNotFoundException.class,
+                                     NSQuotaExceededException.class,
+                                     SafeModeException.class,
+                                     SnapshotAccessControlException.class,
+                                     UnresolvedPathException.class);
+    }
+  }
+  
+  public byte[] getXAttr(String src, String name) throws IOException {
+    checkOpen();
+    try {
+      XAttr xAttr = namenode.getXAttr(src, constructXAttr(name, null));
+      return xAttr != null ? xAttr.getValue() : null;
+    } catch(RemoteException re) {
+      throw re.unwrapRemoteException(AccessControlException.class,
+                                     AclException.class,
+                                     FileNotFoundException.class,
+                                     UnresolvedPathException.class);
+    }
+  }
+  
+  Map<String, byte[]> constructXAttrMap(List<XAttr> xAttrs) {
+    if (xAttrs == null) {
+      return null;
+    }
+    Map<String, byte[]> xAttrMap = Maps.newHashMap();
+    for (XAttr xAttr : xAttrs) {
+      String namespace = xAttr.getNameSpace().toString();
+      String name = namespace.toLowerCase() + "." + xAttr.getName();
+      xAttrMap.put(name, xAttr.getValue());
+    }

Are namespaces case-sensitve or insensitive?

Index: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
===================================================================
--- 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
 (revision 1589028)
+++ 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
 (working copy)
@@ -37,6 +37,7 @@
@@ -1050,6 +1058,13 @@
     op.aclEntries = entries;
     logEdit(op);
   }
+  
+  void logSetXAttrs(String src, List<XAttr> xAttrs) {
+    SetXAttrsOp op = SetXAttrsOp.getInstance();
+    op.src =src;

s/=src/= src/

Index: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
===================================================================
--- 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
       (revision 1589028)
+++ 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
       (working copy)
@@ -54,6 +54,7 @@
@@ -477,6 +551,7 @@
 
       if (this.opCode == OP_ADD) {
         AclEditLogUtil.write(aclEntries, out);
+        XAttrsEditLogUtil.write(xAttrs, out);
         FSImageSerialization.writeString(clientName,out);
         FSImageSerialization.writeString(clientMachine,out);
         // write clientId and callId
@@ -542,6 +617,7 @@
       // clientname, clientMachine and block locations of last block.
       if (this.opCode == OP_ADD) {
         aclEntries = AclEditLogUtil.read(in, logVersion);
+        xAttrs = XAttrsEditLogUtil.read(in, logVersion);
         this.clientName = FSImageSerialization.readString(in);
         this.clientMachine = FSImageSerialization.readString(in);
         // read clientId and callId

Is it possible for xAttrs (and aclEntries for that matter) to be
uninit'd if opCode != OP_ADD? I'm looking at this.clientName and
this.clientMachine and wondering why there isn't an equivalent for
xAttrs (and aclEntries) in the else?

Index: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
===================================================================
--- 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
     (revision 0)
+++ 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
     (working copy)
@@ -0,0 +1,75 @@
+/**
+ * There are 4 types for extended attribute <XAttr>:

There are four types of extended attributes <XAttr> defined by the
following namespaces:

+ * <p/>
+ * USER -- extended user attributes: can be assigned to files and directories 
+ * for storing arbitrary additional information. The access permissions for 
user 
+ * attributes are defined by the file permission bits.

USER -- extended user attributes: these can be assigned to files and
directories to store arbitrary additional information. The access
permissions for user attributes are defined by the file permission
bits.

+ * <p/>
+ * TRUSTED -- trusted extended attributes: are visible and accessible only to 
+ * super user.

TRUSTED -- trusted extended attributes: these are visible/accessible
only to/by the super user.

+ * <p/>
+ * SECURITY -- extended security attributes: is used by hdfs core for security 
purpose, 
+ * and not available through admin/user api.

SECURITY -- extended security attributes: these are used by the HDFS
core for security purposes and are not available through admin/user
API.

+ * <p/>
+ * SYSTEM -- extended system attributes: is used by hdfs core, and not 
+ * available through admin/user api.

SYSTEM -- extended system attributes: these are used by the HDFS
core and are not available through admin/user API.

Index: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java
===================================================================
--- 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java
      (revision 0)
+++ 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java
      (working copy)
@@ -0,0 +1,147 @@
+  public static List<XAttr> filterINodeXAttr(INode inode,
+      int snapshotId, XAttr xAttr) throws QuotaExceededException {
+    List<XAttr> all = readINodeXAttrs(inode, snapshotId);
+    if (xAttr == null) {
+      return all;
+    }
+    
+    Builder<XAttr> builder = new ImmutableList.Builder<XAttr>();
+    for (XAttr a : all) {
+      if (!(a.getNameSpace() == xAttr.getNameSpace()
+          && a.getName().equalsIgnoreCase(xAttr.getName()))) {

Does the namespace need to be filtered for case here too?

+  /**
+   * Set an inode xattr
+   * @param inode INode to set
+   * @param snapshotId
+   * @param xAttr <code>XAttr</code> to set.
+   * @param flag <code>XAttrSetFlag</code> 
+   * @return List<XAttr> INode <code>XAttr</code> list.
+   * @throws QuotaExceededException
+   * @throws IOException
+   */
+  public static List<XAttr> setINodeXAttr(INode inode, int snapshotId, 
+      XAttr xAttr, EnumSet<XAttrSetFlag> flag) throws QuotaExceededException, 
IOException {
+    List<XAttr> all = readINodeXAttrs(inode, snapshotId);
+    if (xAttr == null) {
+      return all;
+    }
+    
+    Builder<XAttr> builder = new ImmutableList.Builder<XAttr>();
+    boolean exist = false;
+    for (XAttr a: all) {
+      if ((a.getNameSpace() == xAttr.getNameSpace()
+          && a.getName().equalsIgnoreCase(xAttr.getName()))) {

same question.



> Support XAttrs from NameNode and implements XAttr APIs for 
> DistributedFileSystem
> --------------------------------------------------------------------------------
>
>                 Key: HDFS-6258
>                 URL: https://issues.apache.org/jira/browse/HDFS-6258
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode
>    Affects Versions: HDFS XAttrs (HDFS-2006)
>            Reporter: Yi Liu
>            Assignee: Yi Liu
>         Attachments: HDFS-6258.1.patch, HDFS-6258.2.patch, HDFS-6258.3.patch, 
> HDFS-6258.patch
>
>
> This JIRA is to implement extended attributes in HDFS: support XAttrs from 
> NameNode, implements XAttr APIs for DistributedFileSystem and so on.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to