From d11dceaccc1a5c9cc120ac876f0140827fbc8950 Mon Sep 17 00:00:00 2001 From: Alena Prokharchyk Date: Tue, 15 May 2012 12:35:00 -0700 Subject: [PATCH] CS-14904 Fixed the bug where vm_instance.ha_enabled wasn't updated during service offering upgrade Conflicts: server/src/com/cloud/server/ManagementServerImpl.java --- .../manager/allocator/HostAllocator.java | 3 +- .../allocator/impl/FirstFitAllocator.java | 3 +- .../allocator/impl/RandomAllocator.java | 3 +- .../allocator/impl/TestingAllocator.java | 3 +- .../cloud/server/ManagementServerImpl.java | 47 ++--------- .../src/com/cloud/vm/UserVmManagerImpl.java | 71 ++-------------- .../com/cloud/vm/VirtualMachineManager.java | 16 +++- .../cloud/vm/VirtualMachineManagerImpl.java | 84 ++++++++++++++++++- .../vm/MockVirtualMachineManagerImpl.java | 3 +- wscript | 2 +- 10 files changed, 119 insertions(+), 116 deletions(-) diff --git a/server/src/com/cloud/agent/manager/allocator/HostAllocator.java b/server/src/com/cloud/agent/manager/allocator/HostAllocator.java index ef413e374fe..b462dc9b2e3 100755 --- a/server/src/com/cloud/agent/manager/allocator/HostAllocator.java +++ b/server/src/com/cloud/agent/manager/allocator/HostAllocator.java @@ -19,7 +19,6 @@ import com.cloud.deploy.DeploymentPlanner.ExcludeList; import com.cloud.host.Host; import com.cloud.host.Host.Type; import com.cloud.offering.ServiceOffering; -import com.cloud.uservm.UserVm; import com.cloud.utils.component.Adapter; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; @@ -30,7 +29,7 @@ public interface HostAllocator extends Adapter { * @param UserVm vm * @param ServiceOffering offering **/ - boolean isVirtualMachineUpgradable(final UserVm vm, final ServiceOffering offering); + boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering); /** * Determines which physical hosts are suitable to diff --git a/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java b/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java index 676aed93a8a..95151cf5be9 100755 --- a/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java +++ b/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java @@ -45,7 +45,6 @@ import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.GuestOSCategoryDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.user.Account; -import com.cloud.uservm.UserVm; import com.cloud.utils.NumbersUtil; import com.cloud.utils.component.ComponentLocator; import com.cloud.utils.component.Inject; @@ -266,7 +265,7 @@ public class FirstFitAllocator implements HostAllocator { } @Override - public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) { + public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) { // currently we do no special checks to rule out a VM being upgradable to an offering, so // return true return true; diff --git a/server/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java b/server/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java index 0e1e8dba1d7..3ca2da16072 100755 --- a/server/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java +++ b/server/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java @@ -30,7 +30,6 @@ import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.offering.ServiceOffering; import com.cloud.resource.ResourceManager; -import com.cloud.uservm.UserVm; import com.cloud.utils.component.ComponentLocator; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; @@ -106,7 +105,7 @@ public class RandomAllocator implements HostAllocator { } @Override - public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) { + public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) { // currently we do no special checks to rule out a VM being upgradable to an offering, so // return true return true; diff --git a/server/src/com/cloud/agent/manager/allocator/impl/TestingAllocator.java b/server/src/com/cloud/agent/manager/allocator/impl/TestingAllocator.java index b2d45bdd203..deb93527521 100755 --- a/server/src/com/cloud/agent/manager/allocator/impl/TestingAllocator.java +++ b/server/src/com/cloud/agent/manager/allocator/impl/TestingAllocator.java @@ -25,7 +25,6 @@ import com.cloud.host.Host; import com.cloud.host.Host.Type; import com.cloud.host.dao.HostDao; import com.cloud.offering.ServiceOffering; -import com.cloud.uservm.UserVm; import com.cloud.utils.component.ComponentLocator; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; @@ -65,7 +64,7 @@ public class TestingAllocator implements HostAllocator { } @Override - public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) { + public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) { // currently we do no special checks to rule out a VM being upgradable to an offering, so // return true return true; diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 35cdb91592a..46974bbb8f9 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -160,7 +160,6 @@ import com.cloud.network.NetworkVO; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.LoadBalancerDao; import com.cloud.network.dao.NetworkDao; -import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping.AllocationState; import com.cloud.projects.Project; import com.cloud.projects.Project.ListProjectResourcesCriteria; @@ -244,9 +243,6 @@ import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.SecondaryStorageVmDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; -import com.cloud.utils.exception.CSExceptionErrorCode; -import com.cloud.utils.AnnotationHelper; - import edu.emory.mathcs.backport.java.util.Arrays; import edu.emory.mathcs.backport.java.util.Collections; @@ -3448,50 +3444,25 @@ public class ManagementServerImpl implements ManagementServer { Long serviceOfferingId = cmd.getServiceOfferingId(); Account caller = UserContext.current().getCaller(); - VMInstanceVO systemVm = _vmInstanceDao.findByIdTypes(systemVmId, VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm); + VMInstanceVO systemVm = _vmInstanceDao.findByIdTypes(systemVmId, VirtualMachine.Type.ConsoleProxy, + VirtualMachine.Type.SecondaryStorageVm); if (systemVm == null) { throw new InvalidParameterValueException("Unable to find SystemVm with id " + systemVmId); } _accountMgr.checkAccess(caller, null, true, systemVm); + + // Check that the specified service offering ID is valid + _itMgr.checkIfCanUpgrade(systemVm, serviceOfferingId); - if (systemVm.getServiceOfferingId() == serviceOfferingId) { - s_logger.debug("System vm id: " + systemVmId + "already has service offering: " + serviceOfferingId); - return systemVm; - } - - ServiceOffering newServiceOffering = _configMgr.getServiceOffering(serviceOfferingId); - if (newServiceOffering == null) { - throw new InvalidParameterValueException("Unable to find service offering with id " + serviceOfferingId); - } - - // check if it is a system service offering, if yes return with error as it cannot be used for user vms - if (!newServiceOffering.getSystemUse()) { - throw new InvalidParameterValueException("Cannot upgrade system vm to a non system service offering " + serviceOfferingId); - } - - // Check that the system vm is stopped - if (!systemVm.getState().equals(State.Stopped)) { - s_logger.warn("Unable to upgrade system vm " + systemVm + " in state " + systemVm.getState()); - throw new InvalidParameterValueException("Unable to upgrade system vm " + systemVm + " in state " + systemVm.getState() - + "; make sure the system vm is stopped and not in an error state before upgrading."); - } - - ServiceOffering currentServiceOffering = _configMgr.getServiceOffering(systemVm.getServiceOfferingId()); - - // Check that the service offering being upgraded to has the same storage pool preference as the VM's current service - // offering - if (currentServiceOffering.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) { - throw new InvalidParameterValueException("Can't upgrade, due to new local storage status : " + newServiceOffering.getUseLocalStorage() + " is different from " - + "curruent local storage status: " + currentServiceOffering.getUseLocalStorage()); - } - - systemVm.setServiceOfferingId(serviceOfferingId); - if (_vmInstanceDao.update(systemVmId, systemVm)) { + boolean result = _itMgr.upgradeVmDb(systemVmId, serviceOfferingId); + + if (result) { return _vmInstanceDao.findById(systemVmId); } else { throw new CloudRuntimeException("Unable to upgrade system vm " + systemVm); } } + } diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 41ab695dd46..5d8bcc84642 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -952,82 +952,27 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager * TODO: cleanup eventually - Refactored API call */ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) { - Long virtualMachineId = cmd.getId(); - Long serviceOfferingId = cmd.getServiceOfferingId(); + Long vmId = cmd.getId(); + Long svcOffId = cmd.getServiceOfferingId(); Account caller = UserContext.current().getCaller(); // Verify input parameters - UserVmVO vmInstance = _vmDao.findById(virtualMachineId); + UserVmVO vmInstance = _vmDao.findById(vmId); if (vmInstance == null) { - throw new InvalidParameterValueException("unable to find a virtual machine with id " + virtualMachineId); + throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); } _accountMgr.checkAccess(caller, null, true, vmInstance); // Check that the specified service offering ID is valid - ServiceOfferingVO newServiceOffering = _offeringDao.findById(serviceOfferingId); - if (newServiceOffering == null) { - throw new InvalidParameterValueException("Unable to find a service offering with id " + serviceOfferingId); - } - - // Check that the VM is stopped - if (!vmInstance.getState().equals(State.Stopped)) { - s_logger.warn("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState()); - throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState() - + "; make sure the virtual machine is stopped and not in an error state before upgrading."); - } - - // Check if the service offering being upgraded to is what the VM is already running with - if (vmInstance.getServiceOfferingId() == newServiceOffering.getId()) { - if (s_logger.isInfoEnabled()) { - s_logger.info("Not upgrading vm " + vmInstance.toString() + " since it already has the requested service offering (" + newServiceOffering.getName() + ")"); - } - - throw new InvalidParameterValueException("Not upgrading vm " + vmInstance.toString() + " since it already has the requested service offering (" + newServiceOffering.getName() + ")"); - } - - ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(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.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() - + ", cannot switch between local storage and shared storage service offerings. Current offering useLocalStorage=" + currentServiceOffering.getUseLocalStorage() - + ", target offering useLocalStorage=" + newServiceOffering.getUseLocalStorage()); - } - - // Check that there are enough resources to upgrade the service offering - if (!_itMgr.isVirtualMachineUpgradable(vmInstance, newServiceOffering)) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine, not enough resources available for an offering of " + newServiceOffering.getCpu() + " cpu(s) at " - + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory"); - } - - // Check that the service offering being upgraded to has all the tags of the current service offering - List currentTags = _configMgr.csvTagsToList(currentServiceOffering.getTags()); - List newTags = _configMgr.csvTagsToList(newServiceOffering.getTags()); - if (!newTags.containsAll(currentTags)) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering does not have all the tags of the " - + "current service offering. Current service offering tags: " + currentTags + "; " + "new service offering tags: " + newTags); - } - - UserVmVO vmForUpdate = _vmDao.createForUpdate(); - vmForUpdate.setServiceOfferingId(serviceOfferingId); - vmForUpdate.setHaEnabled(_serviceOfferingDao.findById(serviceOfferingId).getOfferHA()); - vmForUpdate.setLimitCpuUse(_serviceOfferingDao.findById(serviceOfferingId).getLimitCpuUse()); - _vmDao.update(vmInstance.getId(), vmForUpdate); + _itMgr.checkIfCanUpgrade(vmInstance, svcOffId); + + _itMgr.upgradeVmDb(vmId, svcOffId); return _vmDao.findById(vmInstance.getId()); } + @Override public HashMap getVirtualMachineStatistics(long hostId, String hostName, List vmIds) throws CloudRuntimeException { HashMap vmStatsById = new HashMap(); diff --git a/server/src/com/cloud/vm/VirtualMachineManager.java b/server/src/com/cloud/vm/VirtualMachineManager.java index 0a2b11a769d..3387a8e22e7 100644 --- a/server/src/com/cloud/vm/VirtualMachineManager.java +++ b/server/src/com/cloud/vm/VirtualMachineManager.java @@ -34,7 +34,6 @@ import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateVO; import com.cloud.user.Account; import com.cloud.user.User; -import com.cloud.uservm.UserVm; import com.cloud.utils.Pair; import com.cloud.utils.component.Manager; import com.cloud.utils.fsm.NoTransitionException; @@ -114,10 +113,23 @@ public interface VirtualMachineManager extends Manager { * @param offering * @return true if the host can handle the upgrade, false otherwise */ - boolean isVirtualMachineUpgradable(final UserVm vm, final ServiceOffering offering); + boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering); VMInstanceVO findById(long vmId); T storageMigration(T vm, StoragePool storagePoolId); + /** + * @param vmInstance + * @param newServiceOfferingId + */ + void checkIfCanUpgrade(VirtualMachine vmInstance, long newServiceOfferingId); + + /** + * @param vmId + * @param serviceOfferingId + * @return + */ + boolean upgradeVmDb(long vmId, long serviceOfferingId); + } diff --git a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java index cd03303f8c9..b73d0fd8e48 100755 --- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -86,6 +86,7 @@ import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InsufficientServerCapacityException; import com.cloud.exception.InsufficientVirtualNetworkCapcityException; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ManagementServerException; import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceUnavailableException; @@ -126,7 +127,6 @@ import com.cloud.user.AccountManager; import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.user.dao.UserDao; -import com.cloud.uservm.UserVm; import com.cloud.utils.Journal; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; @@ -1542,7 +1542,7 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene } @Override - public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) { + public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) { Enumeration en = _hostAllocators.enumeration(); boolean isMachineUpgradable = true; while (isMachineUpgradable && en.hasMoreElements()) { @@ -2346,4 +2346,84 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene public VMInstanceVO findById(long vmId) { return _vmDao.findById(vmId); } + + @Override + public void checkIfCanUpgrade(VirtualMachine vmInstance, long newServiceOfferingId) { + ServiceOfferingVO newServiceOffering = _offeringDao.findById(newServiceOfferingId); + if (newServiceOffering == null) { + throw new InvalidParameterValueException("Unable to find a service offering with id " + newServiceOfferingId); + } + + // Check that the VM is stopped + if (!vmInstance.getState().equals(State.Stopped)) { + s_logger.warn("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState()); + throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " " + + "in state " + vmInstance.getState() + + "; make sure the virtual machine is stopped and not in an error state before upgrading."); + } + + // Check if the service offering being upgraded to is what the VM is already running with + if (vmInstance.getServiceOfferingId() == newServiceOffering.getId()) { + if (s_logger.isInfoEnabled()) { + s_logger.info("Not upgrading vm " + vmInstance.toString() + " since it already has the requested " + + "service offering (" + newServiceOffering.getName() + ")"); + } + + throw new InvalidParameterValueException("Not upgrading vm " + vmInstance.toString() + " since it already " + + "has the requested service offering (" + newServiceOffering.getName() + ")"); + } + + ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(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.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) { + throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + + ", cannot switch between local storage and shared storage service offerings. Current offering " + + "useLocalStorage=" + currentServiceOffering.getUseLocalStorage() + + ", target offering useLocalStorage=" + newServiceOffering.getUseLocalStorage()); + } + + // 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.getSystemUse() != newServiceOffering.getSystemUse()) { + throw new InvalidParameterValueException("isSystem property is different for current service offering and new service offering"); + } + + // Check that there are enough resources to upgrade the service offering + if (!isVirtualMachineUpgradable(vmInstance, newServiceOffering)) { + throw new InvalidParameterValueException("Unable to upgrade virtual machine, not enough resources available " + + "for an offering of " + newServiceOffering.getCpu() + " cpu(s) at " + + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory"); + } + + // Check that the service offering being upgraded to has all the tags of the current service offering + List currentTags = _configMgr.csvTagsToList(currentServiceOffering.getTags()); + List newTags = _configMgr.csvTagsToList(newServiceOffering.getTags()); + if (!newTags.containsAll(currentTags)) { + throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering " + + "does not have all the tags of the " + + "current service offering. Current service offering tags: " + currentTags + "; " + "new service " + + "offering tags: " + newTags); + } + } + + @Override + public boolean upgradeVmDb(long vmId, long serviceOfferingId) { + VMInstanceVO vmForUpdate = _vmDao.createForUpdate(); + vmForUpdate.setServiceOfferingId(serviceOfferingId); + ServiceOffering newSvcOff = _configMgr.getServiceOffering(serviceOfferingId); + vmForUpdate.setHaEnabled(newSvcOff.getOfferHA()); + vmForUpdate.setLimitCpuUse(newSvcOff.getLimitCpuUse()); + vmForUpdate.setServiceOfferingId(newSvcOff.getId()); + return _vmDao.update(vmId, vmForUpdate); + } } diff --git a/server/test/com/cloud/vm/MockVirtualMachineManagerImpl.java b/server/test/com/cloud/vm/MockVirtualMachineManagerImpl.java index b8f5c955d3c..ea4950ba2b0 100755 --- a/server/test/com/cloud/vm/MockVirtualMachineManagerImpl.java +++ b/server/test/com/cloud/vm/MockVirtualMachineManagerImpl.java @@ -37,7 +37,6 @@ import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateVO; import com.cloud.user.Account; import com.cloud.user.User; -import com.cloud.uservm.UserVm; import com.cloud.utils.Pair; import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.VirtualMachine.Event; @@ -138,7 +137,7 @@ public class MockVirtualMachineManagerImpl implements VirtualMachineManager { } @Override - public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) { + public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) { // TODO Auto-generated method stub return false; } diff --git a/wscript b/wscript index e081a4de41b..1ead7af8061 100644 --- a/wscript +++ b/wscript @@ -3,7 +3,7 @@ # the following two variables are used by the target "waf dist" # if you change 'em here, you need to change it also in cloud.spec, add a %changelog entry there, and add an entry in debian/changelog -VERSION = '3.0.3.2012-05-11T20:40:47Z' +VERSION = '3.0.3.2012-05-15T19:32:03Z' APPNAME = 'cloud' import shutil,os