cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tall...@apache.org
Subject [26/60] [abbrv] git commit: updated refs/heads/marvin to 0e223d6
Date Tue, 08 Apr 2014 12:25:41 GMT
CLOUDSTACK-3886: Volume attach/detach implementation for ROOT disk
Implemented for Xen hypervisor only by now
Unittests are included


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/3a889aa2
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/3a889aa2
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/3a889aa2

Branch: refs/heads/marvin
Commit: 3a889aa2174d51cf4568c5ae43d72bbf4d4f7503
Parents: af1eba2
Author: Alena Prokharchyk <alena.prokharchyk@citrix.com>
Authored: Mon Mar 31 16:33:13 2014 -0700
Committer: Alena Prokharchyk <alena.prokharchyk@citrix.com>
Committed: Thu Apr 3 11:39:56 2014 -0700

----------------------------------------------------------------------
 .../command/user/volume/AttachVolumeCmd.java    |   2 +-
 .../service/VolumeOrchestrationService.java     |   2 +-
 .../orchestration/VolumeOrchestrator.java       |  17 +-
 .../com/cloud/storage/dao/VolumeDaoImpl.java    |   6 +
 .../deploy/DeploymentPlanningManagerImpl.java   |  53 ++--
 .../com/cloud/storage/VolumeApiServiceImpl.java | 295 +++++++++--------
 .../cloud/storage/VolumeApiServiceImplTest.java | 316 +++++++++++++++++++
 7 files changed, 528 insertions(+), 163 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java b/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
index 53b6bb6..bce4027 100644
--- a/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
@@ -48,7 +48,7 @@ public class AttachVolumeCmd extends BaseAsyncVolumeCmd {
     /////////////////////////////////////////////////////
 
     @Parameter(name = ApiConstants.DEVICE_ID, type = CommandType.LONG, description = "the ID of the device to map the volume to within the guest OS. "
-        + "If no deviceId is passed in, the next available deviceId will be chosen. " + "Possible values for a Linux OS are:" + "* 1 - /dev/xvdb" + "* 2 - /dev/xvdc"
+            + "If no deviceId is passed in, the next available deviceId will be chosen. " + "Possible values for a Linux OS are:" + "* 0 - /dev/xvda" + "* 1 - /dev/xvdb" + "* 2 - /dev/xvdc"
         + "* 4 - /dev/xvde" + "* 5 - /dev/xvdf" + "* 6 - /dev/xvdg" + "* 7 - /dev/xvdh" + "* 8 - /dev/xvdi" + "* 9 - /dev/xvdj")
     private Long deviceId;
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/engine/api/src/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
index 3b555e5..df0b5e8 100644
--- a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
+++ b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
@@ -89,7 +89,7 @@ public interface VolumeOrchestrationService {
 
     DiskProfile allocateRawVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template, Account owner);
 
-    VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, Volume rootVolumeOfVm, VolumeInfo volume, HypervisorType rootDiskHyperType) throws NoTransitionException;
+    VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, VolumeInfo volume, HypervisorType rootDiskHyperType, StoragePool storagePool) throws NoTransitionException;
 
     void release(VirtualMachineProfile profile);
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
index 064ffca..c5e0983 100644
--- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
+++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
@@ -30,8 +30,6 @@ import java.util.concurrent.ExecutionException;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@@ -58,6 +56,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.to.DataTO;
 import com.cloud.agent.api.to.DiskTO;
@@ -721,20 +720,20 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati
     }
 
     @Override
-    public VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, Volume rootVolumeOfVm, VolumeInfo volume, HypervisorType rootDiskHyperType) throws NoTransitionException {
+    public VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, VolumeInfo volume, HypervisorType rootDiskHyperType, StoragePool storagePool) throws NoTransitionException {
         VirtualMachineTemplate rootDiskTmplt = _entityMgr.findById(VirtualMachineTemplate.class, vm.getTemplateId());
         DataCenter dcVO = _entityMgr.findById(DataCenter.class, vm.getDataCenterId());
         Pod pod = _entityMgr.findById(Pod.class, vm.getPodIdToDeployIn());
-        StoragePoolVO rootDiskPool = _storagePoolDao.findById(rootVolumeOfVm.getPoolId());
+
         ServiceOffering svo = _entityMgr.findById(ServiceOffering.class, vm.getServiceOfferingId());
         DiskOffering diskVO = _entityMgr.findById(DiskOffering.class, volume.getDiskOfferingId());
-        Long clusterId = (rootDiskPool == null ? null : rootDiskPool.getClusterId());
+        Long clusterId = (storagePool == null ? null : storagePool.getClusterId());
 
         VolumeInfo vol = null;
         if (volume.getState() == Volume.State.Allocated) {
             vol = createVolume(volume, vm, rootDiskTmplt, dcVO, pod, clusterId, svo, diskVO, new ArrayList<StoragePool>(), volume.getSize(), rootDiskHyperType);
         } else if (volume.getState() == Volume.State.Uploaded) {
-            vol = copyVolume(rootDiskPool, volume, vm, rootDiskTmplt, dcVO, pod, diskVO, svo, rootDiskHyperType);
+            vol = copyVolume(storagePool, volume, vm, rootDiskTmplt, dcVO, pod, diskVO, svo, rootDiskHyperType);
             if (vol != null) {
                 // Moving of Volume is successful, decrement the volume resource count from secondary for an account and increment it into primary storage under same account.
                 _resourceLimitMgr.decrementResourceCount(volume.getAccountId(), ResourceType.secondary_storage, volume.getSize());
@@ -1234,6 +1233,12 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati
             }
             throw new CloudRuntimeException("Unable to prepare Volume for vm because DeployDestination is null, vm:" + vm);
         }
+
+        // don't allow to start vm that doesn't have a root volume
+        if (_volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT).isEmpty()) {
+            throw new CloudRuntimeException("Unable to prepare volumes for vm as ROOT volume is missing");
+        }
+
         List<VolumeVO> vols = _volsDao.findUsableVolumesForInstance(vm.getId());
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("Checking if we need to prepare " + vols.size() + " volumes for " + vm);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java
index 326cba6..fb93610 100755
--- a/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java
+++ b/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java
@@ -225,6 +225,9 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
         volume.setDeviceId(deviceId);
         volume.setUpdated(new Date());
         volume.setAttached(new Date());
+        if (deviceId == 0L) {
+            volume.setVolumeType(Type.ROOT);
+        }
         update(volumeId, volume);
     }
 
@@ -235,6 +238,9 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
         volume.setDeviceId(null);
         volume.setUpdated(new Date());
         volume.setAttached(null);
