bug 11600: aquirelock before deleting a volume, which can be aquired by snapshot manager

status 11600: resolved fixed

agent-simulator/src/com/cloud/agent/manager/MockStorageManager.java

@@ -61,7 +61,7 @@ public interface MockStorageManager extends Manager {
 	public Answer DownloadProcess(DownloadProgressCommand cmd);
 	public GetStorageStatsAnswer GetStorageStats(GetStorageStatsCommand cmd);
 	public Answer ManageSnapshot(ManageSnapshotCommand cmd);
-	public Answer BackupSnapshot(BackupSnapshotCommand cmd);
+	public Answer BackupSnapshot(BackupSnapshotCommand cmd, SimulatorInfo info);
 	public Answer DeleteSnapshotBackup(DeleteSnapshotBackupCommand cmd);
 	public Answer CreateVolumeFromSnapshot(CreateVolumeFromSnapshotCommand cmd);
 	public Answer DeleteTemplate(DeleteTemplateCommand cmd);

agent-simulator/src/com/cloud/agent/manager/MockStorageManagerImpl.java

@@ -295,11 +295,14 @@ public class MockStorageManagerImpl implements MockStorageManager {
         if (volume != null) {
             _mockVolumeDao.remove(volume.getId());
         }
-        MockVm vm = _mockVMDao.findByVmName(cmd.getVmName());
+        
-        vm.setState(State.Expunging);
+        if (cmd.getVmName() != null) {
-        if (vm != null ) {
+        	MockVm vm = _mockVMDao.findByVmName(cmd.getVmName());
-        	MockVMVO vmVo = _mockVMDao.createForUpdate(vm.getId());
+        	vm.setState(State.Expunging);
-        	_mockVMDao.update(vm.getId(), vmVo);
+        	if (vm != null ) {
+        		MockVMVO vmVo = _mockVMDao.createForUpdate(vm.getId());
+        		_mockVMDao.update(vm.getId(), vmVo);
+        	}
         }
         return new Answer(cmd);
     }
@@ -399,7 +402,12 @@ public class MockStorageManagerImpl implements MockStorageManager {
     }
 
     @Override
-    public BackupSnapshotAnswer BackupSnapshot(BackupSnapshotCommand cmd) {
+    public BackupSnapshotAnswer BackupSnapshot(BackupSnapshotCommand cmd, SimulatorInfo info) {
+    	//emulate xenserver backupsnapshot, if the base volume is deleted, then backupsnapshot failed
+    	MockVolumeVO volume = _mockVolumeDao.findByStoragePathAndType(cmd.getVolumePath());
+    	if (volume == null) {
+    		return new BackupSnapshotAnswer(cmd, false, "Can't find base volume: " + cmd.getVolumePath(), null, true);
+    	}
         String snapshotPath = cmd.getSnapshotUuid();
         MockVolumeVO snapshot = _mockVolumeDao.findByStoragePathAndType(snapshotPath);
         if (snapshot == null) {

agent-simulator/src/com/cloud/agent/manager/MockVmManager.java

@@ -49,13 +49,11 @@ public interface MockVmManager extends Manager {
     public Answer stopVM(StopCommand cmd);
 	public Answer rebootVM(RebootCommand cmd);
     
-    public boolean migrate(String vmName, String params);
-    
     public Answer checkVmState(CheckVirtualMachineCommand cmd, String hostGuid);
     public Map<String, State> getVmStates(String hostGuid);
     public Answer getVncPort(GetVncPortCommand cmd);
 
-	Answer startVM(StartCommand cmd, String hostGuid);
+	Answer startVM(StartCommand cmd, SimulatorInfo info);
 
 	Answer getVmStats(GetVmStatsCommand cmd);
     public CheckSshAnswer checkSshCommand(CheckSshCommand cmd);
@@ -66,8 +64,6 @@ public interface MockVmManager extends Manager {
     
     Answer getNetworkUsage(NetworkUsageCommand cmd);
     
-    Answer Migrate(MigrateCommand cmd, String hostGuid);
-    
     Answer IpAssoc(IpAssocCommand cmd);
 
     Answer LoadBalancerConfig(LoadBalancerConfigCommand cmd);
@@ -75,13 +71,14 @@ public interface MockVmManager extends Manager {
     Answer AddDhcpEntry(DhcpEntryCommand cmd);
     
     Answer setVmData(VmDataCommand cmd);
-    Answer CleanupNetworkRules(CleanupNetworkRulesCmd cmd, String hostGuid);
+    Answer CleanupNetworkRules(CleanupNetworkRulesCmd cmd, SimulatorInfo info);
     
     Answer CheckConsoleProxyLoad(CheckConsoleProxyLoadCommand cmd);
     Answer WatchConsoleProxyLoad(WatchConsoleProxyLoadCommand cmd);
     
     Answer SavePassword(SavePasswordCommand cmd);
-    HashMap<String, Pair<Long, Long>> syncNetworkGroups(String hostGuid);
+    HashMap<String, Pair<Long, Long>> syncNetworkGroups(SimulatorInfo info);
-    SecurityIngressRuleAnswer AddSecurityIngressRules(SecurityIngressRulesCmd cmd, String hostGuid);
+    SecurityIngressRuleAnswer AddSecurityIngressRules(SecurityIngressRulesCmd cmd, SimulatorInfo info);
+	MigrateAnswer Migrate(MigrateCommand cmd, SimulatorInfo info);
     
 }

agent-simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java

@@ -180,18 +180,6 @@ public class MockVmManagerImpl implements MockVmManager {
 	    return true;
 	}
 	
-	@Override
-    public boolean migrate(String vmName, String params) {
-		MockVm vm = _mockVmDao.findByVmName(vmName);
-		if(vm != null) {
-		    vm.setState(State.Stopped);
-		    _mockVmDao.remove(vm.getId());
-		    return true;
-		}
-
-		return false;
-	}
-	
 	@Override
     public Map<String, State> getVmStates(String hostGuid) {
 		Map<String, State> states = new HashMap<String, State>();
@@ -248,9 +236,9 @@ public class MockVmManagerImpl implements MockVmManager {
     }
 
     @Override
-    public Answer startVM(StartCommand cmd, String hostGuid) {
+    public Answer startVM(StartCommand cmd, SimulatorInfo info) {
         VirtualMachineTO vm = cmd.getVirtualMachine();
-        String result = startVM(vm.getName(), vm.getNics(), vm.getCpus()* vm.getSpeed(), vm.getMaxRam(), vm.getBootArgs(), hostGuid);
+        String result = startVM(vm.getName(), vm.getNics(), vm.getCpus()* vm.getSpeed(), vm.getMaxRam(), vm.getBootArgs(), info.getHostUuid());
         if (result != null) {
             return new StartAnswer(cmd, result);
         } else {
@@ -279,17 +267,17 @@ public class MockVmManagerImpl implements MockVmManager {
     }
 
     @Override
-    public MigrateAnswer Migrate(MigrateCommand cmd, String hostGuid) {
+    public MigrateAnswer Migrate(MigrateCommand cmd, SimulatorInfo info) {
         String vmName = cmd.getVmName();
         String destGuid = cmd.getHostGuid();
-        MockVMVO vm = _mockVmDao.findByVmNameAndHost(vmName, hostGuid);
+        MockVMVO vm = _mockVmDao.findByVmNameAndHost(vmName, info.getHostUuid());
         if (vm == null) {
-            return new MigrateAnswer(cmd, false, "can;t find vm:" + vmName + " on host:" + hostGuid, null);
+            return new MigrateAnswer(cmd, false, "can;t find vm:" + vmName + " on host:" + info.getHostUuid(), null);
         }
         
         MockHost destHost = _mockHostDao.findByGuid(destGuid);
         if (destHost == null) {
-            return new MigrateAnswer(cmd, false, "can;t find host:" + hostGuid, null);
+            return new MigrateAnswer(cmd, false, "can;t find host:" + info.getHostUuid(), null);
         }
         vm.setHostId(destHost.getId());
         _mockVmDao.update(vm.getId(), vm);
@@ -317,10 +305,10 @@ public class MockVmManagerImpl implements MockVmManager {
     }
 
     @Override
-    public Answer CleanupNetworkRules(CleanupNetworkRulesCmd cmd, String hostGuid) {
+    public Answer CleanupNetworkRules(CleanupNetworkRulesCmd cmd, SimulatorInfo info) {
-        List<MockSecurityRulesVO> rules = _mockSecurityDao.findByHost(hostGuid);
+        List<MockSecurityRulesVO> rules = _mockSecurityDao.findByHost(info.getHostUuid());
         for (MockSecurityRulesVO rule : rules) {
-            MockVMVO vm = _mockVmDao.findByVmNameAndHost(rule.getVmName(), hostGuid);
+            MockVMVO vm = _mockVmDao.findByVmNameAndHost(rule.getVmName(), info.getHostUuid());
             if (vm == null) {
                 _mockSecurityDao.remove(rule.getId());
             }
@@ -365,15 +353,18 @@ public class MockVmManagerImpl implements MockVmManager {
     }
 
     @Override
-    public SecurityIngressRuleAnswer AddSecurityIngressRules(SecurityIngressRulesCmd cmd, String hostGuid) {
+    public SecurityIngressRuleAnswer AddSecurityIngressRules(SecurityIngressRulesCmd cmd, SimulatorInfo info) {
+        if (!info.isEnabled()) {
+        	return new SecurityIngressRuleAnswer(cmd, false, "Disabled", SecurityIngressRuleAnswer.FailureReason.CANNOT_BRIDGE_FIREWALL);
+        }
         
-        Map<String, Ternary<String,Long, Long>> rules = _securityRules.get(hostGuid);
+        Map<String, Ternary<String,Long, Long>> rules = _securityRules.get(info.getHostUuid());
        
         if (rules == null) {
             logSecurityGroupAction(cmd, null);
             rules = new ConcurrentHashMap<String, Ternary<String, Long, Long>>();
             rules.put(cmd.getVmName(), new Ternary<String,Long, Long>(cmd.getSignature(), cmd.getVmId(), cmd.getSeqNum()));
-            _securityRules.put(hostGuid, rules);
+            _securityRules.put(info.getHostUuid(), rules);
         } else {
             logSecurityGroupAction(cmd, rules.get(cmd.getVmName()));
             rules.put(cmd.getVmName(), new Ternary<String, Long,Long>(cmd.getSignature(), cmd.getVmId(), cmd.getSeqNum()));
@@ -437,10 +428,10 @@ public class MockVmManagerImpl implements MockVmManager {
     }
     
     @Override
-    public HashMap<String, Pair<Long, Long>> syncNetworkGroups(String hostGuid) {
+    public HashMap<String, Pair<Long, Long>> syncNetworkGroups(SimulatorInfo info) {
         HashMap<String, Pair<Long, Long>> maps = new HashMap<String, Pair<Long, Long>>();
         
-        Map<String, Ternary<String, Long, Long>> rules = _securityRules.get(hostGuid);
+        Map<String, Ternary<String, Long, Long>> rules = _securityRules.get(info.getHostUuid());
         if (rules == null) {
             return maps;
         }

agent-simulator/src/com/cloud/agent/manager/SimulatorInfo.java

@@ -0,0 +1,43 @@
+package com.cloud.agent.manager;
+
+public class SimulatorInfo {
+	private boolean enabled;
+	private int timeout;
+	private String hostUuid;
+	
+	public SimulatorInfo(boolean enabled, int timeout, String hostUuid) {
+		this.enabled = enabled;
+		this.timeout = timeout;
+		this.hostUuid = hostUuid;
+	}
+	
+	public SimulatorInfo() {
+		this.enabled = true;
+		this.timeout = -1;
+		this.hostUuid = null;
+	}
+	
+	public boolean isEnabled() {
+		return this.enabled;
+	}
+	
+	public int getTimeout() {
+		return this.timeout;
+	}
+	
+	public String getHostUuid() {
+		return this.hostUuid;
+	}
+	
+	public void setEnabled(boolean enabled) {
+		this.enabled = enabled;
+	}
+	
+	public void setTimeout(int timeout) {
+		this.timeout = timeout;
+	}
+	
+	public void setHostUuid(String hostUuid) {
+		this.hostUuid = hostUuid;
+	}
+}

agent-simulator/src/com/cloud/agent/manager/SimulatorManagerImpl.java

@@ -143,18 +143,29 @@ public class SimulatorManagerImpl implements SimulatorManager {
             }
             MockConfigurationVO config = _mockConfigDao.findByNameBottomUP(host.getDataCenterId(), host.getPodId(), host.getClusterId(), host.getId(), cmdName);
             
+            SimulatorInfo info = new SimulatorInfo();
+            info.setHostUuid(hostGuid);
+            
             if (config != null) {
                 Map<String, String> configParameters = config.getParameters();
-                if ("false".equalsIgnoreCase(configParameters.get("enabled"))) {
+                for (Map.Entry<String, String> entry : configParameters.entrySet()) {
-                    return new Answer(cmd, false, "cmd is disabled");
+                	if (entry.getKey().equalsIgnoreCase("enabled")) {
-                } else if (configParameters.get("timeout") != null) {
+                		info.setEnabled(Boolean.parseBoolean(entry.getValue()));
-                	try {
+                	} else if (entry.getKey().equalsIgnoreCase("timeout")) {
-                		int timeout = Integer.valueOf(configParameters.get("timeout"));
+                		try {
-                		Thread.sleep(timeout * 1000);
+                			info.setTimeout(Integer.valueOf(entry.getValue()));
-                	} catch (NumberFormatException e) {
+                		} catch (NumberFormatException e) {
-                		s_logger.debug("invalid timeout parameter: " + e.toString());
+                			s_logger.debug("invalid timeout parameter: " + e.toString());
-                	} catch (InterruptedException e) {
+                		}
-                		s_logger.debug("thread is interrupted: " + e.toString());
+                	} else if (entry.getKey().equalsIgnoreCase("wait")) {
+                		try {
+                			int wait = Integer.valueOf(entry.getValue());
+                			Thread.sleep(wait * 1000);
+                		} catch (NumberFormatException e) {
+                			s_logger.debug("invalid timeout parameter: " + e.toString());
+                		} catch (InterruptedException e) {
+                			s_logger.debug("thread is interrupted: " + e.toString());
+                		}
                 	}
                 }
             }
@@ -166,9 +177,9 @@ public class SimulatorManagerImpl implements SimulatorManager {
             } else if (cmd instanceof PingTestCommand) {
                 return _mockAgentMgr.pingTest((PingTestCommand)cmd);
             } else if (cmd instanceof MigrateCommand) {
-                return _mockVmMgr.Migrate((MigrateCommand)cmd, hostGuid);
+                return _mockVmMgr.Migrate((MigrateCommand)cmd, info);
             } else if (cmd instanceof StartCommand) {
-                return _mockVmMgr.startVM((StartCommand)cmd, hostGuid);
+                return _mockVmMgr.startVM((StartCommand)cmd, info);
             } else if (cmd instanceof CheckSshCommand) {
                 return _mockVmMgr.checkSshCommand((CheckSshCommand)cmd);
             } else if (cmd instanceof SetStaticNatRulesCommand) {
@@ -186,7 +197,7 @@ public class SimulatorManagerImpl implements SimulatorManager {
             } else if (cmd instanceof VmDataCommand) {
                 return _mockVmMgr.setVmData((VmDataCommand)cmd);
             } else if (cmd instanceof CleanupNetworkRulesCmd) {
-                return _mockVmMgr.CleanupNetworkRules((CleanupNetworkRulesCmd)cmd, hostGuid);
+                return _mockVmMgr.CleanupNetworkRules((CleanupNetworkRulesCmd)cmd, info);
             } else if (cmd instanceof StopCommand) {
                 return _mockVmMgr.stopVM((StopCommand)cmd);
             } else if (cmd instanceof RebootCommand) {
@@ -198,7 +209,7 @@ public class SimulatorManagerImpl implements SimulatorManager {
             } else if (cmd instanceof WatchConsoleProxyLoadCommand) {
                 return _mockVmMgr.WatchConsoleProxyLoad((WatchConsoleProxyLoadCommand)cmd);
             } else if (cmd instanceof SecurityIngressRulesCmd) {
-                return _mockVmMgr.AddSecurityIngressRules((SecurityIngressRulesCmd)cmd, hostGuid);
+                return _mockVmMgr.AddSecurityIngressRules((SecurityIngressRulesCmd)cmd, info);
             } else if (cmd instanceof SavePasswordCommand) {
                 return _mockVmMgr.SavePassword((SavePasswordCommand)cmd);
             } else if (cmd instanceof PrimaryStorageDownloadCommand) {
@@ -230,7 +241,7 @@ public class SimulatorManagerImpl implements SimulatorManager {
             } else if (cmd instanceof ManageSnapshotCommand) {
                 return _mockStorageMgr.ManageSnapshot((ManageSnapshotCommand)cmd);
             } else if (cmd instanceof BackupSnapshotCommand) {
-                return _mockStorageMgr.BackupSnapshot((BackupSnapshotCommand)cmd);
+                return _mockStorageMgr.BackupSnapshot((BackupSnapshotCommand)cmd, info);
             } else if (cmd instanceof DeleteSnapshotBackupCommand) {
                 return _mockStorageMgr.DeleteSnapshotBackup((DeleteSnapshotBackupCommand)cmd);
             } else if (cmd instanceof CreateVolumeFromSnapshotCommand) {
@@ -299,7 +310,9 @@ public class SimulatorManagerImpl implements SimulatorManager {
 
     @Override
     public HashMap<String, Pair<Long, Long>> syncNetworkGroups(String hostGuid) {
-        return _mockVmMgr.syncNetworkGroups(hostGuid);
+    	SimulatorInfo info = new SimulatorInfo();
+    	info.setHostUuid(hostGuid);
+        return _mockVmMgr.syncNetworkGroups(info);
     }
 
 }

agent-simulator/src/com/cloud/api/commands/ConfigureSimulator.java

@@ -8,8 +8,6 @@ import com.cloud.api.BaseCmd;
 import com.cloud.api.Implementation;
 import com.cloud.api.Parameter;
 import com.cloud.api.ServerApiException;
-import com.cloud.api.repsonse.ConfigureSimulatorResponse;
-import com.cloud.api.response.BaseResponse;
 import com.cloud.api.response.SuccessResponse;
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.InsufficientCapacityException;
@@ -20,8 +18,8 @@ import com.cloud.user.Account;
 import com.cloud.utils.component.ComponentLocator;
 
 @Implementation(description="configure simulator", responseObject=SuccessResponse.class)
-public class ConfigureSimulatorCmd extends BaseCmd {
+public class ConfigureSimulator extends BaseCmd {
-    public static final Logger s_logger = Logger.getLogger(ConfigureSimulatorCmd.class.getName());
+    public static final Logger s_logger = Logger.getLogger(ConfigureSimulator.class.getName());
     private static final String s_name = "configuresimulatorresponse";
     
     @Parameter(name=ApiConstants.ZONE_ID, type=CommandType.LONG, description="configure range: in a zone")

agent-simulator/tomcatconf/commands-simulator.properties.in

@@ -1 +1 @@
-configureSimulatorCmd=com.cloud.api.commands.ConfigureSimulatorCmd;1
+configureSimulator=com.cloud.api.commands.ConfigureSimulator;1

build/developer.xml

@@ -379,11 +379,12 @@
   <target name="build-apidocs" description="Generate api documentation">
       <property name="commands.file" location="${tomcat.home}/conf/commands.properties" /> 
 	  <property name="commands.ext.file" location="${tomcat.home}/conf/commands-ext.properties" />
+	  	<property name="commands.simulator.file" location="${tomcat.home}/conf/commands-simulator.properties" />
   
 	<echo message="build-apidocs" />
     <exec dir="${apidoc.scripts.dir}" executable="bash">
       <arg value="build-apidoc.sh" />
-      <arg value="-f ${commands.file},${commands.ext.file}" />
+      <arg value="-f ${commands.file},${commands.ext.file},${commands.simulator.file}" />
       <arg value="-d ${dist.dir}" />
       <env key="CATALINA_HOME" value="${tomcat.home}"/>
     </exec>

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

@@ -2428,6 +2428,7 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag
     }
 
     @Override
+    @DB
     @ActionEvent(eventType = EventTypes.EVENT_VOLUME_DELETE, eventDescription = "deleting volume")
     public boolean deleteVolume(DeleteVolumeCmd cmd) throws ConcurrentOperationException {
         Account account = UserContext.current().getCaller();
@@ -2443,54 +2444,59 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag
         }
 
         // Check that the volume ID is valid
-        VolumeVO volume = _volsDao.findById(volumeId);
+        VolumeVO volume = _volsDao.acquireInLockTable(volumeId, 10);
         if (volume == null) {
-            throw new InvalidParameterValueException("Unable to find volume with ID: " + volumeId);
+            throw new InvalidParameterValueException("Unable to aquire volume with ID: " + volumeId);
-        }
-
-        // If the account is not an admin, check that the volume is owned by the account that was passed in
-        if (!isAdmin) {
-            if (account.getId() != volume.getAccountId()) {
-                throw new InvalidParameterValueException("Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName());
-            }
-        } else if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), volume.getDomainId())) {
-            throw new PermissionDeniedException("Unable to delete volume with id " + volumeId + ", permission denied.");
-        }
-
-        // If the account is not an admin, check that the volume is owned by the account that was passed in
-        if (!isAdmin) {
-            if (account.getId() != volume.getAccountId()) {
-                throw new InvalidParameterValueException("Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName());
-            }
-        } else if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), volume.getDomainId())) {
-            throw new PermissionDeniedException("Unable to delete volume with id " + volumeId + ", permission denied.");
-        }
-
-        // Check that the volume is stored on shared storage
-        // NOTE: We used to ensure the volume is on shared storage before deleting. However, this seems like an unnecessary
-        // check since all we allow
-        // is deleting a detached volume. Is there a technical reason why the volume has to be on shared storage? If so,
-        // uncomment this...otherwise,
-        // just delete the detached volume regardless of storage pool.
-        // if (!volumeOnSharedStoragePool(volume)) {
-        // throw new InvalidParameterValueException("Please specify a volume that has been created on a shared storage pool.");
-        // }
-
-        // Check that the volume is not currently attached to any VM
-        if (volume.getInstanceId() != null) {
-            throw new InvalidParameterValueException("Please specify a volume that is not attached to any VM.");
-        }
-
-        // Check that the volume is not already destroyed
-        if (volume.getState() != Volume.State.Destroy) {
-            destroyVolume(volume);
         }
 
         try {
-            expungeVolume(volume);
+
-        } catch (Exception e) {
+        	// If the account is not an admin, check that the volume is owned by the account that was passed in
-            s_logger.warn("Failed to expunge volume:", e);
+        	if (!isAdmin) {
-            return false;
+        		if (account.getId() != volume.getAccountId()) {
+        			throw new InvalidParameterValueException("Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName());
+        		}
+        	} else if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), volume.getDomainId())) {
+        		throw new PermissionDeniedException("Unable to delete volume with id " + volumeId + ", permission denied.");
+        	}
+
+        	// If the account is not an admin, check that the volume is owned by the account that was passed in
+        	if (!isAdmin) {
+        		if (account.getId() != volume.getAccountId()) {
+        			throw new InvalidParameterValueException("Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName());
+        		}
+        	} else if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), volume.getDomainId())) {
+        		throw new PermissionDeniedException("Unable to delete volume with id " + volumeId + ", permission denied.");
+        	}
+
+        	// Check that the volume is stored on shared storage
+        	// NOTE: We used to ensure the volume is on shared storage before deleting. However, this seems like an unnecessary
+        	// check since all we allow
+        	// is deleting a detached volume. Is there a technical reason why the volume has to be on shared storage? If so,
+        	// uncomment this...otherwise,
+        	// just delete the detached volume regardless of storage pool.
+        	// if (!volumeOnSharedStoragePool(volume)) {
+        	// throw new InvalidParameterValueException("Please specify a volume that has been created on a shared storage pool.");
+        	// }
+
+        	// Check that the volume is not currently attached to any VM
+        	if (volume.getInstanceId() != null) {
+        		throw new InvalidParameterValueException("Please specify a volume that is not attached to any VM.");
+        	}
+
+        	// Check that the volume is not already destroyed
+        	if (volume.getState() != Volume.State.Destroy) {
+        		destroyVolume(volume);
+        	}
+
+        	try {
+        		expungeVolume(volume);
+        	} catch (Exception e) {
+        		s_logger.warn("Failed to expunge volume:", e);
+        		return false;
+        	}
+        } finally {
+        	_volumeDao.releaseFromLockTable(volumeId);
         }
 
         return true;

server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -450,8 +450,10 @@ public class SnapshotManagerImpl implements SnapshotManager, SnapshotService, Ma
                 }
             } else {
             	snapshot = _snapshotDao.findById(snapshotId);
-                snapshot.setStatus(Status.Error);
+            	if (snapshot != null) {
-                _snapshotDao.update(snapshotId, snapshot);
+            		snapshot.setStatus(Status.Error);
+            		_snapshotDao.update(snapshotId, snapshot);
+            	}
             }
             
             if ( volume != null ) {

utils/src/com/cloud/utils/db/Merovingian2.java

@@ -429,7 +429,7 @@ public class Merovingian2 extends StandardMBean implements MerovingianMBean {
             pstmt.setString(2, threadName);
             pstmt.setInt(3, threadId);
             int rows = pstmt.executeUpdate();
-            assert (false) : "Abandon hope, all ye who enter here....There were still " + rows + ":" + c + " locks not released when the transaction ended!";
+            assert (false) : "Abandon hope, all ye who enter here....There were still " + rows + ":" + c + " locks not released when the transaction ended, check for lock not released or @DB is not added to the code that using the locks!";
         } catch (SQLException e) {
             throw new CloudRuntimeException("Can't clear locks " + pstmt, e);
         } finally {