curie71 opened a new pull request, #5206:
URL: https://github.com/apache/hadoop/pull/5206

   …stem.
   
   HDFS-16868 checkSuperuserPrivilege and it' s caller log the same msg when an 
ACE occurs.
   
   checkSuperuserPrivilege call logAuditEvent and throw ace when an 
AccessControlException occurs.
   {code:java}
     // This method logs operationName without super user privilege.
     // It should be called without holding FSN lock.
     void checkSuperuserPrivilege(String operationName, String path)
         throws IOException {
       if (isPermissionEnabled) {
         try {
           FSPermissionChecker.setOperationType(operationName);
           FSPermissionChecker pc = getPermissionChecker();
           pc.checkSuperuserPrivilege(path);
         } catch(AccessControlException ace){
           logAuditEvent(false, operationName, path);
           throw ace;
         }
       }
     }
   {code}
   It' s callers like metaSave call it like this: 
   {code:java}
     /**
      * Dump all metadata into specified file
      * @param filename
      */
     void metaSave(String filename) throws IOException {
       String operationName = "metaSave";
       checkSuperuserPrivilege(operationName);
       ......
       try {
           ......
           metaSave(out);
           ......
         }
       } finally {
         readUnlock(operationName, getLockReportInfoSupplier(null));
       }
       logAuditEvent(true, operationName, null);
     }
   {code}
   but setQuota, addCachePool, modifyCachePool, removeCachePool, 
createEncryptionZone and reencryptEncryptionZone catch the ace and log the same 
msg again, it' s a waste of memory I think: 
   
   {code:java}
     /**
      * Set the namespace quota and storage space quota for a directory.
      * See {@link ClientProtocol#setQuota(String, long, long, StorageType)} 
for the
      * contract.
      * 
      * Note: This does not support ".inodes" relative path.
      */
     void setQuota(String src, long nsQuota, long ssQuota, StorageType type)
         throws IOException {
       ......
       try {
         if(!allowOwnerSetQuota) {
           checkSuperuserPrivilege(operationName, src);
         }
        ......
       } catch (AccessControlException ace) {
         logAuditEvent(false, operationName, src);
         throw ace;
       }
       getEditLog().logSync();
       logAuditEvent(true, operationName, src);
     }
   {code}
   Maybe we should move the checkSuperuserPrivilege out of the try block as 
metaSave and other callers do.
   
   
   <!--
     Thanks for sending a pull request!
       1. If this is your first time, please read our contributor guidelines: 
https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
       2. Make sure your PR title starts with JIRA issue id, e.g., 
'HADOOP-17799. Your PR title ...'.
   -->
   
   ### Description of PR
   
   
   ### How was this patch tested?
   
   
   ### For code changes:
   
   - [x] Does the title or this PR starts with the corresponding JIRA issue id 
(e.g. 'HADOOP-17799. Your PR title ...')?
   - [ ] Object storage: have the integration tests been executed and the 
endpoint declared according to the connector-specific documentation?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, 
`NOTICE-binary` files?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to