CLOUDSTACK-3716: State of expunged volumes are not consistent in volumes

table and volume_store_ref.

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

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeDataFactory.java

@@ -18,10 +18,14 @@
  */
 package org.apache.cloudstack.engine.subsystem.api.storage;
 
+import com.cloud.storage.DataStoreRole;
+
 public interface VolumeDataFactory {
     VolumeInfo getVolume(long volumeId, DataStore store);
 
     VolumeInfo getVolume(DataObject volume, DataStore store);
 
+    VolumeInfo getVolume(long volumeId, DataStoreRole storeRole);
+
     VolumeInfo getVolume(long volumeId);
 }

engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java

@@ -51,6 +51,26 @@ public class VolumeDataFactoryImpl implements VolumeDataFactory {
         return vol;
     }
 
+    @Override
+    public VolumeInfo getVolume(long volumeId, DataStoreRole storeRole) {
+        VolumeVO volumeVO = volumeDao.findById(volumeId);
+        VolumeObject vol = null;
+        if (storeRole == DataStoreRole.Image) {
+            VolumeDataStoreVO volumeStore = volumeStoreDao.findByVolume(volumeId);
+            if (volumeStore != null) {
+                DataStore store = this.storeMgr.getDataStore(volumeStore.getDataStoreId(), DataStoreRole.Image);
+                vol = VolumeObject.getVolumeObject(store, volumeVO);
+            }
+        } else {
+            // Primary data store
+            if (volumeVO.getPoolId() != null) {
+                DataStore store = this.storeMgr.getDataStore(volumeVO.getPoolId(), DataStoreRole.Primary);
+                vol = VolumeObject.getVolumeObject(store, volumeVO);
+            }
+        }
+        return vol;
+    }
+
     @Override
     public VolumeInfo getVolume(long volumeId) {
         VolumeVO volumeVO = volumeDao.findById(volumeId);

engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java

@@ -254,9 +254,8 @@ public class VolumeObject implements VolumeInfo {
             }
             if (this.dataStore.getRole() == DataStoreRole.Image) {
                 objectInStoreMgr.update(this, event);
-                if (this.volumeVO.getState() == Volume.State.Migrating
+                if (this.volumeVO.getState() == Volume.State.Migrating || this.volumeVO.getState() == Volume.State.Copying || this.volumeVO.getState() == Volume.State.Uploaded
-                        || this.volumeVO.getState() == Volume.State.Copying
+                        || this.volumeVO.getState() == Volume.State.Expunged) {
-                        || this.volumeVO.getState() == Volume.State.Uploaded) {
                     return;
                 }
                 if (event == ObjectInDataStoreStateMachine.Event.CreateOnlyRequested) {
@@ -513,6 +512,7 @@ public class VolumeObject implements VolumeInfo {
 
     }
 
+    @Override
     public void incRefCount() {
         if (this.dataStore == null) {
             return;

engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java

@@ -75,6 +75,7 @@ import com.cloud.storage.StoragePool;
 import com.cloud.storage.VMTemplateStoragePoolVO;
 import com.cloud.storage.VMTemplateStorageResourceAssoc;
 import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
+import com.cloud.storage.Volume.State;
 import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeVO;
 import com.cloud.storage.dao.VMTemplatePoolDao;
@@ -210,13 +211,34 @@ public class VolumeServiceImpl implements VolumeService {
         }
     }
 
+    // check if a volume is expunged on both primary and secondary
+    private boolean canVolumeBeRemoved(long volumeId) {
+        VolumeVO vol = volDao.findById(volumeId);
+        if (vol == null) {
+            // already removed from volumes table
+            return false;
+        }
+        VolumeDataStoreVO volumeStore = _volumeStoreDao.findByVolume(volumeId);
+        if (vol.getState() == State.Expunged && volumeStore == null) {
+            // volume is expunged from primary, as well as on secondary
+            return true;
+        } else {
+            return false;
+        }
+
+    }
+
     @DB
     @Override
     public AsyncCallFuture<VolumeApiResult> expungeVolumeAsync(VolumeInfo volume) {
         AsyncCallFuture<VolumeApiResult> future = new AsyncCallFuture<VolumeApiResult>();
         VolumeApiResult result = new VolumeApiResult(volume);
         if (volume.getDataStore() == null) {
+            s_logger.info("Expunge volume with no data store specified");
+            if (canVolumeBeRemoved(volume.getId())) {
+                s_logger.info("Volume " + volume.getId() + " is not referred anywhere, remove it from volumes table");
                 volDao.remove(volume.getId());
+            }
             future.complete(result);
             return future;
         }
@@ -245,7 +267,11 @@ public class VolumeServiceImpl implements VolumeService {
         }
         VolumeObject vo = (VolumeObject) volume;
 
+        if (volume.getDataStore().getRole() == DataStoreRole.Image) {
+            volume.processEvent(Event.DestroyRequested);
+        } else if (volume.getDataStore().getRole() == DataStoreRole.Primary) {
             volume.processEvent(Event.ExpungeRequested);
+        }
 
         DeleteVolumeContext<VolumeApiResult> context = new DeleteVolumeContext<VolumeApiResult>(null, vo, future);
         AsyncCallbackDispatcher<VolumeServiceImpl, CommandResult> caller = AsyncCallbackDispatcher.create(this);
@@ -255,6 +281,7 @@ public class VolumeServiceImpl implements VolumeService {
         return future;
     }
 
+
     public Void deleteVolumeCallback(AsyncCallbackDispatcher<VolumeServiceImpl, CommandResult> callback,
             DeleteVolumeContext<VolumeApiResult> context) {
         CommandResult result = callback.getResult();
@@ -262,7 +289,10 @@ public class VolumeServiceImpl implements VolumeService {
         VolumeApiResult apiResult = new VolumeApiResult(vo);
         if (result.isSuccess()) {
             vo.processEvent(Event.OperationSuccessed);
+            if (canVolumeBeRemoved(vo.getId())) {
+                s_logger.info("Volume " + vo.getId() + " is not referred anywhere, remove it from volumes table");
                 volDao.remove(vo.getId());
+            }
         } else {
             vo.processEvent(Event.OperationFailed);
             apiResult.setResult(result.getResult());

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

@@ -31,6 +31,10 @@ import java.util.concurrent.ExecutionException;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
 import org.apache.cloudstack.api.BaseCmd;
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
@@ -69,9 +73,7 @@ import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
-import org.apache.commons.lang.StringUtils;
+
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.storage.CreateVolumeOVAAnswer;
@@ -1355,9 +1357,20 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
                     _usageEventDao.persist(usageEvent);
                 }
             }
-            AsyncCallFuture<VolumeApiResult> future = volService.expungeVolumeAsync(volFactory.getVolume(volume.getId()));
+            // expunge volume from primary if volume is on primary
+            VolumeInfo volOnPrimary = volFactory.getVolume(volume.getId(), DataStoreRole.Primary);
+            if (volOnPrimary != null) {
+                s_logger.info("Expunging volume " + volume.getId() + " from primary data store");
+                AsyncCallFuture<VolumeApiResult> future = volService.expungeVolumeAsync(volOnPrimary);
                 future.get();
-
+            }
+            // expunge volume from secondary if volume is on image store
+            VolumeInfo volOnSecondary = volFactory.getVolume(volume.getId(), DataStoreRole.Image);
+            if (volOnSecondary != null) {
+                s_logger.info("Expunging volume " + volume.getId() + " from secondary data store");
+                AsyncCallFuture<VolumeApiResult> future2 = volService.expungeVolumeAsync(volOnSecondary);
+                future2.get();
+            }
         } catch (Exception e) {
             s_logger.warn("Failed to expunge volume:", e);
             return false;