From 82fbae2553ca1d534128474d602b7a8043bf28d8 Mon Sep 17 00:00:00 2001 From: abhishek Date: Wed, 25 Aug 2010 13:53:43 -0700 Subject: [PATCH] Refactoring the detach volume command --- .../cloud/api/commands/DetachVolumeCmd.java | 109 +++++++++--------- .../com/cloud/server/ManagementServer.java | 4 +- .../cloud/server/ManagementServerImpl.java | 8 +- server/src/com/cloud/vm/UserVmManager.java | 4 +- .../src/com/cloud/vm/UserVmManagerImpl.java | 73 +++++++++--- 5 files changed, 119 insertions(+), 79 deletions(-) diff --git a/server/src/com/cloud/api/commands/DetachVolumeCmd.java b/server/src/com/cloud/api/commands/DetachVolumeCmd.java index cf420ed0c29..627e6cb7be5 100644 --- a/server/src/com/cloud/api/commands/DetachVolumeCmd.java +++ b/server/src/com/cloud/api/commands/DetachVolumeCmd.java @@ -25,21 +25,19 @@ import java.util.Map; import org.apache.log4j.Logger; import com.cloud.api.BaseCmd; +import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; +import com.cloud.api.BaseCmd.Manager; import com.cloud.storage.VolumeVO; import com.cloud.user.Account; import com.cloud.utils.Pair; +@Implementation(method="detachVolumeFromVM", manager=Manager.UserVmManager) public class DetachVolumeCmd extends BaseCmd { public static final Logger s_logger = Logger.getLogger(DetachVolumeCmd.class.getName()); private static final String s_name = "detachvolumeresponse"; - static { - s_properties.add(new Pair(BaseCmd.Properties.ACCOUNT_OBJ, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.ID, Boolean.TRUE)); - } - ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// @@ -65,59 +63,62 @@ public class DetachVolumeCmd extends BaseCmd { return s_name; } - public List> getProperties() { - return s_properties; - } - public static String getResultObjectName() { return "volume"; } - @Override - public List> execute(Map params) { - Account account = (Account) params.get(BaseCmd.Properties.ACCOUNT_OBJ.getName()); - Long volumeId = (Long) params.get(BaseCmd.Properties.ID.getName()); - - boolean isAdmin; - if (account == null) { - // Admin API call - isAdmin = true; - } else { - // User API call - isAdmin = isAdmin(account.getType()); - } +// @Override +// public List> execute(Map params) { +// Account account = (Account) params.get(BaseCmd.Properties.ACCOUNT_OBJ.getName()); +// Long volumeId = (Long) params.get(BaseCmd.Properties.ID.getName()); +// +// boolean isAdmin; +// if (account == null) { +// // Admin API call +// isAdmin = true; +// } else { +// // User API call +// isAdmin = isAdmin(account.getType()); +// } +// +// // Check that the volume ID is valid +// VolumeVO volume = getManagementServer().findVolumeById(volumeId); +// if (volume == null) +// throw new ServerApiException(BaseCmd.PARAM_ERROR, "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 ServerApiException(BaseCmd.PARAM_ERROR, "Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName()); +// } else if (account != null) { +// if (!getManagementServer().isChildDomain(account.getDomainId(), volume.getDomainId())) { +// throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to detach volume with ID: " + volumeId + ", permission denied."); +// } +// } +// +// try { +// long jobId = getManagementServer().detachVolumeFromVMAsync(volumeId); +// +// if (jobId == 0) { +// s_logger.warn("Unable to schedule async-job for DetachVolume comamnd"); +// } else { +// if(s_logger.isDebugEnabled()) +// s_logger.debug("DetachVolume command has been accepted, job id: " + jobId); +// } +// +// List> returnValues = new ArrayList>(); +// returnValues.add(new Pair(BaseCmd.Properties.JOB_ID.getName(), Long.valueOf(jobId))); +// +// return returnValues; +// } catch (Exception ex) { +// throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to detach volume: " + ex.getMessage()); +// } +// } - // Check that the volume ID is valid - VolumeVO volume = getManagementServer().findVolumeById(volumeId); - if (volume == null) - throw new ServerApiException(BaseCmd.PARAM_ERROR, "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 ServerApiException(BaseCmd.PARAM_ERROR, "Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName()); - } else if (account != null) { - if (!getManagementServer().isChildDomain(account.getDomainId(), volume.getDomainId())) { - throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to detach volume with ID: " + volumeId + ", permission denied."); - } - } - - try { - long jobId = getManagementServer().detachVolumeFromVMAsync(volumeId); - - if (jobId == 0) { - s_logger.warn("Unable to schedule async-job for DetachVolume comamnd"); - } else { - if(s_logger.isDebugEnabled()) - s_logger.debug("DetachVolume command has been accepted, job id: " + jobId); - } - - List> returnValues = new ArrayList>(); - returnValues.add(new Pair(BaseCmd.Properties.JOB_ID.getName(), Long.valueOf(jobId))); - - return returnValues; - } catch (Exception ex) { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to detach volume: " + ex.getMessage()); - } - } + @Override + public String getResponse() { + // TODO Auto-generated method stub + return null; + } } \ No newline at end of file diff --git a/server/src/com/cloud/server/ManagementServer.java b/server/src/com/cloud/server/ManagementServer.java index ef1d64a35a5..2ac51949b0a 100644 --- a/server/src/com/cloud/server/ManagementServer.java +++ b/server/src/com/cloud/server/ManagementServer.java @@ -633,8 +633,8 @@ public interface ManagementServer { * @volumeId * @throws InvalidParameterValueException, InternalErrorException */ - void detachVolumeFromVM(long volumeId, long startEventId) throws InternalErrorException; - long detachVolumeFromVMAsync(long volumeId) throws InvalidParameterValueException; +// void detachVolumeFromVM(long volumeId, long startEventId) throws InternalErrorException; +// long detachVolumeFromVMAsync(long volumeId) throws InvalidParameterValueException; /** * Attaches an ISO to the virtual CDROM device of the specified VM. Will fail if the VM already has an ISO mounted. diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index c037c65a3f1..4d9e75b6c87 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -2091,10 +2091,10 @@ public class ManagementServerImpl implements ManagementServer { return _asyncMgr.submitAsyncJob(job); } - @Override - public void detachVolumeFromVM(long volumeId, long startEventId) throws InternalErrorException { - _vmMgr.detachVolumeFromVM(volumeId, startEventId); - } +// @Override +// public void detachVolumeFromVM(long volumeId, long startEventId) throws InternalErrorException { +// _vmMgr.detachVolumeFromVM(volumeId, startEventId); +// } @Override public long detachVolumeFromVMAsync(long volumeId) throws InvalidParameterValueException { diff --git a/server/src/com/cloud/vm/UserVmManager.java b/server/src/com/cloud/vm/UserVmManager.java index e63624c1722..0ae15dfdadd 100644 --- a/server/src/com/cloud/vm/UserVmManager.java +++ b/server/src/com/cloud/vm/UserVmManager.java @@ -22,6 +22,7 @@ import java.util.List; import com.cloud.agent.api.VmStatsEntry; import com.cloud.api.ServerApiException; +import com.cloud.api.commands.DetachVolumeCmd; import com.cloud.api.commands.RebootVMCmd; import com.cloud.api.commands.RecoverVMCmd; import com.cloud.api.commands.ResetVMPasswordCmd; @@ -124,8 +125,9 @@ public interface UserVmManager extends Manager, VirtualMachineManager * Detaches the specified volume from the VM it is currently attached to. * @param volumeId * @throws InternalErrorException + * @throws InvalidParameterValueException */ - void detachVolumeFromVM(long volumeId, long startEventId) throws InternalErrorException; + void detachVolumeFromVM(DetachVolumeCmd cmmd) throws InternalErrorException, InvalidParameterValueException; /** * Attaches an ISO to the virtual CDROM device of the specified VM. Will eject any existing virtual CDROM if isoPath is null. diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index fd02cb56409..54d853972e8 100644 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -66,6 +66,7 @@ import com.cloud.agent.manager.AgentManager; import com.cloud.alert.AlertManager; import com.cloud.api.BaseCmd; import com.cloud.api.ServerApiException; +import com.cloud.api.commands.DetachVolumeCmd; import com.cloud.api.commands.RebootVMCmd; import com.cloud.api.commands.RecoverVMCmd; import com.cloud.api.commands.ResetVMPasswordCmd; @@ -87,6 +88,8 @@ import com.cloud.async.executor.UpgradeVMParam; import com.cloud.async.executor.VMExecutorHelper; import com.cloud.async.executor.VMOperationListener; import com.cloud.async.executor.VMOperationParam; +import com.cloud.async.executor.VolumeOperationParam; +import com.cloud.async.executor.VolumeOperationParam.VolumeOp; import com.cloud.capacity.dao.CapacityDao; import com.cloud.configuration.ConfigurationManager; import com.cloud.configuration.ResourceCount.ResourceType; @@ -483,25 +486,59 @@ public class UserVmManagerImpl implements UserVmManager { } @Override - public void detachVolumeFromVM(long volumeId, long startEventId) throws InternalErrorException { - VolumeVO volume = _volsDao.findById(volumeId); + public void detachVolumeFromVM(DetachVolumeCmd cmmd) throws InternalErrorException, InvalidParameterValueException { - Long vmId = volume.getInstanceId(); + Account account = (Account) UserContext.current().getAccountObject(); + Long volumeId = cmmd.getId(); - if (vmId == null) { - return; + boolean isAdmin; + if (account == null) { + // Admin API call + isAdmin = true; + } else { + // User API call + isAdmin = isAdmin(account.getType()); } - - EventVO event = new EventVO(); - event.setType(EventTypes.EVENT_VOLUME_DETACH); - event.setUserId(1L); - event.setAccountId(volume.getAccountId()); - event.setState(EventState.Started); - event.setStartId(startEventId); - event.setDescription("Detaching volume: "+volumeId+" from Vm: "+vmId); - _eventDao.persist(event); - - UserVmVO vm = _vmDao.findById(vmId); + + // Check that the volume ID is valid + VolumeVO volume = _volsDao.findById(volumeId); + if (volume == null) + throw new ServerApiException(BaseCmd.PARAM_ERROR, "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 ServerApiException(BaseCmd.PARAM_ERROR, "Unable to find volume with ID: " + volumeId + " for account: " + account.getAccountName()); + } else if (account != null) { + if (!_domainDao.isChildDomain(account.getDomainId(), volume.getDomainId())) { + throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to detach volume with ID: " + volumeId + ", permission denied."); + } + } + + // Check that the volume is a data volume + if (volume.getVolumeType() != VolumeType.DATADISK) { + throw new InvalidParameterValueException("Please specify a data volume."); + } + + // Check that the volume is stored on shared storage + if (!_storageMgr.volumeOnSharedStoragePool(volume)) { + throw new InvalidParameterValueException("Please specify a volume that has been created on a shared storage pool."); + } + + Long vmId = volume.getInstanceId(); + + // Check that the volume is currently attached to a VM + if (vmId == null) { + throw new InvalidParameterValueException("The specified volume is not attached to a VM."); + } + + // Check that the VM is in the correct state + UserVmVO vm = _vmDao.findById(vmId); + if (vm.getState() != State.Running && vm.getState() != State.Stopped) { + throw new InvalidParameterValueException("Please specify a VM that is either running or stopped."); + } + + long eventId = EventUtils.saveScheduledEvent(1L, volume.getAccountId(), EventTypes.EVENT_VOLUME_DETACH, "detaching volume: "+volumeId+" from Vm: "+vmId); AsyncJobExecutor asyncExecutor = BaseAsyncJobExecutor.getCurrentExecutor(); if(asyncExecutor != null) { @@ -528,12 +565,12 @@ public class UserVmManagerImpl implements UserVmManager { } } - event = new EventVO(); + EventVO event = new EventVO(); event.setAccountId(volume.getAccountId()); event.setUserId(1L); event.setType(EventTypes.EVENT_VOLUME_DETACH); event.setState(EventState.Completed); - event.setStartId(startEventId); + event.setStartId(eventId); if (!sendCommand || (answer != null && answer.getResult())) { // Mark the volume as detached _volsDao.detachVolume(volume.getId());