From ae83246acb05baedad91015f5d5555e9360d4577 Mon Sep 17 00:00:00 2001 From: Edison Su Date: Mon, 3 Oct 2011 16:39:31 -0700 Subject: [PATCH] bug 11600: aquirelock before deleting a volume, which can be aquired by snapshot manager status 11600: resolved fixed --- .../agent/manager/MockStorageManager.java | 2 +- .../agent/manager/MockStorageManagerImpl.java | 20 ++-- .../cloud/agent/manager/MockVmManager.java | 13 +-- .../agent/manager/MockVmManagerImpl.java | 43 ++++----- .../cloud/agent/manager/SimulatorInfo.java | 43 +++++++++ .../agent/manager/SimulatorManagerImpl.java | 45 +++++---- ...ulatorCmd.java => ConfigureSimulator.java} | 6 +- .../commands-simulator.properties.in | 2 +- build/developer.xml | 3 +- .../com/cloud/storage/StorageManagerImpl.java | 94 ++++++++++--------- .../storage/snapshot/SnapshotManagerImpl.java | 6 +- .../src/com/cloud/utils/db/Merovingian2.java | 2 +- 12 files changed, 169 insertions(+), 110 deletions(-) create mode 100644 agent-simulator/src/com/cloud/agent/manager/SimulatorInfo.java rename agent-simulator/src/com/cloud/api/commands/{ConfigureSimulatorCmd.java => ConfigureSimulator.java} (93%) diff --git a/agent-simulator/src/com/cloud/agent/manager/MockStorageManager.java b/agent-simulator/src/com/cloud/agent/manager/MockStorageManager.java index 345c51e2f51..0fa9f7b74d7 100644 --- a/agent-simulator/src/com/cloud/agent/manager/MockStorageManager.java +++ b/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); diff --git a/agent-simulator/src/com/cloud/agent/manager/MockStorageManagerImpl.java b/agent-simulator/src/com/cloud/agent/manager/MockStorageManagerImpl.java index f13369ffd6e..a443406a137 100644 --- a/agent-simulator/src/com/cloud/agent/manager/MockStorageManagerImpl.java +++ b/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 (vm != null ) { - MockVMVO vmVo = _mockVMDao.createForUpdate(vm.getId()); - _mockVMDao.update(vm.getId(), vmVo); + + if (cmd.getVmName() != null) { + MockVm vm = _mockVMDao.findByVmName(cmd.getVmName()); + vm.setState(State.Expunging); + 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) { diff --git a/agent-simulator/src/com/cloud/agent/manager/MockVmManager.java b/agent-simulator/src/com/cloud/agent/manager/MockVmManager.java index 65a54a20b9c..474a0a97b23 100644 --- a/agent-simulator/src/com/cloud/agent/manager/MockVmManager.java +++ b/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 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> syncNetworkGroups(String hostGuid); - SecurityIngressRuleAnswer AddSecurityIngressRules(SecurityIngressRulesCmd cmd, String hostGuid); + HashMap> syncNetworkGroups(SimulatorInfo info); + SecurityIngressRuleAnswer AddSecurityIngressRules(SecurityIngressRulesCmd cmd, SimulatorInfo info); + MigrateAnswer Migrate(MigrateCommand cmd, SimulatorInfo info); } diff --git a/agent-simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java b/agent-simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java index cff8b72277a..fa46b1536d3 100644 --- a/agent-simulator/src/com/cloud/agent/manager/MockVmManagerImpl.java +++ b/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 getVmStates(String hostGuid) { Map states = new HashMap(); @@ -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) { - List rules = _mockSecurityDao.findByHost(hostGuid); + public Answer CleanupNetworkRules(CleanupNetworkRulesCmd cmd, SimulatorInfo info) { + List 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> rules = _securityRules.get(hostGuid); + Map> rules = _securityRules.get(info.getHostUuid()); if (rules == null) { logSecurityGroupAction(cmd, null); rules = new ConcurrentHashMap>(); rules.put(cmd.getVmName(), new Ternary(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(cmd.getSignature(), cmd.getVmId(), cmd.getSeqNum())); @@ -437,10 +428,10 @@ public class MockVmManagerImpl implements MockVmManager { } @Override - public HashMap> syncNetworkGroups(String hostGuid) { + public HashMap> syncNetworkGroups(SimulatorInfo info) { HashMap> maps = new HashMap>(); - Map> rules = _securityRules.get(hostGuid); + Map> rules = _securityRules.get(info.getHostUuid()); if (rules == null) { return maps; } diff --git a/agent-simulator/src/com/cloud/agent/manager/SimulatorInfo.java b/agent-simulator/src/com/cloud/agent/manager/SimulatorInfo.java new file mode 100644 index 00000000000..9ae9c5b6e92 --- /dev/null +++ b/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; + } +} diff --git a/agent-simulator/src/com/cloud/agent/manager/SimulatorManagerImpl.java b/agent-simulator/src/com/cloud/agent/manager/SimulatorManagerImpl.java index cd7cca7d143..5e45031df39 100644 --- a/agent-simulator/src/com/cloud/agent/manager/SimulatorManagerImpl.java +++ b/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 configParameters = config.getParameters(); - if ("false".equalsIgnoreCase(configParameters.get("enabled"))) { - return new Answer(cmd, false, "cmd is disabled"); - } else if (configParameters.get("timeout") != null) { - try { - int timeout = Integer.valueOf(configParameters.get("timeout")); - Thread.sleep(timeout * 1000); - } catch (NumberFormatException e) { - s_logger.debug("invalid timeout parameter: " + e.toString()); - } catch (InterruptedException e) { - s_logger.debug("thread is interrupted: " + e.toString()); + for (Map.Entry entry : configParameters.entrySet()) { + if (entry.getKey().equalsIgnoreCase("enabled")) { + info.setEnabled(Boolean.parseBoolean(entry.getValue())); + } else if (entry.getKey().equalsIgnoreCase("timeout")) { + try { + info.setTimeout(Integer.valueOf(entry.getValue())); + } catch (NumberFormatException e) { + s_logger.debug("invalid timeout parameter: " + 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> syncNetworkGroups(String hostGuid) { - return _mockVmMgr.syncNetworkGroups(hostGuid); + SimulatorInfo info = new SimulatorInfo(); + info.setHostUuid(hostGuid); + return _mockVmMgr.syncNetworkGroups(info); } } diff --git a/agent-simulator/src/com/cloud/api/commands/ConfigureSimulatorCmd.java b/agent-simulator/src/com/cloud/api/commands/ConfigureSimulator.java similarity index 93% rename from agent-simulator/src/com/cloud/api/commands/ConfigureSimulatorCmd.java rename to agent-simulator/src/com/cloud/api/commands/ConfigureSimulator.java index 3c4ad8653f3..226f95f036c 100644 --- a/agent-simulator/src/com/cloud/api/commands/ConfigureSimulatorCmd.java +++ b/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 static final Logger s_logger = Logger.getLogger(ConfigureSimulatorCmd.class.getName()); +public class ConfigureSimulator extends BaseCmd { + 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") diff --git a/agent-simulator/tomcatconf/commands-simulator.properties.in b/agent-simulator/tomcatconf/commands-simulator.properties.in index 862b60dbbc6..cfc5feefd64 100644 --- a/agent-simulator/tomcatconf/commands-simulator.properties.in +++ b/agent-simulator/tomcatconf/commands-simulator.properties.in @@ -1 +1 @@ -configureSimulatorCmd=com.cloud.api.commands.ConfigureSimulatorCmd;1 \ No newline at end of file +configureSimulator=com.cloud.api.commands.ConfigureSimulator;1 diff --git a/build/developer.xml b/build/developer.xml index c616a01f9d8..51e8a273d7b 100755 --- a/build/developer.xml +++ b/build/developer.xml @@ -379,11 +379,12 @@ + - + diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 8c8a0435464..fab6a909e48 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/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); - } - - // 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); + throw new InvalidParameterValueException("Unable to aquire volume with ID: " + volumeId); } try { - expungeVolume(volume); - } catch (Exception e) { - s_logger.warn("Failed to expunge volume:", e); - return false; + + // 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) { + s_logger.warn("Failed to expunge volume:", e); + return false; + } + } finally { + _volumeDao.releaseFromLockTable(volumeId); } return true; diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index 3ca022f9dce..f2bfe4591ec 100755 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/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); - _snapshotDao.update(snapshotId, snapshot); + if (snapshot != null) { + snapshot.setStatus(Status.Error); + _snapshotDao.update(snapshotId, snapshot); + } } if ( volume != null ) { diff --git a/utils/src/com/cloud/utils/db/Merovingian2.java b/utils/src/com/cloud/utils/db/Merovingian2.java index 5d12909e494..3ec27e39895 100644 --- a/utils/src/com/cloud/utils/db/Merovingian2.java +++ b/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 {