HDFS-10506. OIV's ReverseXML processor cannot reconstruct some snapshot details. Contributed by Akira Ajisaka.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageReconstructor.java

@@ -568,6 +568,13 @@ class OfflineImageReconstructor {
   private void processFileXml(Node node, INodeSection.INode.Builder inodeBld)
       throws IOException {
     inodeBld.setType(INodeSection.INode.Type.FILE);
+    INodeSection.INodeFile.Builder bld = createINodeFileBuilder(node);
+    inodeBld.setFile(bld);
+    // Will check remaining keys and serialize in processINodeXml
+  }
+
+  private INodeSection.INodeFile.Builder createINodeFileBuilder(Node node)
+      throws IOException {
     INodeSection.INodeFile.Builder bld = INodeSection.INodeFile.newBuilder();
     Integer ival = node.removeChildInt(SECTION_REPLICATION);
     if (ival != null) {
@@ -596,24 +603,7 @@ class OfflineImageReconstructor {
         if (block == null) {
           break;
         }
-        HdfsProtos.BlockProto.Builder blockBld =
+        bld.addBlocks(createBlockBuilder(block));
-            HdfsProtos.BlockProto.newBuilder();
-        Long id = block.removeChildLong(SECTION_ID);
-        if (id == null) {
-          throw new IOException("<block> found without <id>");
-        }
-        blockBld.setBlockId(id);
-        Long genstamp = block.removeChildLong(INODE_SECTION_GEMSTAMP);
-        if (genstamp == null) {
-          throw new IOException("<block> found without <genstamp>");
-        }
-        blockBld.setGenStamp(genstamp);
-        Long numBytes = block.removeChildLong(INODE_SECTION_NUM_BYTES);
-        if (numBytes == null) {
-          throw new IOException("<block> found without <numBytes>");
-        }
-        blockBld.setNumBytes(numBytes);
-        bld.addBlocks(blockBld);
       }
     }
     Node fileUnderConstruction =
@@ -650,13 +640,43 @@ class OfflineImageReconstructor {
     if (ival != null) {
       bld.setStoragePolicyID(ival);
     }
-    inodeBld.setFile(bld);
+    return bld;
     // Will check remaining keys and serialize in processINodeXml
   }
 
+  private HdfsProtos.BlockProto.Builder createBlockBuilder(Node block)
+      throws IOException {
+    HdfsProtos.BlockProto.Builder blockBld =
+        HdfsProtos.BlockProto.newBuilder();
+    Long id = block.removeChildLong(SECTION_ID);
+    if (id == null) {
+      throw new IOException("<block> found without <id>");
+    }
+    blockBld.setBlockId(id);
+    Long genstamp = block.removeChildLong(INODE_SECTION_GENSTAMP);
+    if (genstamp == null) {
+      throw new IOException("<block> found without <genstamp>");
+    }
+    blockBld.setGenStamp(genstamp);
+    Long numBytes = block.removeChildLong(INODE_SECTION_NUM_BYTES);
+    if (numBytes == null) {
+      throw new IOException("<block> found without <numBytes>");
+    }
+    blockBld.setNumBytes(numBytes);
+    return blockBld;
+  }
+
   private void processDirectoryXml(Node node,
           INodeSection.INode.Builder inodeBld) throws IOException {
     inodeBld.setType(INodeSection.INode.Type.DIRECTORY);
+    INodeSection.INodeDirectory.Builder bld =
+        createINodeDirectoryBuilder(node);
+    inodeBld.setDirectory(bld);
+    // Will check remaining keys and serialize in processINodeXml
+  }
+
+  private INodeSection.INodeDirectory.Builder
+      createINodeDirectoryBuilder(Node node) throws IOException {
     INodeSection.INodeDirectory.Builder bld =
         INodeSection.INodeDirectory.newBuilder();
     Long lval = node.removeChildLong(INODE_SECTION_MTIME);
@@ -710,8 +730,7 @@ class OfflineImageReconstructor {
       qf.addQuotas(qbld);
     }
     bld.setTypeQuotas(qf);
-    inodeBld.setDirectory(bld);
+    return bld;
-    // Will check remaining keys and serialize in processINodeXml
   }
 
   private void processSymlinkXml(Node node,
@@ -1355,7 +1374,11 @@ class OfflineImageReconstructor {
         if (name != null) {
           bld.setName(ByteString.copyFrom(name, "UTF8"));
         }
-        // TODO: add missing snapshotCopy field to XML
+        Node snapshotCopy = dirDiff.removeChild(
+            SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY);
+        if (snapshotCopy != null) {
+          bld.setSnapshotCopy(createINodeDirectoryBuilder(snapshotCopy));
+        }
         Integer expectedCreatedListSize = dirDiff.removeChildInt(
             SNAPSHOT_DIFF_SECTION_CREATED_LIST_SIZE);
         if (expectedCreatedListSize == null) {
@@ -1454,8 +1477,21 @@ class OfflineImageReconstructor {
         if (name != null) {
           bld.setName(ByteString.copyFrom(name, "UTF8"));
         }
-        // TODO: missing snapshotCopy
+        Node snapshotCopy = fileDiff.removeChild(
-        // TODO: missing blocks
+            SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY);
+        if (snapshotCopy != null) {
+          bld.setSnapshotCopy(createINodeFileBuilder(snapshotCopy));
+        }
+        Node blocks = fileDiff.removeChild(INODE_SECTION_BLOCKS);
+        if (blocks != null) {
+          while (true) {
+            Node block = blocks.removeChild(INODE_SECTION_BLOCK);
+            if (block == null) {
+              break;
+            }
+            bld.addBlocks(createBlockBuilder(block));
+          }
+        }
         fileDiff.verifyNoRemainingKeys("fileDiff");
         bld.build().writeDelimitedTo(out);
       }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/PBImageXmlWriter.java

@@ -118,7 +118,7 @@ public final class PBImageXmlWriter {
   public static final String INODE_SECTION_PERMISSION = "permission";
   public static final String INODE_SECTION_BLOCKS = "blocks";
   public static final String INODE_SECTION_BLOCK = "block";
-  public static final String INODE_SECTION_GEMSTAMP = "genstamp";
+  public static final String INODE_SECTION_GENSTAMP = "genstamp";
   public static final String INODE_SECTION_NUM_BYTES = "numBytes";
   public static final String INODE_SECTION_FILE_UNDER_CONSTRUCTION =
       "file-under-construction";
@@ -188,6 +188,8 @@ public final class PBImageXmlWriter {
       "childrenSize";
   public static final String SNAPSHOT_DIFF_SECTION_IS_SNAPSHOT_ROOT =
       "isSnapshotRoot";
+  public static final String SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY =
+      "snapshotCopy";
   public static final String SNAPSHOT_DIFF_SECTION_CREATED_LIST_SIZE =
       "createdListSize";
   public static final String SNAPSHOT_DIFF_SECTION_DELETED_INODE =
@@ -482,7 +484,7 @@ public final class PBImageXmlWriter {
       for (BlockProto b : f.getBlocksList()) {
         out.print("<" + INODE_SECTION_BLOCK + ">");
         o(SECTION_ID, b.getBlockId())
-            .o(INODE_SECTION_GEMSTAMP, b.getGenStamp())
+            .o(INODE_SECTION_GENSTAMP, b.getGenStamp())
             .o(INODE_SECTION_NUM_BYTES, b.getNumBytes());
         out.print("</" + INODE_SECTION_BLOCK + ">\n");
       }
@@ -660,6 +662,23 @@ public final class PBImageXmlWriter {
           o(SNAPSHOT_DIFF_SECTION_SNAPSHOT_ID, f.getSnapshotId())
               .o(SNAPSHOT_DIFF_SECTION_SIZE, f.getFileSize())
               .o(SECTION_NAME, f.getName().toStringUtf8());
+          INodeSection.INodeFile snapshotCopy = f.getSnapshotCopy();
+          if (snapshotCopy != null) {
+            out.print("<" + SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY + ">");
+            dumpINodeFile(snapshotCopy);
+            out.print("</" + SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY + ">\n");
+          }
+          if (f.getBlocksCount() > 0) {
+            out.print("<" + INODE_SECTION_BLOCKS + ">");
+            for (BlockProto b : f.getBlocksList()) {
+              out.print("<" + INODE_SECTION_BLOCK + ">");
+              o(SECTION_ID, b.getBlockId())
+                  .o(INODE_SECTION_GENSTAMP, b.getGenStamp())
+                  .o(INODE_SECTION_NUM_BYTES, b.getNumBytes());
+              out.print("</" + INODE_SECTION_BLOCK + ">\n");
+            }
+            out.print("</" + INODE_SECTION_BLOCKS + ">\n");
+          }
           out.print("</" + SNAPSHOT_DIFF_SECTION_FILE_DIFF + ">\n");
         }
       }
@@ -672,9 +691,14 @@ public final class PBImageXmlWriter {
           o(SNAPSHOT_DIFF_SECTION_SNAPSHOT_ID, d.getSnapshotId())
               .o(SNAPSHOT_DIFF_SECTION_CHILDREN_SIZE, d.getChildrenSize())
               .o(SNAPSHOT_DIFF_SECTION_IS_SNAPSHOT_ROOT, d.getIsSnapshotRoot())
-              .o(SECTION_NAME, d.getName().toStringUtf8())
+              .o(SECTION_NAME, d.getName().toStringUtf8());
-              .o(SNAPSHOT_DIFF_SECTION_CREATED_LIST_SIZE,
+          INodeDirectory snapshotCopy = d.getSnapshotCopy();
-                  d.getCreatedListSize());
+          if (snapshotCopy != null) {
+            out.print("<" + SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY + ">");
+            dumpINodeDirectory(snapshotCopy);
+            out.print("</" + SNAPSHOT_DIFF_SECTION_SNAPSHOT_COPY + ">\n");
+          }
+          o(SNAPSHOT_DIFF_SECTION_CREATED_LIST_SIZE, d.getCreatedListSize());
           for (long did : d.getDeletedINodeList()) {
             o(SNAPSHOT_DIFF_SECTION_DELETED_INODE, did);
           }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java

@@ -181,14 +181,27 @@ public class TestOfflineImageViewer {
       hdfs.mkdirs(src);
       dirCount++;
       writtenFiles.put(src.toString(), hdfs.getFileStatus(src));
+
+      // Create snapshot and snapshotDiff.
       final Path orig = new Path("/src/orig");
       hdfs.mkdirs(orig);
+      final Path file1 = new Path("/src/file");
+      FSDataOutputStream o = hdfs.create(file1);
+      o.write(23);
+      o.write(45);
+      o.close();
       hdfs.allowSnapshot(src);
       hdfs.createSnapshot(src, "snapshot");
       final Path dst = new Path("/dst");
+      // Rename a directory in the snapshot directory to add snapshotCopy
+      // field to the dirDiff entry.
       hdfs.rename(orig, dst);
       dirCount++;
       writtenFiles.put(dst.toString(), hdfs.getFileStatus(dst));
+      // Truncate a file in the snapshot directory to add snapshotCopy and
+      // blocks fields to the fileDiff entry.
+      hdfs.truncate(file1, 1);
+      writtenFiles.put(file1.toString(), hdfs.getFileStatus(file1));
 
       // Set XAttrs so the fsimage contains XAttr ops
       final Path xattr = new Path("/xattr");
@@ -279,7 +292,7 @@ public class TestOfflineImageViewer {
     Matcher matcher = p.matcher(outputString);
     assertTrue(matcher.find() && matcher.groupCount() == 1);
     int totalFiles = Integer.parseInt(matcher.group(1));
-    assertEquals(NUM_DIRS * FILES_PER_DIR, totalFiles);
+    assertEquals(NUM_DIRS * FILES_PER_DIR + 1, totalFiles);
 
     p = Pattern.compile("totalDirectories = (\\d+)\n");
     matcher = p.matcher(outputString);