+        if (findById(volumeId).getVolumeType() == Type.ROOT) {
+            volume.setVolumeType(Type.DATADISK);
+        }
         update(volumeId, volume);
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
index 9cbbb10..fc7c300 100644
--- a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
+++ b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
@@ -17,13 +17,13 @@
 package com.cloud.deploy;
 
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
+import java.util.Set;
 import java.util.Timer;
 import java.util.TreeSet;
 
@@ -33,7 +33,7 @@ import javax.naming.ConfigurationException;
 
 import org.apache.cloudstack.affinity.AffinityGroupProcessor;
 import org.apache.cloudstack.affinity.AffinityGroupService;
-import org.apache.cloudstack.affinity.AffinityGroupVMMapVO;
+import org.apache.cloudstack.affinity.AffinityGroupVMMapVO;
 import org.apache.cloudstack.affinity.AffinityGroupVO;
 import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
@@ -49,17 +49,17 @@ import org.apache.cloudstack.managed.context.ManagedContextTimerTask;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
-import org.apache.log4j.Logger;
-
-import com.cloud.agent.AgentManager;
-import com.cloud.agent.Listener;
-import com.cloud.agent.api.AgentControlAnswer;
-import com.cloud.agent.api.AgentControlCommand;
-import com.cloud.agent.api.Answer;
-import com.cloud.agent.api.Command;
-import com.cloud.agent.api.StartupCommand;
-import com.cloud.agent.api.StartupRoutingCommand;
-import com.cloud.agent.manager.allocator.HostAllocator;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.Listener;
+import com.cloud.agent.api.AgentControlAnswer;
+import com.cloud.agent.api.AgentControlCommand;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.StartupCommand;
+import com.cloud.agent.api.StartupRoutingCommand;
+import com.cloud.agent.manager.allocator.HostAllocator;
 import com.cloud.capacity.CapacityManager;
 import com.cloud.capacity.dao.CapacityDao;
 import com.cloud.configuration.Config;
@@ -68,11 +68,11 @@ import com.cloud.dc.ClusterDetailsVO;
 import com.cloud.dc.ClusterVO;
 import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenterVO;
-import com.cloud.dc.DedicatedResourceVO;
+import com.cloud.dc.DedicatedResourceVO;
 import com.cloud.dc.Pod;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.dc.dao.DataCenterDao;
-import com.cloud.dc.dao.DedicatedResourceDao;
+import com.cloud.dc.dao.DedicatedResourceDao;
 import com.cloud.dc.dao.HostPodDao;
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 import com.cloud.deploy.DeploymentPlanner.PlannerResourceUsage;
@@ -80,7 +80,7 @@ import com.cloud.deploy.dao.PlannerHostReservationDao;
 import com.cloud.exception.AffinityConflictException;
 import com.cloud.exception.ConnectionException;
 import com.cloud.exception.InsufficientServerCapacityException;
-import com.cloud.gpu.GPU;
+import com.cloud.gpu.GPU;
 import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
@@ -89,13 +89,13 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.offering.ServiceOffering;
 import com.cloud.org.Cluster;
 import com.cloud.org.Grouping;
-import com.cloud.resource.ResourceManager;
+import com.cloud.resource.ResourceManager;
 import com.cloud.resource.ResourceState;
-import com.cloud.service.ServiceOfferingDetailsVO;
-import com.cloud.service.dao.ServiceOfferingDetailsDao;
+import com.cloud.service.ServiceOfferingDetailsVO;
+import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.ScopeType;
-import com.cloud.storage.Storage;
+import com.cloud.storage.Storage;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
 import com.cloud.storage.StoragePoolHostVO;
@@ -117,7 +117,7 @@ import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.Transaction;
 import com.cloud.utils.db.TransactionCallback;
 import com.cloud.utils.db.TransactionStatus;
-import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.fsm.StateListener;
 import com.cloud.vm.DiskProfile;
 import com.cloud.vm.ReservationContext;
@@ -125,7 +125,7 @@ import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachine.Event;
 import com.cloud.vm.VirtualMachine.State;
-import com.cloud.vm.VirtualMachineProfile;
+import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
 
@@ -1161,6 +1161,11 @@ public class DeploymentPlanningManagerImpl extends ManagerBase implements Deploy
             throw new CloudRuntimeException("Unable to create deployment, no usable volumes found for the VM");
         }
 
+        // don't allow to start vm that doesn't have a root volume
+        if (_volsDao.findByInstanceAndType(vmProfile.getId(), Volume.Type.ROOT).isEmpty()) {
+            throw new CloudRuntimeException("Unable to prepare volumes for vm as ROOT volume is missing");
+        }
+
         // for each volume find list of suitable storage pools by calling the
         // allocators
         Set<Long> originalAvoidPoolSet = avoid.getPoolsToAvoid();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/server/src/com/cloud/storage/VolumeApiServiceImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
