Pearl1594 commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717303713



##########
File path: 
server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
 
         private void cleanupOldDiagnosticFiles(DataStore store) {
             String mountPoint = null;
-            try {
-                mountPoint = 
serviceImpl.mountManager.getMountPoint(store.getUri(), null);
-                if (StringUtils.isNotBlank(mountPoint)) {
-                    File directory = new File(mountPoint + File.separator + 
DIAGNOSTICS_DIRECTORY);
-                    if (directory.isDirectory()) {
-                        deleteOldDiagnosticsFiles(directory, store.getName());
-                    }
-                }
-            } finally {
-                if (StringUtils.isNotBlank(mountPoint)) {
-                    umountSecondaryStorage(mountPoint);
+            mountPoint = 
serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+            if (StringUtils.isNotBlank(mountPoint)) {
+                File directory = new File(mountPoint + File.separator + 
DIAGNOSTICS_DIRECTORY);
+                if (directory.isDirectory()) {
+                    deleteOldDiagnosticsFiles(directory, store.getName());

Review comment:
       Sorry @sureshanaparti I seemed to have missed your earlier comment. 
Currently what's happening is that due to the permission issue, we see the 
secondary stores remain mounted on all the hypervisors. We could fix the 
permission issue using this solution - 
https://github.com/shapeblue/cloudstack/commit/a9b460869b8f244f4b3db3cc3973f3419dffed83.
 However, I am of the opinion that we probably don't need to mount and un-mount 
every time the garbage collection thread runs. As fixing the permission issue 
wouldn't solve the issue entirely, we'd also need to cleanup the mount path (at 
/var/cloudstack/mnt) and remove that mount path from the static list of 
storageMounts maintained, so that in successive runs are successful. 




-- 
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]


Reply via email to