HDFS-6951. Correctly persist raw namespace xattrs to edit log and fsimage. Contributed by Charles Lamb.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java

@@ -111,6 +111,18 @@ public class EncryptionZoneManager {
    */
   void addEncryptionZone(Long inodeId, String keyName) {
     assert dir.hasWriteLock();
+    unprotectedAddEncryptionZone(inodeId, keyName);
+  }
+
+  /**
+   * Add a new encryption zone.
+   * <p/>
+   * Does not assume that the FSDirectory lock is held.
+   *
+   * @param inodeId of the encryption zone
+   * @param keyName encryption zone key name
+   */
+  void unprotectedAddEncryptionZone(Long inodeId, String keyName) {
     final EncryptionZoneInt ez = new EncryptionZoneInt(inodeId, keyName);
     encryptionZones.put(inodeId, ez);
   }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java

@@ -2100,7 +2100,7 @@ public class FSDirectory implements Closeable {
           for (XAttr xattr : xattrs) {
             final String xaName = XAttrHelper.getPrefixName(xattr);
             if (CRYPTO_XATTR_ENCRYPTION_ZONE.equals(xaName)) {
-              ezManager.addEncryptionZone(inode.getId(),
+              ezManager.unprotectedAddEncryptionZone(inode.getId(),
                   new String(xattr.getValue()));
             }
           }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java

@@ -82,6 +82,11 @@ public final class FSImageFormatPBINode {
   private static final int XATTR_NAMESPACE_OFFSET = 30;
   private static final int XATTR_NAME_MASK = (1 << 24) - 1;
   private static final int XATTR_NAME_OFFSET = 6;
+
+  /* See the comments in fsimage.proto for an explanation of the following. */
+  private static final int XATTR_NAMESPACE_EXT_OFFSET = 5;
+  private static final int XATTR_NAMESPACE_EXT_MASK = 1;
+
   private static final XAttr.NameSpace[] XATTR_NAMESPACE_VALUES =
       XAttr.NameSpace.values();
   
@@ -122,6 +127,8 @@ public final class FSImageFormatPBINode {
         int v = xAttrCompactProto.getName();
         int nid = (v >> XATTR_NAME_OFFSET) & XATTR_NAME_MASK;
         int ns = (v >> XATTR_NAMESPACE_OFFSET) & XATTR_NAMESPACE_MASK;
+        ns |=
+            ((v >> XATTR_NAMESPACE_EXT_OFFSET) & XATTR_NAMESPACE_EXT_MASK) << 2;
         String name = stringTable[nid];
         byte[] value = null;
         if (xAttrCompactProto.getValue() != null) {
@@ -370,10 +377,13 @@ public final class FSImageFormatPBINode {
       for (XAttr a : f.getXAttrs()) {
         XAttrCompactProto.Builder xAttrCompactBuilder = XAttrCompactProto.
             newBuilder();
-        int v = ((a.getNameSpace().ordinal() & XATTR_NAMESPACE_MASK) << 
+        int nsOrd = a.getNameSpace().ordinal();
-            XATTR_NAMESPACE_OFFSET) 
+        Preconditions.checkArgument(nsOrd < 8, "Too many namespaces.");
+        int v = ((nsOrd & XATTR_NAMESPACE_MASK) << XATTR_NAMESPACE_OFFSET)
             | ((stringMap.getId(a.getName()) & XATTR_NAME_MASK) <<
                 XATTR_NAME_OFFSET);
+        v |= (((nsOrd >> 2) & XATTR_NAMESPACE_EXT_MASK) <<
+            XATTR_NAMESPACE_EXT_OFFSET);
         xAttrCompactBuilder.setName(v);
         if (a.getValue() != null) {
           xAttrCompactBuilder.setValue(PBHelper.getByteString(a.getValue()));

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java

@@ -67,7 +67,8 @@ public class NameNodeLayoutVersion {
     EDITLOG_LENGTH(-56, "Add length field to every edit log op"),
     XATTRS(-57, "Extended attributes"),
     CREATE_OVERWRITE(-58, "Use single editlog record for " +
-        "creating file with overwrite");
+      "creating file with overwrite"),
+    XATTRS_NAMESPACE_EXT(-59, "Increase number of xattr namespaces");
     
     private final FeatureInfo info;
 

hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto

@@ -114,7 +114,11 @@ message INodeSection {
      * [0:2) -- the namespace of XAttr (XAttrNamespaceProto)
      * [2:26) -- the name of the entry, which is an ID that points to a
      * string in the StringTableSection.
-     * [26:32) -- reserved for future uses.
+     * [26:27) -- namespace extension. Originally there were only 4 namespaces
+     * so only 2 bits were needed. At that time, this bit was reserved. When a
+     * 5th namespace was created (raw) this bit became used as a 3rd namespace
+     * bit.
+     * [27:32) -- reserved for future uses.
      */
     required fixed32 name = 1;
     optional bytes value = 2;

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java

@@ -49,6 +49,7 @@ import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.client.HdfsAdmin;
 import org.apache.hadoop.hdfs.protocol.EncryptionZone;
 import org.apache.hadoop.hdfs.protocol.LocatedBlocks;
+import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction;
 import org.apache.hadoop.hdfs.server.namenode.EncryptionFaultInjector;
 import org.apache.hadoop.hdfs.server.namenode.EncryptionZoneManager;
 import org.apache.hadoop.security.AccessControlException;
@@ -314,6 +315,13 @@ public class TestEncryptionZones {
       assertNumZones(numZones);
       assertZonePresent(null, zonePath.toString());
     }
+
+    fs.setSafeMode(SafeModeAction.SAFEMODE_ENTER);
+    fs.saveNamespace();
+    fs.setSafeMode(SafeModeAction.SAFEMODE_LEAVE);
+    cluster.restartNameNode(true);
+    assertNumZones(numZones);
+    assertZonePresent(null, zone1.toString());
   }
 
   /**

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSXAttrBaseTest.java

@@ -56,6 +56,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 
 /**
  * Tests NameNode interaction for all XAttr APIs.
@@ -129,51 +130,73 @@ public class FSXAttrBaseTest {
    */
   @Test(timeout = 120000)
   public void testCreateXAttr() throws Exception {
-    FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short)0750));
+    Map<String, byte[]> expectedXAttrs = Maps.newHashMap();
-    fs.setXAttr(path, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
+    expectedXAttrs.put(name1, value1);
+    expectedXAttrs.put(name2, null);
+    doTestCreateXAttr(path, expectedXAttrs);
+    expectedXAttrs.put(raw1, value1);
+    doTestCreateXAttr(rawPath, expectedXAttrs);
+  }
 
-    Map<String, byte[]> xattrs = fs.getXAttrs(path);
+  private void doTestCreateXAttr(Path usePath, Map<String,
+      byte[]> expectedXAttrs) throws Exception {
+    FileSystem.mkdirs(fs, usePath, FsPermission.createImmutable((short)0750));
+    fs.setXAttr(usePath, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
+    
+    Map<String, byte[]> xattrs = fs.getXAttrs(usePath);
     Assert.assertEquals(xattrs.size(), 1);
     Assert.assertArrayEquals(value1, xattrs.get(name1));
     
-    fs.removeXAttr(path, name1);
+    fs.removeXAttr(usePath, name1);
     
-    xattrs = fs.getXAttrs(path);
+    xattrs = fs.getXAttrs(usePath);
     Assert.assertEquals(xattrs.size(), 0);
     
     // Create xattr which already exists.
-    fs.setXAttr(path, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
+    fs.setXAttr(usePath, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
     try {
-      fs.setXAttr(path, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
+      fs.setXAttr(usePath, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
       Assert.fail("Creating xattr which already exists should fail.");
     } catch (IOException e) {
     }
-    fs.removeXAttr(path, name1);
+    fs.removeXAttr(usePath, name1);
     
-    // Create two xattrs
+    // Create the xattrs
-    fs.setXAttr(path, name1, value1, EnumSet.of(XAttrSetFlag.CREATE));
+    for (Map.Entry<String, byte[]> ent : expectedXAttrs.entrySet()) {
-    fs.setXAttr(path, name2, null, EnumSet.of(XAttrSetFlag.CREATE));
+      fs.setXAttr(usePath, ent.getKey(), ent.getValue(),
-    xattrs = fs.getXAttrs(path);
+          EnumSet.of(XAttrSetFlag.CREATE));
-    Assert.assertEquals(xattrs.size(), 2);
+    }
-    Assert.assertArrayEquals(value1, xattrs.get(name1));
+    xattrs = fs.getXAttrs(usePath);
-    Assert.assertArrayEquals(new byte[0], xattrs.get(name2));
+    Assert.assertEquals(xattrs.size(), expectedXAttrs.size());
+    for (Map.Entry<String, byte[]> ent : expectedXAttrs.entrySet()) {
+      final byte[] val =
+          (ent.getValue() == null) ? new byte[0] : ent.getValue();
+      Assert.assertArrayEquals(val, xattrs.get(ent.getKey()));
+    }
     
     restart(false);
     initFileSystem();
-    xattrs = fs.getXAttrs(path);
+    xattrs = fs.getXAttrs(usePath);
-    Assert.assertEquals(xattrs.size(), 2);
+    Assert.assertEquals(xattrs.size(), expectedXAttrs.size());
-    Assert.assertArrayEquals(value1, xattrs.get(name1));
+    for (Map.Entry<String, byte[]> ent : expectedXAttrs.entrySet()) {
-    Assert.assertArrayEquals(new byte[0], xattrs.get(name2));
+      final byte[] val =
+          (ent.getValue() == null) ? new byte[0] : ent.getValue();
+      Assert.assertArrayEquals(val, xattrs.get(ent.getKey()));
+    }
     
     restart(true);
     initFileSystem();
-    xattrs = fs.getXAttrs(path);
+    xattrs = fs.getXAttrs(usePath);
-    Assert.assertEquals(xattrs.size(), 2);
+    Assert.assertEquals(xattrs.size(), expectedXAttrs.size());
-    Assert.assertArrayEquals(value1, xattrs.get(name1));
+    for (Map.Entry<String, byte[]> ent : expectedXAttrs.entrySet()) {
-    Assert.assertArrayEquals(new byte[0], xattrs.get(name2));
+      final byte[] val =
+          (ent.getValue() == null) ? new byte[0] : ent.getValue();
+      Assert.assertArrayEquals(val, xattrs.get(ent.getKey()));
+    }
 
-    fs.removeXAttr(path, name1);
+    for (Map.Entry<String, byte[]> ent : expectedXAttrs.entrySet()) {
-    fs.removeXAttr(path, name2);
+      fs.removeXAttr(usePath, ent.getKey());
+    }
   }
   
   /**

hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <EDITS>
-  <EDITS_VERSION>-58</EDITS_VERSION>
+  <EDITS_VERSION>-59</EDITS_VERSION>
   <RECORD>
     <OPCODE>OP_START_LOG_SEGMENT</OPCODE>
     <DATA>