Author: wang
Date: Mon May 12 17:55:23 2014
New Revision: 1594037
URL: http://svn.apache.org/r1594037
Log:
HDFS-6351. Command hdfs dfs -rm -r can't remove empty directory. Contributed by
Yongjun Zhang.
Added:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFsShellPermission.java
- copied unchanged from r1594036,
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFsShellPermission.java
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/ (props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/ (props
changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/native/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/datanode/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/secondary/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/hdfs/
(props changed)
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/
(props changed)
Propchange: hadoop/common/branches/branch-2/hadoop-hdfs-project/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project:r1589530,1590776,1592438,1594036
Propchange: hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/
------------------------------------------------------------------------------
Merged /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs:r1594036
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1594037&r1=1594036&r2=1594037&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
(original)
+++ hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
Mon May 12 17:55:23 2014
@@ -188,12 +188,15 @@ Release 2.5.0 - UNRELEASED
HDFS-6337. Setfacl testcase is failing due to dash character in username
in TestAclCLI (umamahesh)
- HDFS-5381. ExtendedBlock#hashCode should use both blockId and block pool
ID
+ HDFS-5381. ExtendedBlock#hashCode should use both blockId and block pool ID
(Benoy Antony via Colin Patrick McCabe)
HDFS-6240. WebImageViewer returns 404 if LISTSTATUS to an empty directory.
(Akira Ajisaka via wheat9)
+ HDFS-6351. Command hdfs dfs -rm -r can't remove empty directory.
+ (Yongjun Zhang via wang)
+
Release 2.4.1 - UNRELEASED
INCOMPATIBLE CHANGES
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java:r1594036
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1594037&r1=1594036&r2=1594037&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Mon May 12 17:55:23 2014
@@ -142,6 +142,7 @@ import org.apache.hadoop.fs.Options;
import org.apache.hadoop.fs.Options.Rename;
import org.apache.hadoop.fs.ParentNotDirectoryException;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
import org.apache.hadoop.fs.UnresolvedLinkException;
import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.AclStatus;
@@ -3242,10 +3243,11 @@ public class FSNamesystem implements Nam
// Rename does not operates on link targets
// Do not resolveLink when checking permissions of src and dst
// Check write access to parent of src
- checkPermission(pc, src, false, null, FsAction.WRITE, null, null, false);
+ checkPermission(pc, src, false, null, FsAction.WRITE, null, null,
+ false, false);
// Check write access to ancestor of dst
checkPermission(pc, actualdst, false, FsAction.WRITE, null, null, null,
- false);
+ false, false);
}
if (dir.renameTo(src, dst, logRetryCache)) {
@@ -3306,9 +3308,11 @@ public class FSNamesystem implements Nam
// Rename does not operates on link targets
// Do not resolveLink when checking permissions of src and dst
// Check write access to parent of src
- checkPermission(pc, src, false, null, FsAction.WRITE, null, null, false);
+ checkPermission(pc, src, false, null, FsAction.WRITE, null, null, false,
+ false);
// Check write access to ancestor of dst
- checkPermission(pc, dst, false, FsAction.WRITE, null, null, null, false);
+ checkPermission(pc, dst, false, FsAction.WRITE, null, null, null, false,
+ false);
}
dir.renameTo(src, dst, logRetryCache, options);
@@ -3388,11 +3392,11 @@ public class FSNamesystem implements Nam
checkNameNodeSafeMode("Cannot delete " + src);
src = FSDirectory.resolvePath(src, pathComponents, dir);
if (!recursive && dir.isNonEmptyDirectory(src)) {
- throw new IOException(src + " is non empty");
+ throw new PathIsNotEmptyDirectoryException(src + " is non empty");
}
if (enforcePermission && isPermissionEnabled) {
checkPermission(pc, src, false, null, FsAction.WRITE, null,
- FsAction.ALL, false);
+ FsAction.ALL, true, false);
}
// Unlink the target directory from directory tree
if (!dir.delete(src, collectedBlocks, removedINodes, logRetryCache)) {
@@ -3544,7 +3548,8 @@ public class FSNamesystem implements Nam
checkOperation(OperationCategory.READ);
src = FSDirectory.resolvePath(src, pathComponents, dir);
if (isPermissionEnabled) {
- checkPermission(pc, src, false, null, null, null, null, resolveLink);
+ checkPermission(pc, src, false, null, null, null, null, false,
+ resolveLink);
}
stat = dir.getFileInfo(src, resolveLink);
} catch (AccessControlException e) {
@@ -5543,7 +5548,7 @@ public class FSNamesystem implements Nam
FsAction parentAccess, FsAction access, FsAction subAccess)
throws AccessControlException, UnresolvedLinkException {
checkPermission(pc, path, doCheckOwner, ancestorAccess,
- parentAccess, access, subAccess, true);
+ parentAccess, access, subAccess, false, true);
}
/**
@@ -5554,14 +5559,14 @@ public class FSNamesystem implements Nam
private void checkPermission(FSPermissionChecker pc,
String path, boolean doCheckOwner, FsAction ancestorAccess,
FsAction parentAccess, FsAction access, FsAction subAccess,
- boolean resolveLink)
+ boolean ignoreEmptyDir, boolean resolveLink)
throws AccessControlException, UnresolvedLinkException {
if (!pc.isSuperUser()) {
dir.waitForReady();
readLock();
try {
pc.checkPermission(path, dir, doCheckOwner, ancestorAccess,
- parentAccess, access, subAccess, resolveLink);
+ parentAccess, access, subAccess, ignoreEmptyDir, resolveLink);
} finally {
readUnlock();
}
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java?rev=1594037&r1=1594036&r2=1594037&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
Mon May 12 17:55:23 2014
@@ -32,6 +32,7 @@ import org.apache.hadoop.fs.permission.A
import org.apache.hadoop.fs.permission.AclEntryType;
import org.apache.hadoop.fs.permission.FsAction;
import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.util.ReadOnlyList;
import org.apache.hadoop.security.AccessControlException;
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.util.StringUtils;
@@ -136,6 +137,7 @@ class FSPermissionChecker {
* @param subAccess If path is a directory,
* it is the access required of the path and all the sub-directories.
* If path is not a directory, there is no effect.
+ * @param ignoreEmptyDir Ignore permission checking for empty directory?
* @param resolveLink whether to resolve the final path component if it is
* a symlink
* @throws AccessControlException
@@ -146,7 +148,7 @@ class FSPermissionChecker {
*/
void checkPermission(String path, FSDirectory dir, boolean doCheckOwner,
FsAction ancestorAccess, FsAction parentAccess, FsAction access,
- FsAction subAccess, boolean resolveLink)
+ FsAction subAccess, boolean ignoreEmptyDir, boolean resolveLink)
throws AccessControlException, UnresolvedLinkException {
if (LOG.isDebugEnabled()) {
LOG.debug("ACCESS CHECK: " + this
@@ -155,6 +157,7 @@ class FSPermissionChecker {
+ ", parentAccess=" + parentAccess
+ ", access=" + access
+ ", subAccess=" + subAccess
+ + ", ignoreEmptyDir=" + ignoreEmptyDir
+ ", resolveLink=" + resolveLink);
}
// check if (parentAccess != null) && file exists, then check sb
@@ -182,7 +185,7 @@ class FSPermissionChecker {
check(last, snapshotId, access);
}
if (subAccess != null) {
- checkSubAccess(last, snapshotId, subAccess);
+ checkSubAccess(last, snapshotId, subAccess, ignoreEmptyDir);
}
if (doCheckOwner) {
checkOwner(last, snapshotId);
@@ -207,8 +210,8 @@ class FSPermissionChecker {
}
/** Guarded by {@link FSNamesystem#readLock()} */
- private void checkSubAccess(INode inode, int snapshotId, FsAction access
- ) throws AccessControlException {
+ private void checkSubAccess(INode inode, int snapshotId, FsAction access,
+ boolean ignoreEmptyDir) throws AccessControlException {
if (inode == null || !inode.isDirectory()) {
return;
}
@@ -216,9 +219,12 @@ class FSPermissionChecker {
Stack<INodeDirectory> directories = new Stack<INodeDirectory>();
for(directories.push(inode.asDirectory()); !directories.isEmpty(); ) {
INodeDirectory d = directories.pop();
- check(d, snapshotId, access);
+ ReadOnlyList<INode> cList = d.getChildrenList(snapshotId);
+ if (!(cList.isEmpty() && ignoreEmptyDir)) {
+ check(d, snapshotId, access);
+ }
- for(INode child : d.getChildrenList(snapshotId)) {
+ for(INode child : cList) {
if (child.isDirectory()) {
directories.push(child.asDirectory());
}
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/native/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native:r1594036
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/datanode/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/datanode:r1594036
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs:r1594036
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/secondary/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/secondary:r1594036
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/hdfs/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/hdfs:r1594036
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java?rev=1594037&r1=1594036&r2=1594037&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
Mon May 12 17:55:23 2014
@@ -429,6 +429,7 @@ public class TestDFSPermission {
short[] ancestorPermission, short[] parentPermission,
short[] filePermission, Path[] parentDirs, Path[] files, Path[] dirs)
throws Exception {
+ boolean[] isDirEmpty = new boolean[NUM_TEST_PERMISSIONS];
login(SUPERUSER);
for (int i = 0; i < NUM_TEST_PERMISSIONS; i++) {
create(OpType.CREATE, files[i]);
@@ -441,6 +442,8 @@ public class TestDFSPermission {
FsPermission fsPermission = new FsPermission(filePermission[i]);
fs.setPermission(files[i], fsPermission);
fs.setPermission(dirs[i], fsPermission);
+
+ isDirEmpty[i] = (fs.listStatus(dirs[i]).length == 0);
}
login(ugi);
@@ -461,7 +464,7 @@ public class TestDFSPermission {
parentPermission[i], ancestorPermission[next],
parentPermission[next]);
testDeleteFile(ugi, files[i], ancestorPermission[i],
parentPermission[i]);
testDeleteDir(ugi, dirs[i], ancestorPermission[i], parentPermission[i],
- filePermission[i], null);
+ filePermission[i], null, isDirEmpty[i]);
}
// test non existent file
@@ -924,7 +927,8 @@ public class TestDFSPermission {
}
/* A class that verifies the permission checking is correct for
- * directory deletion */
+ * directory deletion
+ */
private class DeleteDirPermissionVerifier extends DeletePermissionVerifier {
private short[] childPermissions;
@@ -958,6 +962,17 @@ public class TestDFSPermission {
}
}
+ /* A class that verifies the permission checking is correct for
+ * empty-directory deletion
+ */
+ private class DeleteEmptyDirPermissionVerifier extends
DeleteDirPermissionVerifier {
+ @Override
+ void setOpPermission() {
+ this.opParentPermission = SEARCH_MASK | WRITE_MASK;
+ this.opPermission = NULL_MASK;
+ }
+ }
+
final DeletePermissionVerifier fileDeletionVerifier =
new DeletePermissionVerifier();
@@ -971,14 +986,19 @@ public class TestDFSPermission {
final DeleteDirPermissionVerifier dirDeletionVerifier =
new DeleteDirPermissionVerifier();
+ final DeleteEmptyDirPermissionVerifier emptyDirDeletionVerifier =
+ new DeleteEmptyDirPermissionVerifier();
+
/* test if the permission checking of directory deletion is correct */
private void testDeleteDir(UserGroupInformation ugi, Path path,
short ancestorPermission, short parentPermission, short permission,
- short[] childPermissions) throws Exception {
- dirDeletionVerifier.set(path, ancestorPermission, parentPermission,
- permission, childPermissions);
- dirDeletionVerifier.verifyPermission(ugi);
-
+ short[] childPermissions,
+ final boolean isDirEmpty) throws Exception {
+ DeleteDirPermissionVerifier ddpv = isDirEmpty?
+ emptyDirDeletionVerifier : dirDeletionVerifier;
+ ddpv.set(path, ancestorPermission, parentPermission, permission,
+ childPermissions);
+ ddpv.verifyPermission(ugi);
}
/* log into dfs as the given user */
Modified:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java?rev=1594037&r1=1594036&r2=1594037&view=diff
==============================================================================
---
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java
(original)
+++
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java
Mon May 12 17:55:23 2014
@@ -393,14 +393,14 @@ public class TestFSPermissionChecker {
private void assertPermissionGranted(UserGroupInformation user, String path,
FsAction access) throws IOException {
new FSPermissionChecker(SUPERUSER, SUPERGROUP, user).checkPermission(path,
- dir, false, null, null, access, null, true);
+ dir, false, null, null, access, null, false, true);
}
private void assertPermissionDenied(UserGroupInformation user, String path,
FsAction access) throws IOException {
try {
new FSPermissionChecker(SUPERUSER, SUPERGROUP,
user).checkPermission(path,
- dir, false, null, null, access, null, true);
+ dir, false, null, null, access, null, false, true);
fail("expected AccessControlException for user + " + user + ", path = " +
path + ", access = " + access);
} catch (AccessControlException e) {
Propchange:
hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/
------------------------------------------------------------------------------
Merged
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot:r1594036
