diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index 40777b38b6d2..96704641bc45 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -257,5 +257,11 @@ static String getHypervisorHostname(String name) { UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException; + /** + * Returns true if the VM's Root volume is allocated at a local storage pool + */ + boolean isRootVolumeOnLocalStorage(long vmId); + Pair findClusterAndHostIdForVm(long vmId); + } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 765bb907b27c..6967e74f9970 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -3632,22 +3632,7 @@ public void checkIfCanUpgrade(final VirtualMachine vmInstance, final ServiceOffe final ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); - // Check that the service offering being upgraded to has the same Guest IP type as the VM's current service offering - // NOTE: With the new network refactoring in 2.2, we shouldn't need the check for same guest IP type anymore. - /* - * if (!currentServiceOffering.getGuestIpType().equals(newServiceOffering.getGuestIpType())) { String errorMsg = - * "The service offering being upgraded to has a guest IP type: " + newServiceOffering.getGuestIpType(); errorMsg += - * ". Please select a service offering with the same guest IP type as the VM's current service offering (" + - * currentServiceOffering.getGuestIpType() + ")."; throw new InvalidParameterValueException(errorMsg); } - */ - - // Check that the service offering being upgraded to has the same storage pool preference as the VM's current service - // offering - if (currentServiceOffering.isUseLocalStorage() != newServiceOffering.isUseLocalStorage()) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + - ", cannot switch between local storage and shared storage service offerings. Current offering " + "useLocalStorage=" + - currentServiceOffering.isUseLocalStorage() + ", target offering useLocalStorage=" + newServiceOffering.isUseLocalStorage()); - } + checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstance, newServiceOffering); // if vm is a system vm, check if it is a system service offering, if yes return with error as it cannot be used for user vms if (currentServiceOffering.isSystemUse() != newServiceOffering.isSystemUse()) { @@ -3669,6 +3654,39 @@ public void checkIfCanUpgrade(final VirtualMachine vmInstance, final ServiceOffe } } + /** + * Throws an InvalidParameterValueException in case the new service offerings does not match the storage scope (e.g. local or shared). + */ + protected void checkIfNewOfferingStorageScopeMatchesStoragePool(VirtualMachine vmInstance, ServiceOffering newServiceOffering) { + boolean isRootVolumeOnLocalStorage = isRootVolumeOnLocalStorage(vmInstance.getId()); + + if (newServiceOffering.isUseLocalStorage() && !isRootVolumeOnLocalStorage) { + String message = String .format("Unable to upgrade virtual machine %s, target offering use local storage but the storage pool where " + + "the volume is allocated is a shared storage.", vmInstance.toString()); + throw new InvalidParameterValueException(message); + } + + if (!newServiceOffering.isUseLocalStorage() && isRootVolumeOnLocalStorage) { + String message = String.format("Unable to upgrade virtual machine %s, target offering use shared storage but the storage pool where " + + "the volume is allocated is a local storage.", vmInstance.toString()); + throw new InvalidParameterValueException(message); + } + } + + public boolean isRootVolumeOnLocalStorage(long vmId) { + ScopeType poolScope = ScopeType.ZONE; + List volumes = _volsDao.findByInstanceAndType(vmId, Type.ROOT); + if(CollectionUtils.isNotEmpty(volumes)) { + VolumeVO rootDisk = volumes.get(0); + Long poolId = rootDisk.getPoolId(); + if (poolId != null) { + StoragePoolVO storagePoolVO = _storagePoolDao.findById(poolId); + poolScope = storagePoolVO.getScope(); + } + } + return ScopeType.HOST == poolScope; + } + @Override public boolean upgradeVmDb(final long vmId, final ServiceOffering newServiceOffering, ServiceOffering currentServiceOffering) { diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 1725a413145c..d8df27d07319 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; +import com.cloud.exception.InvalidParameterValueException; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -623,4 +624,59 @@ public void matchesOfSorts() { assertTrue(VirtualMachineManagerImpl.matches(tags,three)); assertTrue(VirtualMachineManagerImpl.matches(others,three)); } + + @Test + public void isRootVolumeOnLocalStorageTestOnLocal() { + prepareAndTestIsRootVolumeOnLocalStorage(ScopeType.HOST, true); + } + + @Test + public void isRootVolumeOnLocalStorageTestCluster() { + prepareAndTestIsRootVolumeOnLocalStorage(ScopeType.CLUSTER, false); + } + + @Test + public void isRootVolumeOnLocalStorageTestZone() { + prepareAndTestIsRootVolumeOnLocalStorage(ScopeType.ZONE, false); + } + + private void prepareAndTestIsRootVolumeOnLocalStorage(ScopeType scope, boolean expected) { + StoragePoolVO storagePoolVoMock = Mockito.mock(StoragePoolVO.class); + Mockito.doReturn(storagePoolVoMock).when(storagePoolDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(scope).when(storagePoolVoMock).getScope(); + List mockedVolumes = new ArrayList<>(); + mockedVolumes.add(volumeVoMock); + Mockito.doReturn(mockedVolumes).when(volumeDaoMock).findByInstanceAndType(Mockito.anyLong(), Mockito.any()); + + boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l); + + Assert.assertEquals(expected, result); + } + + @Test + public void checkIfNewOfferingStorageScopeMatchesStoragePoolTestLocalLocal() { + prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(true, true); + } + + @Test + public void checkIfNewOfferingStorageScopeMatchesStoragePoolTestSharedShared() { + prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(false, false); + } + + @Test (expected = InvalidParameterValueException.class) + public void checkIfNewOfferingStorageScopeMatchesStoragePoolTestLocalShared() { + prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(true, false); + } + + @Test (expected = InvalidParameterValueException.class) + public void checkIfNewOfferingStorageScopeMatchesStoragePoolTestSharedLocal() { + prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(false, true); + } + + private void prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(boolean isRootOnLocal, boolean isOfferingUsingLocal) { + Mockito.doReturn(isRootOnLocal).when(virtualMachineManagerImpl).isRootVolumeOnLocalStorage(Mockito.anyLong()); + Mockito.doReturn("vmInstanceMockedToString").when(vmInstanceMock).toString(); + Mockito.doReturn(isOfferingUsingLocal).when(serviceOfferingMock).isUseLocalStorage(); + virtualMachineManagerImpl.checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstanceMock, serviceOfferingMock); + } } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index cf01b8db3409..2e6bc8a39cc9 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -32,6 +32,7 @@ import javax.inject.Inject; import com.cloud.storage.dao.VMTemplateDetailsDao; +import com.cloud.vm.VirtualMachineManager; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO; import org.apache.cloudstack.affinity.AffinityGroupResponse; @@ -424,6 +425,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject private UserDao userDao; + @Inject + private VirtualMachineManager virtualMachineManager; + /* * (non-Javadoc) * @@ -2959,8 +2963,10 @@ private Pair, Integer> searchForServiceOfferingsInte sc.addAnd("id", SearchCriteria.Op.NEQ, currentVmOffering.getId()); } - // 1. Only return offerings with the same storage type - sc.addAnd("useLocalStorage", SearchCriteria.Op.EQ, currentVmOffering.isUseLocalStorage()); + boolean isRootVolumeUsingLocalStorage = virtualMachineManager.isRootVolumeOnLocalStorage(vmId); + + // 1. Only return offerings with the same storage type than the storage pool where the VM's root volume is allocated + sc.addAnd("useLocalStorage", SearchCriteria.Op.EQ, isRootVolumeUsingLocalStorage); // 2.In case vm is running return only offerings greater than equal to current offering compute. if (vmInstance.getState() == VirtualMachine.State.Running) {