index f0b7497..cd3d897 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -26,8 +26,6 @@ import java.util.concurrent.ExecutionException;
 
 import javax.inject.Inject;
 
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
@@ -70,6 +68,7 @@ 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.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
@@ -146,82 +145,74 @@ import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
 public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiService, VmWorkJobHandler {
     private final static Logger s_logger = Logger.getLogger(VolumeApiServiceImpl.class);
-
     public static final String VM_WORK_JOB_HANDLER = VolumeApiServiceImpl.class.getSimpleName();
 
     @Inject
-    private VolumeOrchestrationService _volumeMgr;
-
-    @Inject
-    private EntityManager _entityMgr;
+    VolumeOrchestrationService _volumeMgr;
     @Inject
-    private AgentManager _agentMgr;
+    EntityManager _entityMgr;
     @Inject
-    private TemplateManager _tmpltMgr;
+    AgentManager _agentMgr;
     @Inject
-    private AsyncJobManager _asyncMgr;
+    TemplateManager _tmpltMgr;
     @Inject
-    private SnapshotManager _snapshotMgr;
+    SnapshotManager _snapshotMgr;
     @Inject
-    private AccountManager _accountMgr;
+    AccountManager _accountMgr;
     @Inject
-    private ConfigurationManager _configMgr;
+    ConfigurationManager _configMgr;
     @Inject
-    private VolumeDao _volsDao;
+    VolumeDao _volsDao;
     @Inject
-    private HostDao _hostDao;
+    HostDao _hostDao;
     @Inject
-    private SnapshotDao _snapshotDao;
+    SnapshotDao _snapshotDao;
     @Inject
-    protected ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
+    ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
     @Inject
     StoragePoolDetailsDao storagePoolDetailsDao;
     @Inject
-    private UserVmDao _userVmDao;
+    UserVmDao _userVmDao;
     @Inject
     VolumeDataStoreDao _volumeStoreDao;
     @Inject
-    private VMInstanceDao _vmInstanceDao;
+    VMInstanceDao _vmInstanceDao;
     @Inject
-    private final PrimaryDataStoreDao _storagePoolDao = null;
+    PrimaryDataStoreDao _storagePoolDao;
     @Inject
-    private DiskOfferingDao _diskOfferingDao;
+    DiskOfferingDao _diskOfferingDao;
     @Inject
-    private AccountDao _accountDao;
+    AccountDao _accountDao;
     @Inject
-    private final DataCenterDao _dcDao = null;
+    final DataCenterDao _dcDao = null;
     @Inject
-    private VMTemplateDao _templateDao;
+    VMTemplateDao _templateDao;
     @Inject
-    private VolumeDao _volumeDao;
+    ResourceLimitService _resourceLimitMgr;
     @Inject
-    private ResourceLimitService _resourceLimitMgr;
+    VmDiskStatisticsDao _vmDiskStatsDao;
     @Inject
-    private VmDiskStatisticsDao _vmDiskStatsDao;
+    VMSnapshotDao _vmSnapshotDao;
     @Inject
-    private VMSnapshotDao _vmSnapshotDao;
-    private List<StoragePoolAllocator> _storagePoolAllocators;
+    ConfigurationDao _configDao;
     @Inject
-    private ConfigurationDao _configDao;
+    DataStoreManager dataStoreMgr;
     @Inject
-    private DataStoreManager dataStoreMgr;
+    VolumeService volService;
     @Inject
-    private VolumeService volService;
+    VolumeDataFactory volFactory;
     @Inject
-    private VolumeDataFactory volFactory;
+    SnapshotApiService snapshotMgr;
     @Inject
-    private SnapshotApiService snapshotMgr;
+    UUIDManager _uuidMgr;
     @Inject
-    private UUIDManager _uuidMgr;
-
+    HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
     @Inject
-    private HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
-
+    AsyncJobManager _jobMgr;
     @Inject
-    private AsyncJobManager _jobMgr;
+    VmWorkJobDao _workJobDao;
 
-    @Inject
-    protected VmWorkJobDao _workJobDao;
+    private List<StoragePoolAllocator> _storagePoolAllocators;
 
     VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this);
 
@@ -848,7 +839,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                     else if (jobResult instanceof Throwable)
                         throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
                     else if (jobResult instanceof Long) {
-                        vol = _volumeDao.findById((Long)jobResult);
+                        vol = _volsDao.findById((Long)jobResult);
                     }
                 }
                 return volume;
@@ -1051,7 +1042,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 else if (jobResult instanceof Throwable)
                     throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
                 else if (jobResult instanceof Long) {
-                    vol = _volumeDao.findById((Long)jobResult);
+                    vol = _volsDao.findById((Long) jobResult);
                 }
             }
             return vol;
@@ -1067,19 +1058,19 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         Account caller = CallContext.current().getCallingAccount();
 
         // Check that the volume ID is valid
-        VolumeInfo volume = volFactory.getVolume(volumeId);
+        VolumeInfo volumeToAttach = volFactory.getVolume(volumeId);
         // Check that the volume is a data volume
