Merge release branch 4.6 to master

* 4.6:
  CLOUDSTACK-9022: move storage.cleanup related global configurations to StorageManager
  CLOUDSTACK-9022: keep Destroyed volumes for sometime

 Conflicts:
	server/src/com/cloud/storage/StorageManagerImpl.java

engine/schema/src/com/cloud/storage/dao/VolumeDao.java

@@ -16,6 +16,7 @@
 // under the License.
 package com.cloud.storage.dao;
 
+import java.util.Date;
 import java.util.List;
 
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
@@ -79,6 +80,8 @@ public interface VolumeDao extends GenericDao<VolumeVO, Long>, StateDao<Volume.S
 
     List<VolumeVO> listVolumesToBeDestroyed();
 
+    List<VolumeVO> listVolumesToBeDestroyed(Date date);
+
     ImageFormat getImageFormat(Long volumeId);
 
     List<VolumeVO> findReadyRootVolumesByInstance(long instanceId);

engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java

@@ -329,6 +329,7 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
         AllFieldsSearch.and("id", AllFieldsSearch.entity().getId(), Op.EQ);
         AllFieldsSearch.and("destroyed", AllFieldsSearch.entity().getState(), Op.EQ);
         AllFieldsSearch.and("notDestroyed", AllFieldsSearch.entity().getState(), Op.NEQ);
+        AllFieldsSearch.and("updateTime", AllFieldsSearch.entity().getUpdated(), SearchCriteria.Op.LT);
         AllFieldsSearch.and("updatedCount", AllFieldsSearch.entity().getUpdatedCount(), Op.EQ);
         AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.done();
