diff --git a/api/src/com/cloud/storage/Volume.java b/api/src/com/cloud/storage/Volume.java index b84622f5c53..17d57d68175 100644 --- a/api/src/com/cloud/storage/Volume.java +++ b/api/src/com/cloud/storage/Volume.java @@ -72,6 +72,8 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba static { s_fsm.addTransition(new StateMachine2.Transition(Allocated, Event.CreateRequested, Creating, null)); s_fsm.addTransition(new StateMachine2.Transition(Allocated, Event.DestroyRequested, Destroy, null)); + s_fsm.addTransition(new StateMachine2.Transition(Allocated, Event.OperationFailed, Allocated, null)); + s_fsm.addTransition(new StateMachine2.Transition(Allocated, Event.OperationSucceeded, Allocated, null)); s_fsm.addTransition(new StateMachine2.Transition(Creating, Event.OperationRetry, Creating, null)); s_fsm.addTransition(new StateMachine2.Transition(Creating, Event.OperationFailed, Allocated, null)); s_fsm.addTransition(new StateMachine2.Transition(Creating, Event.OperationSucceeded, Ready, null)); diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index d2979f7415d..4abb190dcf2 100644 --- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -217,6 +217,9 @@ public class VolumeServiceImpl implements VolumeService { @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { DataStoreDriver dataStoreDriver = dataStore != null ? dataStore.getDriver() : null; + if (dataStoreDriver == null) { + return; + } if (dataStoreDriver instanceof PrimaryDataStoreDriver) { ((PrimaryDataStoreDriver)dataStoreDriver).revokeAccess(dataObject, host, dataStore); diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java index 33a5fa75f72..085c6995348 100644 --- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java @@ -1888,6 +1888,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } + if (volumePool == null) { + sendCommand = false; + } + Answer answer = null; if (sendCommand) { @@ -1920,14 +1924,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic // Mark the volume as detached _volsDao.detachVolume(volume.getId()); - // volumePool() should be null if the VM we are detaching the disk from has never been started before - // only revoke access on volumes that are actually on a datastore - if (volumePool != null) { + // volume.getPoolId() should be null if the VM we are detaching the disk from has never been started before + if (volume.getPoolId() != null) { DataStore dataStore = dataStoreMgr.getDataStore(volume.getPoolId(), DataStoreRole.Primary); volService.revokeAccess(volFactory.getVolume(volume.getId()), host, dataStore); + } + if (volumePool != null && hostId != null) { handleTargetsForVMware(hostId, volumePool.getHostAddress(), volumePool.getPort(), volume.get_iScsiName()); } - return _volsDao.findById(volumeId); } else { @@ -2641,13 +2645,19 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (volumeToAttach.isAttachedVM()) { throw new CloudRuntimeException("volume: " + volumeToAttach.getName() + " is already attached to a VM: " + volumeToAttach.getAttachedVmName()); } - if (volumeToAttach.getState().equals(Volume.State.Ready)) { - volumeToAttach.stateTransit(Volume.Event.AttachRequested); - } else if (!volumeToAttach.getState().equals(Volume.State.Allocated)) { - final String error = "Volume: " + volumeToAttach.getName() + " is in " + volumeToAttach.getState() + ". It should be in Ready or Allocated state"; - s_logger.error(error); - throw new CloudRuntimeException(error); + + if (Volume.State.Allocated.equals(volumeToAttach.getState())) { + return; } + + if (Volume.State.Ready.equals(volumeToAttach.getState())) { + volumeToAttach.stateTransit(Volume.Event.AttachRequested); + return; + } + + final String error = "Volume: " + volumeToAttach.getName() + " is in " + volumeToAttach.getState() + ". It should be in Ready or Allocated state"; + s_logger.error(error); + throw new CloudRuntimeException(error); } private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, Long deviceId) { @@ -2769,9 +2779,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic volumeToAttach = _volsDao.findById(volumeToAttach.getId()); - if (volumeToAttach.getState().equals(Volume.State.Ready) && - vm.getHypervisorType() == HypervisorType.KVM && - volumeToAttachStoragePool.isManaged() && volumeToAttach.getPath() == null) { + if (vm.getHypervisorType() == HypervisorType.KVM && + volumeToAttachStoragePool != null && volumeToAttachStoragePool.isManaged() && + volumeToAttach.getPath() == null && volumeToAttach.get_iScsiName() != null) { volumeToAttach.setPath(volumeToAttach.get_iScsiName()); _volsDao.update(volumeToAttach.getId(), volumeToAttach); } @@ -2798,19 +2808,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic throw new CloudRuntimeException(errorMsg); } } finally { - final VolumeInfo volInfo = volFactory.getVolume(volumeToAttach.getId()); - // Transit events are only fired when volume was allocated at some point of time. - if (volInfo.getPoolId() != null || volInfo.getLastPoolId() != null) { - final Volume.Event ev; - if (attached) { - ev = Volume.Event.OperationSucceeded; - s_logger.debug("Volume: " + volInfo.getName() + " successfully attached to VM: " + volInfo.getAttachedVmName()); - } else { - ev = Volume.Event.OperationFailed; - s_logger.debug("Volume: " + volInfo.getName() + " failed to attach to VM: " + volInfo.getAttachedVmName()); - } - volInfo.stateTransit(ev); + Volume.Event ev = Volume.Event.OperationFailed; + VolumeInfo volInfo = volFactory.getVolume(volumeToAttach.getId()); + if (attached) { + ev = Volume.Event.OperationSucceeded; + s_logger.debug("Volume: " + volInfo.getName() + " successfully attached to VM: " + volInfo.getAttachedVmName()); + } else { + s_logger.debug("Volume: " + volInfo.getName() + " failed to attach to VM: " + volInfo.getAttachedVmName()); } + volInfo.stateTransit(ev); } return _volsDao.findById(volumeToAttach.getId()); }