-        if (volume == null || volume.getVolumeType() != Volume.Type.DATADISK) {
-            throw new InvalidParameterValueException("Please specify a valid data volume.");
+        if (volumeToAttach == null || !(volumeToAttach.getVolumeType() == Volume.Type.DATADISK || volumeToAttach.getVolumeType() == Volume.Type.ROOT)) {
+            throw new InvalidParameterValueException("Please specify a volume with the valid type: " + Volume.Type.ROOT.toString() + " or " + Volume.Type.DATADISK.toString());
         }
 
         // Check that the volume is not currently attached to any VM
-        if (volume.getInstanceId() != null) {
+        if (volumeToAttach.getInstanceId() != null) {
             throw new InvalidParameterValueException("Please specify a volume that is not attached to any VM.");
         }
 
         // Check that the volume is not destroyed
-        if (volume.getState() == Volume.State.Destroy) {
+        if (volumeToAttach.getState() == Volume.State.Destroy) {
             throw new InvalidParameterValueException("Please specify a volume that is not destroyed.");
         }
 
@@ -1094,31 +1085,46 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             throw new InvalidParameterValueException("Please specify a VM that is either running or stopped.");
         }
 
+        // Check that the VM and the volume are in the same zone
+        if (vm.getDataCenterId() != volumeToAttach.getDataCenterId()) {
+            throw new InvalidParameterValueException("Please specify a VM that is in the same zone as the volume.");
+        }
+
+        // permission check
+        _accountMgr.checkAccess(caller, null, true, volumeToAttach, vm);
+
         // Check that the device ID is valid
         if (deviceId != null) {
+            // validate ROOT volume type
             if (deviceId.longValue() == 0) {
-                throw new InvalidParameterValueException("deviceId can't be 0, which is used by Root device");
+                validateRootVolumeDetachAttach(_volsDao.findById(volumeToAttach.getId()), vm);
+                // vm shouldn't have any volume with deviceId 0
+                if (!_volsDao.findByInstanceAndDeviceId(vm.getId(), 0).isEmpty()) {
+                    throw new InvalidParameterValueException("Vm already has root volume attached to it");
+                }
+                // volume can't be in Uploaded state
+                if (volumeToAttach.getState() == Volume.State.Uploaded) {
+                    throw new InvalidParameterValueException("No support for Root volume attach in state " + Volume.State.Uploaded);
+                }
             }
         }
 
         // Check that the number of data volumes attached to VM is less than
         // that supported by hypervisor
-        List<VolumeVO> existingDataVolumes = _volsDao.findByInstanceAndType(vmId, Volume.Type.DATADISK);
-        int maxDataVolumesSupported = getMaxDataVolumesSupported(vm);
-        if (existingDataVolumes.size() >= maxDataVolumesSupported) {
-            throw new InvalidParameterValueException("The specified VM already has the maximum number of data disks (" + maxDataVolumesSupported + "). Please specify another VM.");
-        }
-
-        // Check that the VM and the volume are in the same zone
-        if (vm.getDataCenterId() != volume.getDataCenterId()) {
-            throw new InvalidParameterValueException("Please specify a VM that is in the same zone as the volume.");
+        if (deviceId == null || deviceId.longValue() != 0) {
+            List<VolumeVO> existingDataVolumes = _volsDao.findByInstanceAndType(vmId, Volume.Type.DATADISK);
+            int maxDataVolumesSupported = getMaxDataVolumesSupported(vm);
+            if (existingDataVolumes.size() >= maxDataVolumesSupported) {
+                throw new InvalidParameterValueException("The specified VM already has the maximum number of data disks (" + maxDataVolumesSupported + "). Please specify another VM.");
+            }
         }
+        deviceId = getDeviceId(vmId, deviceId);
 
         // If local storage is disabled then attaching a volume with local disk
         // offering not allowed
-        DataCenterVO dataCenter = _dcDao.findById(volume.getDataCenterId());
+        DataCenterVO dataCenter = _dcDao.findById(volumeToAttach.getDataCenterId());
         if (!dataCenter.isLocalStorageEnabled()) {
-            DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
+            DiskOfferingVO diskOffering = _diskOfferingDao.findById(volumeToAttach.getDiskOfferingId());
             if (diskOffering.getUseLocalStorage()) {
                 throw new InvalidParameterValueException("Zone is not configured to use local storage but volume's disk offering " + diskOffering.getName() + " uses it");
             }
@@ -1130,48 +1136,53 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             throw new InvalidParameterValueException("Unable to attach volume, please specify a VM that does not have VM snapshots");
         }
 
-        // permission check
-        _accountMgr.checkAccess(caller, null, true, volume, vm);
-
-        if (!(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Ready.equals(volume.getState()) || Volume.State.Uploaded.equals(volume.getState()))) {
+        if (!(Volume.State.Allocated.equals(volumeToAttach.getState()) || Volume.State.Ready.equals(volumeToAttach.getState()) || Volume.State.Uploaded.equals(volumeToAttach.getState()))) {
             throw new InvalidParameterValueException("Volume state must be in Allocated, Ready or in Uploaded state");
         }
 
-        VolumeVO rootVolumeOfVm = null;
+        VolumeVO exstingVolumeOfVm = null;
         List<VolumeVO> rootVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.ROOT);
-        if (rootVolumesOfVm.size() != 1) {
+        if (rootVolumesOfVm.size() > 1) {
             throw new CloudRuntimeException("The VM " + vm.getHostName() + " has more than one ROOT volume and is in an invalid state.");
         } else {
-            rootVolumeOfVm = rootVolumesOfVm.get(0);
+            if (!rootVolumesOfVm.isEmpty()) {
+                exstingVolumeOfVm = rootVolumesOfVm.get(0);
+            } else {
+                // locate data volume of the vm
+                List<VolumeVO> diskVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.DATADISK);
+                for (VolumeVO diskVolume : diskVolumesOfVm) {
+                    if (diskVolume.getState() != Volume.State.Allocated) {
+                        exstingVolumeOfVm = diskVolume;
+                        break;
+                    }
+                }
+            }
         }
 
         HypervisorType rootDiskHyperType = vm.getHypervisorType();
+        HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId());
 
-        HypervisorType dataDiskHyperType = _volsDao.getHypervisorType(volume.getId());
-
-        VolumeVO dataDiskVol = _volsDao.findById(volume.getId());
-        StoragePoolVO dataDiskStoragePool = _storagePoolDao.findById(dataDiskVol.getPoolId());
+        StoragePoolVO volumeToAttachStoragePool = _storagePoolDao.findById(volumeToAttach.getPoolId());
 
         // managed storage can be used for different types of hypervisors
         // only perform this check if the volume's storage pool is not null and not managed
-        if (dataDiskStoragePool != null && !dataDiskStoragePool.isManaged()) {
-        if (dataDiskHyperType != HypervisorType.None && rootDiskHyperType != dataDiskHyperType) {
-                throw new InvalidParameterValueException("Can't attach a volume created by: " + dataDiskHyperType + " to a " + rootDiskHyperType + " vm");
+        if (volumeToAttachStoragePool != null && !volumeToAttachStoragePool.isManaged()) {
+            if (volumeToAttachHyperType != HypervisorType.None && rootDiskHyperType != volumeToAttachHyperType) {
+                throw new InvalidParameterValueException("Can't attach a volume created by: " + volumeToAttachHyperType + " to a " + rootDiskHyperType + " vm");
             }
         }
 
-        deviceId = getDeviceId(vmId, deviceId);
-        VolumeInfo volumeOnPrimaryStorage = volume;
+        VolumeInfo newVolumeOnPrimaryStorage = volumeToAttach;
+
         //don't create volume on primary storage if its being attached to the vm which Root's volume hasn't been created yet
-        List<VolumeVO> existingRootVolumes = _volsDao.findByInstanceAndType(vmId, Volume.Type.ROOT);
-        boolean rootVolumeCreatedOnPrimary = true;
-        if (existingRootVolumes.get(0).getState().equals(Volume.State.Allocated)) {
-            rootVolumeCreatedOnPrimary = false;
+        StoragePoolVO destPrimaryStorage = null;
+        if (exstingVolumeOfVm != null && !exstingVolumeOfVm.getState().equals(Volume.State.Allocated)) {
+            destPrimaryStorage = _storagePoolDao.findById(exstingVolumeOfVm.getPoolId());
         }
 
-        if ((volume.getState().equals(Volume.State.Allocated) && rootVolumeCreatedOnPrimary) || volume.getState() == Volume.State.Uploaded) {
+        if (destPrimaryStorage != null && (volumeToAttach.getState() == Volume.State.Allocated || volumeToAttach.getState() == Volume.State.Uploaded)) {
             try {
-                volumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, rootVolumeOfVm, volume, rootDiskHyperType);
+                newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, rootDiskHyperType, destPrimaryStorage);
             } catch (NoTransitionException e) {
                 s_logger.debug("Failed to create volume on primary storage", e);
                 throw new CloudRuntimeException("Failed to create volume on primary storage", e);
@@ -1179,20 +1190,20 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
 
         // reload the volume from db
-        volumeOnPrimaryStorage = volFactory.getVolume(volumeOnPrimaryStorage.getId());
-        boolean moveVolumeNeeded = needMoveVolume(rootVolumeOfVm, volumeOnPrimaryStorage);
+        newVolumeOnPrimaryStorage = volFactory.getVolume(newVolumeOnPrimaryStorage.getId());
+        boolean moveVolumeNeeded = needMoveVolume(exstingVolumeOfVm, newVolumeOnPrimaryStorage);
 
         if (moveVolumeNeeded) {
-            PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)volumeOnPrimaryStorage.getDataStore();
+            PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)newVolumeOnPrimaryStorage.getDataStore();
             if (primaryStore.isLocal()) {
-                throw new CloudRuntimeException("Failed to attach local data volume " + volume.getName() + " to VM " + vm.getDisplayName()
+                throw new CloudRuntimeException("Failed to attach local data volume " + volumeToAttach.getName() + " to VM " + vm.getDisplayName()
                         + " as migration of local data volume is not allowed");
             }
-            StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(rootVolumeOfVm.getPoolId());
+            StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(exstingVolumeOfVm.getPoolId());
 
             try {
-                volumeOnPrimaryStorage = _volumeMgr.moveVolume(volumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
-                        vmRootVolumePool.getClusterId(), dataDiskHyperType);
+                newVolumeOnPrimaryStorage = _volumeMgr.moveVolume(newVolumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(),
+                        vmRootVolumePool.getClusterId(), volumeToAttachHyperType);
             } catch (ConcurrentOperationException e) {
                 s_logger.debug("move volume failed", e);
                 throw new CloudRuntimeException("move volume failed", e);
@@ -1211,10 +1222,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 s_logger.info("Trying to attaching volume " + volumeId + " to vm instance:" + vm.getId() + ", update async job-" + job.getId() + " progress status");
             }
 
-            _asyncMgr.updateAsyncJobAttachment(job.getId(), "volume", volumeId);
+            _jobMgr.updateAsyncJobAttachment(job.getId(), "volume", volumeId);
         }
 
-        VolumeVO newVol = _volumeDao.findById(volumeOnPrimaryStorage.getId());
+        VolumeVO newVol = _volsDao.findById(newVolumeOnPrimaryStorage.getId());
         newVol = sendAttachVolumeCommand(vm, newVol, deviceId);
         return newVol;
     }
@@ -1223,7 +1234,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     @ActionEvent(eventType = EventTypes.EVENT_VOLUME_UPDATE, eventDescription = "updating volume", async = true)
     public Volume updateVolume(long volumeId, String path, String state, Long storageId, Boolean displayVolume, String customId, long entityOwnerId, String chainInfo) {
 
-        VolumeVO volume = _volumeDao.findById(volumeId);
+        VolumeVO volume = _volsDao.findById(volumeId);
 
         if(volume == null)
             throw new InvalidParameterValueException("The volume id doesn't exist");
@@ -1259,7 +1270,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
         updateDisplay(volume, displayVolume);
 
-        _volumeDao.update(volumeId, volume);
+        _volsDao.update(volumeId, volume);
 
         return volume;
     }
@@ -1277,7 +1288,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         if (displayVolume != null && displayVolume != volume.isDisplayVolume()){
             // FIXME - Confused - typecast for now.
             ((VolumeVO)volume).setDisplayVolume(displayVolume);
-            _volumeDao.update(volume.getId(), (VolumeVO)volume);
+            _volsDao.update(volume.getId(), (VolumeVO) volume);
         }
 
     }
@@ -1347,10 +1358,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         // Permissions check
         _accountMgr.checkAccess(caller, null, true, volume);
 
-        // Check that the volume is a data volume
-        if (volume.getVolumeType() != Volume.Type.DATADISK) {
-            throw new InvalidParameterValueException("Please specify a data volume.");
-        }
 
         // Check that the volume is currently attached to a VM
         if (vmId == null) {
@@ -1363,6 +1370,16 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             throw new InvalidParameterValueException("Please specify a VM that is either running or stopped.");
         }
 
+        // Check that the volume is a data/root volume
+        if (!(volume.getVolumeType() == Volume.Type.ROOT || volume.getVolumeType() == Volume.Type.DATADISK)) {
+            throw new InvalidParameterValueException("Please specify volume of type " + Volume.Type.DATADISK.toString() + " or " + Volume.Type.ROOT.toString());
+        }
+
+        // Root volume detach is allowed for following hypervisors: Xen/KVM/VmWare
+        if (volume.getVolumeType() == Volume.Type.ROOT) {
+            validateRootVolumeDetachAttach(volume, vm);
+        }
+
         // Don't allow detach if target VM has associated VM snapshots
         List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.findByVm(vmId);
         if (vmSnapshots.size() > 0) {
@@ -1377,7 +1394,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 s_logger.info("Trying to attaching volume " + volumeId + "to vm instance:" + vm.getId() + ", update async job-" + job.getId() + " progress status");
             }
 
-            _asyncMgr.updateAsyncJobAttachment(job.getId(), "volume", volumeId);
+            _jobMgr.updateAsyncJobAttachment(job.getId(), "volume", volumeId);
         }
 
         AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
@@ -1388,7 +1405,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 placeHolder = createPlaceHolderWork(vmId);
             }
             try {
-            return orchestrateDetachVolumeFromVM(vmId, volumeId);
+                return orchestrateDetachVolumeFromVM(vmId, volumeId);
             } finally {
                 if (VmJobEnabled.value())
                     _workJobDao.expunge(placeHolder.getId());
@@ -1412,16 +1429,32 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 else if (jobResult instanceof Throwable)
                     throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
                 else if (jobResult instanceof Long) {
-                    vol = _volumeDao.findById((Long)jobResult);
+                    vol = _volsDao.findById((Long) jobResult);
                 }
             }
             return vol;
         }
     }
 