@@ -480,6 +481,15 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> listVolumesToBeDestroyed(Date date) {
+        SearchCriteria<VolumeVO> sc = AllFieldsSearch.create();
+        sc.setParameters("state", Volume.State.Destroy);
+        sc.setParameters("updateTime", date);
+
+        return listBy(sc);
+    }
+
     @Override
     public boolean updateState(com.cloud.storage.Volume.State currentState, Event event, com.cloud.storage.Volume.State nextState, Volume vo, Object data) {
 

server/src/com/cloud/configuration/Config.java

@@ -664,15 +664,6 @@ public enum Config {
             "600",
             "Time in seconds between retries to stop or destroy a vm",
             null),
-    StorageCleanupInterval(
-            "Advanced",
-            StorageManager.class,
-            Integer.class,
-            "storage.cleanup.interval",
-            "86400",
-            "The interval (in seconds) to wait before running the storage cleanup thread.",
-            null),
-    StorageCleanupEnabled("Advanced", StorageManager.class, Boolean.class, "storage.cleanup.enabled", "true", "Enables/disables the storage cleanup thread.", null),
     UpdateWait("Advanced", AgentManager.class, Integer.class, "update.wait", "600", "Time to wait (in seconds) before alerting on a updating agent", null),
     XapiWait("Advanced", AgentManager.class, Integer.class, "xapiwait", "60", "Time (in seconds) to wait for XAPI to return", null),
     MigrateWait("Advanced", AgentManager.class, Integer.class, "migratewait", "3600", "Time (in seconds) to wait for VM migrate finish", null),

server/src/com/cloud/storage/StorageManager.java

@@ -21,6 +21,7 @@ import java.util.List;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
+import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 
 import com.cloud.agent.api.Answer;
@@ -39,6 +40,13 @@ import com.cloud.utils.Pair;
 import com.cloud.vm.VMInstanceVO;
 
 public interface StorageManager extends StorageService {
+    static final ConfigKey<Integer> StorageCleanupInterval = new ConfigKey<Integer>(Integer.class, "storage.cleanup.interval", "Advanced", "86400",
+            "The interval (in seconds) to wait before running the storage cleanup thread.", false, ConfigKey.Scope.Global, null);
+    static final ConfigKey<Integer> StorageCleanupDelay = new ConfigKey<Integer>(Integer.class, "storage.cleanup.delay", "Advanced", "86400",
+            "Determines how long (in seconds) to wait before actually expunging destroyed volumes. The default value = the default value of storage.cleanup.interval.", false, ConfigKey.Scope.Global, null);
+    static final ConfigKey<Boolean> StorageCleanupEnabled = new ConfigKey<Boolean>(Boolean.class, "storage.cleanup.enabled", "Advanced", "true",
+            "Enables/disables the storage cleanup thread.", false, ConfigKey.Scope.Global, null);
+
     /**
      * Returns a comma separated list of tags for the specified storage pool
      * @param poolId

server/src/com/cloud/storage/StorageManagerImpl.java

@@ -78,6 +78,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
 import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
@@ -188,7 +190,7 @@ import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.dao.VMInstanceDao;
 
 @Component
-public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener {
+public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener, Configurable {
     private static final Logger s_logger = Logger.getLogger(StorageManagerImpl.class);
 
     protected String _name;
@@ -295,9 +297,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
     protected SearchBuilder<StoragePoolVO> LocalStorageSearch;
 
     ScheduledExecutorService _executor = null;
-    boolean _storageCleanupEnabled;
     boolean _templateCleanupEnabled = true;
-    int _storageCleanupInterval;
     int _storagePoolAcquisitionWaitSeconds = 1800; // 30 minutes
     int _downloadUrlCleanupInterval;
     int _downloadUrlExpirationInterval;
@@ -460,17 +460,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
 
         _agentMgr.registerForHostEvents(new StoragePoolMonitor(this, _storagePoolDao), true, false, true);
 
-        String storageCleanupEnabled = configs.get("storage.cleanup.enabled");
-        _storageCleanupEnabled = (storageCleanupEnabled == null) ? true : Boolean.parseBoolean(storageCleanupEnabled);
-
         String value = _configDao.getValue(Config.StorageTemplateCleanupEnabled.key());
         _templateCleanupEnabled = (value == null ? true : Boolean.parseBoolean(value));
 
-        String time = configs.get("storage.cleanup.interval");
+        s_logger.info("Storage cleanup enabled: " + StorageCleanupEnabled.value() + ", interval: " + StorageCleanupInterval.value() + ", delay: " + StorageCleanupDelay.value()  +
-        _storageCleanupInterval = NumbersUtil.parseInt(time, 86400);
+                ", template cleanup enabled: " + _templateCleanupEnabled);
-
-        s_logger.info("Storage cleanup enabled: " + _storageCleanupEnabled + ", interval: " + _storageCleanupInterval + ", template cleanup enabled: " +
-                _templateCleanupEnabled);
 
         String cleanupInterval = configs.get("extract.url.cleanup.interval");
         _downloadUrlCleanupInterval = NumbersUtil.parseInt(cleanupInterval, 7200);
@@ -523,10 +517,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
 
     @Override
     public boolean start() {
-        if (_storageCleanupEnabled) {
+        if (StorageCleanupEnabled.value()) {
             Random generator = new Random();
-            int initialDelay = generator.nextInt(_storageCleanupInterval);
+            int initialDelay = generator.nextInt(StorageCleanupInterval.value());
-            _executor.scheduleWithFixedDelay(new StorageGarbageCollector(), initialDelay, _storageCleanupInterval, TimeUnit.SECONDS);
+            _executor.scheduleWithFixedDelay(new StorageGarbageCollector(), initialDelay, StorageCleanupInterval.value(), TimeUnit.SECONDS);
         } else {
             s_logger.debug("Storage cleanup is not enabled, so the storage cleanup thread is not being scheduled.");
         }
@@ -538,7 +532,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
 
     @Override
     public boolean stop() {
-        if (_storageCleanupEnabled) {
+        if (StorageCleanupEnabled.value()) {
             _executor.shutdown();
         }
         return true;
@@ -1109,7 +1103,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
 
                     cleanupSecondaryStorage(recurring);
 
-                    List<VolumeVO> vols = _volsDao.listVolumesToBeDestroyed();
+                    List<VolumeVO> vols = _volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) StorageCleanupDelay.value() << 10)));
                     for (VolumeVO vol : vols) {
                         try {
                             volService.expungeVolumeAsync(volFactory.getVolume(vol.getId()));
@@ -2304,4 +2298,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         }
         return 0L;
     }
+
+    @Override
+    public String getConfigComponentName() {
+        return StorageManager.class.getSimpleName();
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey<?>[] {StorageCleanupInterval, StorageCleanupDelay, StorageCleanupEnabled};
+    }
 }