+    private void validateRootVolumeDetachAttach(VolumeVO volume, UserVmVO vm) {
+        if (!(vm.getHypervisorType() == HypervisorType.XenServer)) {
+            throw new InvalidParameterValueException("Root volume detach is allowed for hypervisor type " + HypervisorType.XenServer + " only");
+        }
+        if (!(vm.getState() == State.Stopped) || (vm.getState() == State.Destroyed)) {
+            throw new InvalidParameterValueException("Root volume detach can happen only when vm is in states: " + State.Stopped.toString() + " or " + State.Destroyed.toString());
+        }
+
+        if (volume.getPoolId() != null) {
+            StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
+            if (pool.isManaged()) {
+                throw new InvalidParameterValueException("Root volume detach is not supported for Managed DataStores");
+            }
+        }
+    }
+
     private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) {
 
-        Volume volume = _volumeDao.findById(volumeId);
+        Volume volume = _volsDao.findById(volumeId);
         VMInstanceVO vm = _vmInstanceDao.findById(vmId);
 
         String errorMsg = "Failed to detach volume " + volume.getName() + " from VM " + vm.getHostName();
@@ -1782,7 +1815,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             throw new PermissionDeniedException("Extraction has been disabled by admin");
         }
 
-        VolumeVO volume = _volumeDao.findById(volumeId);
+        VolumeVO volume = _volsDao.findById(volumeId);
         if (volume == null) {
             InvalidParameterValueException ex = new InvalidParameterValueException("Unable to find volume with specified volumeId");
             ex.addProxyObject(volumeId.toString(), "volumeId");
@@ -1877,7 +1910,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
     @Override
     public boolean isDisplayResourceEnabled(Long id) {
-        Volume volume = _volumeDao.findById(id);
+        Volume volume = _volsDao.findById(id);
         if (volume == null) {
             return true; // bad id given, default to true
         }
@@ -1902,55 +1935,55 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
     }
 
-    private boolean needMoveVolume(VolumeVO rootVolumeOfVm, VolumeInfo volume) {
-        if (rootVolumeOfVm.getPoolId() == null || volume.getPoolId() == null) {
+    private boolean needMoveVolume(VolumeVO existingVolume, VolumeInfo newVolume) {
+        if (existingVolume == null || existingVolume.getPoolId() == null || newVolume.getPoolId() == null) {
             return false;
         }
 
-        DataStore storeForRootVol = dataStoreMgr.getPrimaryDataStore(rootVolumeOfVm.getPoolId());
-        DataStore storeForDataVol = dataStoreMgr.getPrimaryDataStore(volume.getPoolId());
+        DataStore storeForExistingVol = dataStoreMgr.getPrimaryDataStore(existingVolume.getPoolId());
+        DataStore storeForNewVol = dataStoreMgr.getPrimaryDataStore(newVolume.getPoolId());
 
-        Scope storeForRootStoreScope = storeForRootVol.getScope();
-        if (storeForRootStoreScope == null) {
-            throw new CloudRuntimeException("Can't get scope of data store: " + storeForRootVol.getId());
+        Scope storeForExistingStoreScope = storeForExistingVol.getScope();
+        if (storeForExistingStoreScope == null) {
+            throw new CloudRuntimeException("Can't get scope of data store: " + storeForExistingVol.getId());
         }
 
-        Scope storeForDataStoreScope = storeForDataVol.getScope();
-        if (storeForDataStoreScope == null) {
-            throw new CloudRuntimeException("Can't get scope of data store: " + storeForDataVol.getId());
+        Scope storeForNewStoreScope = storeForNewVol.getScope();
+        if (storeForNewStoreScope == null) {
+            throw new CloudRuntimeException("Can't get scope of data store: " + storeForNewVol.getId());
         }
 
-        if (storeForDataStoreScope.getScopeType() == ScopeType.ZONE) {
+        if (storeForNewStoreScope.getScopeType() == ScopeType.ZONE) {
             return false;
         }
 
-        if (storeForRootStoreScope.getScopeType() != storeForDataStoreScope.getScopeType()) {
-            if (storeForDataStoreScope.getScopeType() == ScopeType.CLUSTER) {
+        if (storeForExistingStoreScope.getScopeType() != storeForNewStoreScope.getScopeType()) {
+            if (storeForNewStoreScope.getScopeType() == ScopeType.CLUSTER) {
                 Long vmClusterId = null;
-                if (storeForRootStoreScope.getScopeType() == ScopeType.HOST) {
-                HostScope hs = (HostScope)storeForRootStoreScope;
+                if (storeForExistingStoreScope.getScopeType() == ScopeType.HOST) {
+                HostScope hs = (HostScope)storeForExistingStoreScope;
                     vmClusterId = hs.getClusterId();
-                } else if (storeForRootStoreScope.getScopeType() == ScopeType.ZONE) {
-                    Long hostId = _vmInstanceDao.findById(rootVolumeOfVm.getInstanceId()).getHostId();
+                } else if (storeForExistingStoreScope.getScopeType() == ScopeType.ZONE) {
+                    Long hostId = _vmInstanceDao.findById(existingVolume.getInstanceId()).getHostId();
                     if (hostId != null) {
                         HostVO host = _hostDao.findById(hostId);
                         vmClusterId = host.getClusterId();
                     }
                 }
-                if (storeForDataStoreScope.getScopeId().equals(vmClusterId)) {
+                if (storeForNewStoreScope.getScopeId().equals(vmClusterId)) {
                     return false;
             }
-            } else if (storeForDataStoreScope.getScopeType() == ScopeType.HOST
-                    && (storeForRootStoreScope.getScopeType() == ScopeType.CLUSTER || storeForRootStoreScope.getScopeType() == ScopeType.ZONE)) {
-                Long hostId = _vmInstanceDao.findById(rootVolumeOfVm.getInstanceId()).getHostId();
-                if (storeForDataStoreScope.getScopeId().equals(hostId)) {
+            } else if (storeForNewStoreScope.getScopeType() == ScopeType.HOST
+                    && (storeForExistingStoreScope.getScopeType() == ScopeType.CLUSTER || storeForExistingStoreScope.getScopeType() == ScopeType.ZONE)) {
+                Long hostId = _vmInstanceDao.findById(existingVolume.getInstanceId()).getHostId();
+                if (storeForNewStoreScope.getScopeId().equals(hostId)) {
                     return false;
                 }
             }
-            throw new CloudRuntimeException("Can't move volume between scope: " + storeForDataStoreScope.getScopeType() + " and " + storeForRootStoreScope.getScopeType());
+            throw new CloudRuntimeException("Can't move volume between scope: " + storeForNewStoreScope.getScopeType() + " and " + storeForExistingStoreScope.getScopeType());
         }
 
-        return !storeForRootStoreScope.isSameScope(storeForDataStoreScope);
+        return !storeForExistingStoreScope.isSameScope(storeForNewStoreScope);
     }
 
     private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, Long deviceId) {
@@ -1975,7 +2008,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         if (hostId != null) {
             host = _hostDao.findById(hostId);
 
-            if (host != null && host.getHypervisorType() == HypervisorType.XenServer && volumeToAttachStoragePool.isManaged()) {
+            if (host != null && host.getHypervisorType() == HypervisorType.XenServer && volumeToAttachStoragePool != null && volumeToAttachStoragePool.isManaged()) {
                 sendCommand = true;
             }
         }
@@ -2100,7 +2133,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         // allocate deviceId
         List<VolumeVO> vols = _volsDao.findByInstance(vmId);
         if (deviceId != null) {
-            if (deviceId.longValue() > 15 || deviceId.longValue() == 0 || deviceId.longValue() == 3) {
+            if (deviceId.longValue() > 15 || deviceId.longValue() == 3) {
                 throw new RuntimeException("deviceId should be 1,2,4-15");
             }
             for (VolumeVO vol : vols) {
@@ -2161,7 +2194,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
         @Override
         protected Volume retrieve() {
-            return _volumeDao.findById(_volumeId);
+            return _volsDao.findById(_volumeId);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3a889aa2/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
new file mode 100644
index 0000000..7f60229
--- /dev/null
+++ b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
@@ -0,0 +1,316 @@
+package com.cloud.storage;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.when;
+
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.ControlledEntity;
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext;
+import org.apache.cloudstack.framework.jobs.AsyncJobManager;
+import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao;
+import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+import com.cloud.user.AccountVO;
+import com.cloud.user.UserVO;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+
+public class VolumeApiServiceImplTest {
+    @Inject
+    VolumeApiServiceImpl _svc = new VolumeApiServiceImpl();
+    @Mock
+    VolumeDao _volumeDao;
+    @Mock
+    AccountManager _accountMgr;
+    @Mock
+    UserVmDao _userVmDao;
+    @Mock
+    PrimaryDataStoreDao _storagePoolDao;
+    @Mock
+    VMSnapshotDao _vmSnapshotDao;
+    @Mock
+    AsyncJobManager _jobMgr;
+    @Mock
+    AsyncJobJoinMapDao _joinMapDao;
+    @Mock
+    VolumeDataFactory _volFactory;
+
+    @Mock
+    VMInstanceDao _vmInstanceDao;
+
+    DetachVolumeCmd detachCmd = new DetachVolumeCmd();
+    Class<?> _detachCmdClass = detachCmd.getClass();
+
+
+    @Before
+    public void setup() throws Exception {
+        MockitoAnnotations.initMocks(this);
+        _svc._volsDao = _volumeDao;
+        _svc._accountMgr = _accountMgr;
+        _svc._userVmDao = _userVmDao;
+        _svc._storagePoolDao = _storagePoolDao;
+        _svc._vmSnapshotDao = _vmSnapshotDao;
+        _svc._vmInstanceDao = _vmInstanceDao;
+        _svc._jobMgr = _jobMgr;
+        _svc.volFactory = _volFactory;
+
+        // mock caller context
+        AccountVO account = new AccountVO("admin", 1L, "networkDomain", Account.ACCOUNT_TYPE_NORMAL, "uuid");
+        UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString());
+        CallContext.register(user, account);
+        // mock async context
+        AsyncJobExecutionContext context = new AsyncJobExecutionContext();
+        AsyncJobExecutionContext.init(_svc._jobMgr, _joinMapDao);
+        AsyncJobVO job = new AsyncJobVO();
+        context.setJob(job);
+        AsyncJobExecutionContext.setCurrentExecutionContext(context);
+
+        TransactionLegacy txn = TransactionLegacy.open("runVolumeDaoImplTest");
+        try {
+            // volume of running vm id=1
+            VolumeVO volumeOfRunningVm = new VolumeVO("root", 1L, 1L, 1L, 1L, 1L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            when(_svc._volsDao.findById(1L)).thenReturn(volumeOfRunningVm);
+
+            UserVmVO runningVm = new UserVmVO(1L, "vm", "vm", 1, HypervisorType.XenServer, 1L, false,
+                    false, 1L, 1L, 1L, null, "vm", null);
+            runningVm.setState(State.Running);
+            runningVm.setDataCenterId(1L);
+            when(_svc._userVmDao.findById(1L)).thenReturn(runningVm);
+
+            // volume of stopped vm id=2
+            VolumeVO volumeOfStoppedVm = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            volumeOfStoppedVm.setPoolId(1L);
+            when(_svc._volsDao.findById(2L)).thenReturn(volumeOfStoppedVm);
+
+            UserVmVO stoppedVm = new UserVmVO(2L, "vm", "vm", 1, HypervisorType.XenServer, 1L, false,
+                    false, 1L, 1L, 1L, null, "vm", null);
+            stoppedVm.setState(State.Stopped);
+            stoppedVm.setDataCenterId(1L);
+            when(_svc._userVmDao.findById(2L)).thenReturn(stoppedVm);
+
+
+            // volume of hyperV vm id=3
+            UserVmVO hyperVVm = new UserVmVO(3L, "vm", "vm", 1, HypervisorType.Hyperv, 1L, false,
+                    false, 1L, 1L, 1L, null, "vm", null);
+            hyperVVm.setState(State.Stopped);
+            hyperVVm.setDataCenterId(1L);
+            when(_svc._userVmDao.findById(3L)).thenReturn(hyperVVm);
+
+            VolumeVO volumeOfStoppeHyperVVm = new VolumeVO("root", 1L, 1L, 1L, 1L, 3L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            volumeOfStoppeHyperVVm.setPoolId(1L);
+            when(_svc._volsDao.findById(3L)).thenReturn(volumeOfStoppeHyperVVm);
+
+            StoragePoolVO unmanagedPool = new StoragePoolVO();
+            when(_svc._storagePoolDao.findById(1L)).thenReturn(unmanagedPool);
+
+            // volume of managed pool id=4
+            StoragePoolVO managedPool = new StoragePoolVO();
+            managedPool.setManaged(true);
+            when(_svc._storagePoolDao.findById(2L)).thenReturn(managedPool);
+            VolumeVO managedPoolVolume = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            managedPoolVolume.setPoolId(2L);
+            when(_svc._volsDao.findById(4L)).thenReturn(managedPoolVolume);
+
+            // non-root non-datadisk volume
+            VolumeInfo volumeWithIncorrectVolumeType = Mockito.mock(VolumeInfo.class);
+            when(volumeWithIncorrectVolumeType.getId()).thenReturn(5L);
+            when(volumeWithIncorrectVolumeType.getVolumeType()).thenReturn(Volume.Type.ISO);
+            when(_svc.volFactory.getVolume(5L)).thenReturn(volumeWithIncorrectVolumeType);
+
+            // correct root volume
+            VolumeInfo correctRootVolume = Mockito.mock(VolumeInfo.class);
+            when(correctRootVolume.getId()).thenReturn(6L);
+            when(correctRootVolume.getDataCenterId()).thenReturn(1L);
+            when(correctRootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT);
+            when(correctRootVolume.getInstanceId()).thenReturn(null);
+            when(_svc.volFactory.getVolume(6L)).thenReturn(correctRootVolume);
+
+            VolumeVO correctRootVolumeVO = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            when(_svc._volsDao.findById(6L)).thenReturn(correctRootVolumeVO);
+
+            // managed root volume
+            VolumeInfo managedVolume = Mockito.mock(VolumeInfo.class);
+            when(managedVolume.getId()).thenReturn(7L);
+            when(managedVolume.getDataCenterId()).thenReturn(1L);
+            when(managedVolume.getVolumeType()).thenReturn(Volume.Type.ROOT);
+            when(managedVolume.getInstanceId()).thenReturn(null);
+            when(managedVolume.getPoolId()).thenReturn(2L);
+            when(_svc.volFactory.getVolume(7L)).thenReturn(managedVolume);
+
+            VolumeVO managedVolume1 = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            managedVolume1.setPoolId(2L);
+            managedVolume1.setDataCenterId(1L);
+            when(_svc._volsDao.findById(7L)).thenReturn(managedVolume1);
+
+            // vm having root volume
+            UserVmVO vmHavingRootVolume = new UserVmVO(4L, "vm", "vm", 1, HypervisorType.XenServer, 1L, false,
+                    false, 1L, 1L, 1L, null, "vm", null);
+            vmHavingRootVolume.setState(State.Stopped);
+            vmHavingRootVolume.setDataCenterId(1L);
+            when(_svc._userVmDao.findById(4L)).thenReturn(vmHavingRootVolume);
+            List<VolumeVO> vols = new ArrayList<VolumeVO>();
+            vols.add(new VolumeVO());
+            when(_svc._volsDao.findByInstanceAndDeviceId(4L, 0L)).thenReturn(vols);
+
+            // volume in uploaded state
+            VolumeInfo uploadedVolume = Mockito.mock(VolumeInfo.class);
+            when(uploadedVolume.getId()).thenReturn(8L);
+            when(uploadedVolume.getDataCenterId()).thenReturn(1L);
+            when(uploadedVolume.getVolumeType()).thenReturn(Volume.Type.ROOT);
+            when(uploadedVolume.getInstanceId()).thenReturn(null);
+            when(uploadedVolume.getPoolId()).thenReturn(1L);
+            when(uploadedVolume.getState()).thenReturn(Volume.State.Uploaded);
+            when(_svc.volFactory.getVolume(8L)).thenReturn(uploadedVolume);
+
+            VolumeVO upVolume = new VolumeVO("root", 1L, 1L, 1L, 1L, 2L, "root", "root", 1, null,
+                    null, "root", Volume.Type.ROOT);
+            upVolume.setPoolId(1L);
+            upVolume.setDataCenterId(1L);
+            upVolume.setState(Volume.State.Uploaded);
+            when(_svc._volsDao.findById(8L)).thenReturn(upVolume);
+
+            // helper dao methods mock
+            when(_svc._vmSnapshotDao.findByVm(any(Long.class))).thenReturn(new ArrayList<VMSnapshotVO>());
+            when(_svc._vmInstanceDao.findById(any(Long.class))).thenReturn(stoppedVm);
+
+        } finally {
+            txn.close("runVolumeDaoImplTest");
+        }
+
+        // helper methods mock
+        doNothing().when(_svc._accountMgr).checkAccess(any(Account.class), any(AccessType.class), any(Boolean.class), any(ControlledEntity.class));
+        doNothing().when(_svc._jobMgr).updateAsyncJobAttachment(any(Long.class), any(String.class), any(Long.class));
+        when(_svc._jobMgr.submitAsyncJob(any(AsyncJobVO.class), any(String.class), any(Long.class))).thenReturn(1L);
+    }
+
+    /**
+     * TESTS FOR DETACH ROOT VOLUME, COUNT=4
+     * @throws Exception
+     */
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testDetachVolumeFromRunningVm() throws NoSuchFieldException, IllegalAccessException {
+        Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
+        dedicateIdField.setAccessible(true);
+        dedicateIdField.set(detachCmd, 1L);
+        _svc.detachVolumeFromVM(detachCmd);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testDetachVolumeFromStoppedHyperVVm() throws NoSuchFieldException, IllegalAccessException {
+        Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
+        dedicateIdField.setAccessible(true);
+        dedicateIdField.set(detachCmd, 3L);
+        _svc.detachVolumeFromVM(detachCmd);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testDetachVolumeOfManagedDataStore() throws NoSuchFieldException, IllegalAccessException {
+        Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
+        dedicateIdField.setAccessible(true);
+        dedicateIdField.set(detachCmd, 4L);
+        _svc.detachVolumeFromVM(detachCmd);
+    }
+
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
+    @Test
+    public void testDetachVolumeFromStoppedXenVm() throws NoSuchFieldException, IllegalAccessException {
+        thrown.expect(NullPointerException.class);
+        Field dedicateIdField = _detachCmdClass.getDeclaredField("id");
+        dedicateIdField.setAccessible(true);
+        dedicateIdField.set(detachCmd, 2L);
+        _svc.detachVolumeFromVM(detachCmd);
+    }
+
+    /**
+     * TESTS FOR ATTACH ROOT VOLUME, COUNT=7
+     */
+
+    // Negative test - try to attach non-root non-datadisk volume
+    @Test(expected = InvalidParameterValueException.class)
+    public void attachIncorrectDiskType() throws NoSuchFieldException, IllegalAccessException {
+        _svc.attachVolumeToVM(1L, 5L, 0L);
+    }
+
+    // Negative test - attach root volume to running vm
+    @Test(expected = InvalidParameterValueException.class)
+    public void attachRootDiskToRunningVm() throws NoSuchFieldException, IllegalAccessException {
+        _svc.attachVolumeToVM(1L, 6L, 0L);
+    }
+
+    // Negative test - attach root volume to non-xen vm
+    @Test(expected = InvalidParameterValueException.class)
+    public void attachRootDiskToHyperVm() throws NoSuchFieldException, IllegalAccessException {
+        _svc.attachVolumeToVM(3L, 6L, 0L);
+    }
+
+    // Negative test - attach root volume from the managed data store
+    @Test(expected = InvalidParameterValueException.class)
+    public void attachRootDiskOfManagedDataStore() throws NoSuchFieldException, IllegalAccessException {
+        _svc.attachVolumeToVM(2L, 7L, 0L);
+    }
+
+    // Negative test - root volume can't be attached to the vm already having a root volume attached
+    @Test(expected = InvalidParameterValueException.class)
+    public void attachRootDiskToVmHavingRootDisk() throws NoSuchFieldException, IllegalAccessException {
+        _svc.attachVolumeToVM(4L, 6L, 0L);
+    }
+
+    // Negative test - root volume in uploaded state can't be attached
+    @Test(expected = InvalidParameterValueException.class)
+    public void attachRootInUploadedState() throws NoSuchFieldException, IllegalAccessException {
+        _svc.attachVolumeToVM(2L, 8L, 0L);
+    }
+
+    // Positive test - attach ROOT volume in correct state, to the vm not having root volume attached
+    @Test
+    public void attachRootVolumePositive() throws NoSuchFieldException, IllegalAccessException {
+        thrown.expect(NullPointerException.class);
+        _svc.attachVolumeToVM(2L, 6L, 0L);
+    }
+
+    @After
+    public void tearDown() {
+        CallContext.unregister();
+    }
+}


Mime
View raw message