Skip to content

Commit d11dcea

Browse files
author
Alena Prokharchyk
committed
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
1 parent cf4ef95 commit d11dcea

10 files changed

+119
-116
lines changed

server/src/com/cloud/agent/manager/allocator/HostAllocator.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.cloud.host.Host;
2020
import com.cloud.host.Host.Type;
2121
import com.cloud.offering.ServiceOffering;
22-
import com.cloud.uservm.UserVm;
2322
import com.cloud.utils.component.Adapter;
2423
import com.cloud.vm.VirtualMachine;
2524
import com.cloud.vm.VirtualMachineProfile;
@@ -30,7 +29,7 @@ public interface HostAllocator extends Adapter {
3029
* @param UserVm vm
3130
* @param ServiceOffering offering
3231
**/
33-
boolean isVirtualMachineUpgradable(final UserVm vm, final ServiceOffering offering);
32+
boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering);
3433

3534
/**
3635
* Determines which physical hosts are suitable to

server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import com.cloud.storage.dao.GuestOSCategoryDao;
4646
import com.cloud.storage.dao.GuestOSDao;
4747
import com.cloud.user.Account;
48-
import com.cloud.uservm.UserVm;
4948
import com.cloud.utils.NumbersUtil;
5049
import com.cloud.utils.component.ComponentLocator;
5150
import com.cloud.utils.component.Inject;
@@ -266,7 +265,7 @@ private List<HostVO> reorderHostsByNumberOfVms(DeploymentPlan plan, List<HostVO>
266265
}
267266

268267
@Override
269-
public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) {
268+
public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) {
270269
// currently we do no special checks to rule out a VM being upgradable to an offering, so
271270
// return true
272271
return true;

server/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.cloud.host.dao.HostDao;
3131
import com.cloud.offering.ServiceOffering;
3232
import com.cloud.resource.ResourceManager;
33-
import com.cloud.uservm.UserVm;
3433
import com.cloud.utils.component.ComponentLocator;
3534
import com.cloud.vm.VirtualMachine;
3635
import com.cloud.vm.VirtualMachineProfile;
@@ -106,7 +105,7 @@ public List<Host> allocateTo(VirtualMachineProfile<? extends VirtualMachine> vmP
106105
}
107106

108107
@Override
109-
public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) {
108+
public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) {
110109
// currently we do no special checks to rule out a VM being upgradable to an offering, so
111110
// return true
112111
return true;

server/src/com/cloud/agent/manager/allocator/impl/TestingAllocator.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.cloud.host.Host.Type;
2626
import com.cloud.host.dao.HostDao;
2727
import com.cloud.offering.ServiceOffering;
28-
import com.cloud.uservm.UserVm;
2928
import com.cloud.utils.component.ComponentLocator;
3029
import com.cloud.vm.VirtualMachine;
3130
import com.cloud.vm.VirtualMachineProfile;
@@ -65,7 +64,7 @@ public List<Host> allocateTo(VirtualMachineProfile<? extends VirtualMachine> vmP
6564
}
6665

6766
@Override
68-
public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) {
67+
public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) {
6968
// currently we do no special checks to rule out a VM being upgradable to an offering, so
7069
// return true
7170
return true;

server/src/com/cloud/server/ManagementServerImpl.java

+9-38
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@
160160
import com.cloud.network.dao.IPAddressDao;
161161
import com.cloud.network.dao.LoadBalancerDao;
162162
import com.cloud.network.dao.NetworkDao;
163-
import com.cloud.offering.ServiceOffering;
164163
import com.cloud.org.Grouping.AllocationState;
165164
import com.cloud.projects.Project;
166165
import com.cloud.projects.Project.ListProjectResourcesCriteria;
@@ -244,9 +243,6 @@
244243
import com.cloud.vm.dao.SecondaryStorageVmDao;
245244
import com.cloud.vm.dao.UserVmDao;
246245
import com.cloud.vm.dao.VMInstanceDao;
247-
import com.cloud.utils.exception.CSExceptionErrorCode;
248-
import com.cloud.utils.AnnotationHelper;
249-
250246
import edu.emory.mathcs.backport.java.util.Arrays;
251247
import edu.emory.mathcs.backport.java.util.Collections;
252248

@@ -3448,50 +3444,25 @@ public VirtualMachine upgradeSystemVM(UpgradeSystemVMCmd cmd) {
34483444
Long serviceOfferingId = cmd.getServiceOfferingId();
34493445
Account caller = UserContext.current().getCaller();
34503446

3451-
VMInstanceVO systemVm = _vmInstanceDao.findByIdTypes(systemVmId, VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm);
3447+
VMInstanceVO systemVm = _vmInstanceDao.findByIdTypes(systemVmId, VirtualMachine.Type.ConsoleProxy,
3448+
VirtualMachine.Type.SecondaryStorageVm);
34523449
if (systemVm == null) {
34533450
throw new InvalidParameterValueException("Unable to find SystemVm with id " + systemVmId);
34543451
}
34553452

34563453
_accountMgr.checkAccess(caller, null, true, systemVm);
3454+
3455+
// Check that the specified service offering ID is valid
3456+
_itMgr.checkIfCanUpgrade(systemVm, serviceOfferingId);
34573457

3458-
if (systemVm.getServiceOfferingId() == serviceOfferingId) {
3459-
s_logger.debug("System vm id: " + systemVmId + "already has service offering: " + serviceOfferingId);
3460-
return systemVm;
3461-
}
3462-
3463-
ServiceOffering newServiceOffering = _configMgr.getServiceOffering(serviceOfferingId);
3464-
if (newServiceOffering == null) {
3465-
throw new InvalidParameterValueException("Unable to find service offering with id " + serviceOfferingId);
3466-
}
3467-
3468-
// check if it is a system service offering, if yes return with error as it cannot be used for user vms
3469-
if (!newServiceOffering.getSystemUse()) {
3470-
throw new InvalidParameterValueException("Cannot upgrade system vm to a non system service offering " + serviceOfferingId);
3471-
}
3472-
3473-
// Check that the system vm is stopped
3474-
if (!systemVm.getState().equals(State.Stopped)) {
3475-
s_logger.warn("Unable to upgrade system vm " + systemVm + " in state " + systemVm.getState());
3476-
throw new InvalidParameterValueException("Unable to upgrade system vm " + systemVm + " in state " + systemVm.getState()
3477-
+ "; make sure the system vm is stopped and not in an error state before upgrading.");
3478-
}
3479-
3480-
ServiceOffering currentServiceOffering = _configMgr.getServiceOffering(systemVm.getServiceOfferingId());
3481-
3482-
// Check that the service offering being upgraded to has the same storage pool preference as the VM's current service
3483-
// offering
3484-
if (currentServiceOffering.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) {
3485-
throw new InvalidParameterValueException("Can't upgrade, due to new local storage status : " + newServiceOffering.getUseLocalStorage() + " is different from "
3486-
+ "curruent local storage status: " + currentServiceOffering.getUseLocalStorage());
3487-
}
3488-
3489-
systemVm.setServiceOfferingId(serviceOfferingId);
3490-
if (_vmInstanceDao.update(systemVmId, systemVm)) {
3458+
boolean result = _itMgr.upgradeVmDb(systemVmId, serviceOfferingId);
3459+
3460+
if (result) {
34913461
return _vmInstanceDao.findById(systemVmId);
34923462
} else {
34933463
throw new CloudRuntimeException("Unable to upgrade system vm " + systemVm);
34943464
}
34953465

34963466
}
3467+
34973468
}

server/src/com/cloud/vm/UserVmManagerImpl.java

+8-63
Original file line numberDiff line numberDiff line change
@@ -952,82 +952,27 @@ private UserVm rebootVirtualMachine(long userId, long vmId) throws InsufficientC
952952
* TODO: cleanup eventually - Refactored API call
953953
*/
954954
public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) {
955-
Long virtualMachineId = cmd.getId();
956-
Long serviceOfferingId = cmd.getServiceOfferingId();
955+
Long vmId = cmd.getId();
956+
Long svcOffId = cmd.getServiceOfferingId();
957957
Account caller = UserContext.current().getCaller();
958958

959959
// Verify input parameters
960-
UserVmVO vmInstance = _vmDao.findById(virtualMachineId);
960+
UserVmVO vmInstance = _vmDao.findById(vmId);
961961
if (vmInstance == null) {
962-
throw new InvalidParameterValueException("unable to find a virtual machine with id " + virtualMachineId);
962+
throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId);
963963
}
964964

965965
_accountMgr.checkAccess(caller, null, true, vmInstance);
966966

967967
// Check that the specified service offering ID is valid
968-
ServiceOfferingVO newServiceOffering = _offeringDao.findById(serviceOfferingId);
969-
if (newServiceOffering == null) {
970-
throw new InvalidParameterValueException("Unable to find a service offering with id " + serviceOfferingId);
971-
}
972-
973-
// Check that the VM is stopped
974-
if (!vmInstance.getState().equals(State.Stopped)) {
975-
s_logger.warn("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState());
976-
throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState()
977-
+ "; make sure the virtual machine is stopped and not in an error state before upgrading.");
978-
}
979-
980-
// Check if the service offering being upgraded to is what the VM is already running with
981-
if (vmInstance.getServiceOfferingId() == newServiceOffering.getId()) {
982-
if (s_logger.isInfoEnabled()) {
983-
s_logger.info("Not upgrading vm " + vmInstance.toString() + " since it already has the requested service offering (" + newServiceOffering.getName() + ")");
984-
}
985-
986-
throw new InvalidParameterValueException("Not upgrading vm " + vmInstance.toString() + " since it already has the requested service offering (" + newServiceOffering.getName() + ")");
987-
}
988-
989-
ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(vmInstance.getServiceOfferingId());
990-
991-
// Check that the service offering being upgraded to has the same Guest IP type as the VM's current service offering
992-
// NOTE: With the new network refactoring in 2.2, we shouldn't need the check for same guest IP type anymore.
993-
/*
994-
* if (!currentServiceOffering.getGuestIpType().equals(newServiceOffering.getGuestIpType())) { String errorMsg =
995-
* "The service offering being upgraded to has a guest IP type: " + newServiceOffering.getGuestIpType(); errorMsg +=
996-
* ". Please select a service offering with the same guest IP type as the VM's current service offering (" +
997-
* currentServiceOffering.getGuestIpType() + ")."; throw new InvalidParameterValueException(errorMsg); }
998-
*/
999-
1000-
// Check that the service offering being upgraded to has the same storage pool preference as the VM's current service
1001-
// offering
1002-
if (currentServiceOffering.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) {
1003-
throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString()
1004-
+ ", cannot switch between local storage and shared storage service offerings. Current offering useLocalStorage=" + currentServiceOffering.getUseLocalStorage()
1005-
+ ", target offering useLocalStorage=" + newServiceOffering.getUseLocalStorage());
1006-
}
1007-
1008-
// Check that there are enough resources to upgrade the service offering
1009-
if (!_itMgr.isVirtualMachineUpgradable(vmInstance, newServiceOffering)) {
1010-
throw new InvalidParameterValueException("Unable to upgrade virtual machine, not enough resources available for an offering of " + newServiceOffering.getCpu() + " cpu(s) at "
1011-
+ newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory");
1012-
}
1013-
1014-
// Check that the service offering being upgraded to has all the tags of the current service offering
1015-
List<String> currentTags = _configMgr.csvTagsToList(currentServiceOffering.getTags());
1016-
List<String> newTags = _configMgr.csvTagsToList(newServiceOffering.getTags());
1017-
if (!newTags.containsAll(currentTags)) {
1018-
throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering does not have all the tags of the "
1019-
+ "current service offering. Current service offering tags: " + currentTags + "; " + "new service offering tags: " + newTags);
1020-
}
1021-
1022-
UserVmVO vmForUpdate = _vmDao.createForUpdate();
1023-
vmForUpdate.setServiceOfferingId(serviceOfferingId);
1024-
vmForUpdate.setHaEnabled(_serviceOfferingDao.findById(serviceOfferingId).getOfferHA());
1025-
vmForUpdate.setLimitCpuUse(_serviceOfferingDao.findById(serviceOfferingId).getLimitCpuUse());
1026-
_vmDao.update(vmInstance.getId(), vmForUpdate);
968+
_itMgr.checkIfCanUpgrade(vmInstance, svcOffId);
969+
970+
_itMgr.upgradeVmDb(vmId, svcOffId);
1027971

1028972
return _vmDao.findById(vmInstance.getId());
1029973
}
1030974

975+
1031976
@Override
1032977
public HashMap<Long, VmStatsEntry> getVirtualMachineStatistics(long hostId, String hostName, List<Long> vmIds) throws CloudRuntimeException {
1033978
HashMap<Long, VmStatsEntry> vmStatsById = new HashMap<Long, VmStatsEntry>();

server/src/com/cloud/vm/VirtualMachineManager.java

+14-2
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import com.cloud.storage.VMTemplateVO;
3535
import com.cloud.user.Account;
3636
import com.cloud.user.User;
37-
import com.cloud.uservm.UserVm;
3837
import com.cloud.utils.Pair;
3938
import com.cloud.utils.component.Manager;
4039
import com.cloud.utils.fsm.NoTransitionException;
@@ -114,10 +113,23 @@ <T extends VMInstanceVO> T allocate(T vm,
114113
* @param offering
115114
* @return true if the host can handle the upgrade, false otherwise
116115
*/
117-
boolean isVirtualMachineUpgradable(final UserVm vm, final ServiceOffering offering);
116+
boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering);
118117

119118
VMInstanceVO findById(long vmId);
120119

121120
<T extends VMInstanceVO> T storageMigration(T vm, StoragePool storagePoolId);
122121

122+
/**
123+
* @param vmInstance
124+
* @param newServiceOfferingId
125+
*/
126+
void checkIfCanUpgrade(VirtualMachine vmInstance, long newServiceOfferingId);
127+
128+
/**
129+
* @param vmId
130+
* @param serviceOfferingId
131+
* @return
132+
*/
133+
boolean upgradeVmDb(long vmId, long serviceOfferingId);
134+
123135
}

server/src/com/cloud/vm/VirtualMachineManagerImpl.java

+82-2
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
import com.cloud.exception.InsufficientCapacityException;
8787
import com.cloud.exception.InsufficientServerCapacityException;
8888
import com.cloud.exception.InsufficientVirtualNetworkCapcityException;
89+
import com.cloud.exception.InvalidParameterValueException;
8990
import com.cloud.exception.ManagementServerException;
9091
import com.cloud.exception.OperationTimedoutException;
9192
import com.cloud.exception.ResourceUnavailableException;
@@ -126,7 +127,6 @@
126127
import com.cloud.user.User;
127128
import com.cloud.user.dao.AccountDao;
128129
import com.cloud.user.dao.UserDao;
129-
import com.cloud.uservm.UserVm;
130130
import com.cloud.utils.Journal;
131131
import com.cloud.utils.NumbersUtil;
132132
import com.cloud.utils.Pair;
@@ -1542,7 +1542,7 @@ public void run() {
15421542
}
15431543

15441544
@Override
1545-
public boolean isVirtualMachineUpgradable(UserVm vm, ServiceOffering offering) {
1545+
public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering offering) {
15461546
Enumeration<HostAllocator> en = _hostAllocators.enumeration();
15471547
boolean isMachineUpgradable = true;
15481548
while (isMachineUpgradable && en.hasMoreElements()) {
@@ -2346,4 +2346,84 @@ public String getHostUuid() {
23462346
public VMInstanceVO findById(long vmId) {
23472347
return _vmDao.findById(vmId);
23482348
}
2349+
2350+
@Override
2351+
public void checkIfCanUpgrade(VirtualMachine vmInstance, long newServiceOfferingId) {
2352+
ServiceOfferingVO newServiceOffering = _offeringDao.findById(newServiceOfferingId);
2353+
if (newServiceOffering == null) {
2354+
throw new InvalidParameterValueException("Unable to find a service offering with id " + newServiceOfferingId);
2355+
}
2356+
2357+
// Check that the VM is stopped
2358+
if (!vmInstance.getState().equals(State.Stopped)) {
2359+
s_logger.warn("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState());
2360+
throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " " +
2361+
"in state " + vmInstance.getState()
2362+
+ "; make sure the virtual machine is stopped and not in an error state before upgrading.");
2363+
}
2364+
2365+
// Check if the service offering being upgraded to is what the VM is already running with
2366+
if (vmInstance.getServiceOfferingId() == newServiceOffering.getId()) {
2367+
if (s_logger.isInfoEnabled()) {
2368+
s_logger.info("Not upgrading vm " + vmInstance.toString() + " since it already has the requested " +
2369+
"service offering (" + newServiceOffering.getName() + ")");
2370+
}
2371+
2372+
throw new InvalidParameterValueException("Not upgrading vm " + vmInstance.toString() + " since it already " +
2373+
"has the requested service offering (" + newServiceOffering.getName() + ")");
2374+
}
2375+
2376+
ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(vmInstance.getServiceOfferingId());
2377+
2378+
// Check that the service offering being upgraded to has the same Guest IP type as the VM's current service offering
2379+
// NOTE: With the new network refactoring in 2.2, we shouldn't need the check for same guest IP type anymore.
2380+
/*
2381+
* if (!currentServiceOffering.getGuestIpType().equals(newServiceOffering.getGuestIpType())) { String errorMsg =
2382+
* "The service offering being upgraded to has a guest IP type: " + newServiceOffering.getGuestIpType(); errorMsg +=
2383+
* ". Please select a service offering with the same guest IP type as the VM's current service offering (" +
2384+
* currentServiceOffering.getGuestIpType() + ")."; throw new InvalidParameterValueException(errorMsg); }
2385+
*/
2386+
2387+
// Check that the service offering being upgraded to has the same storage pool preference as the VM's current service
2388+
// offering
2389+
if (currentServiceOffering.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) {
2390+
throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString()
2391+
+ ", cannot switch between local storage and shared storage service offerings. Current offering " +
2392+
"useLocalStorage=" + currentServiceOffering.getUseLocalStorage()
2393+
+ ", target offering useLocalStorage=" + newServiceOffering.getUseLocalStorage());
2394+
}
2395+
2396+
// 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
2397+
if (currentServiceOffering.getSystemUse() != newServiceOffering.getSystemUse()) {
2398+
throw new InvalidParameterValueException("isSystem property is different for current service offering and new service offering");
2399+
}
2400+
2401+
// Check that there are enough resources to upgrade the service offering
2402+
if (!isVirtualMachineUpgradable(vmInstance, newServiceOffering)) {
2403+
throw new InvalidParameterValueException("Unable to upgrade virtual machine, not enough resources available " +
2404+
"for an offering of " + newServiceOffering.getCpu() + " cpu(s) at "
2405+
+ newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory");
2406+
}
2407+
2408+
// Check that the service offering being upgraded to has all the tags of the current service offering
2409+
List<String> currentTags = _configMgr.csvTagsToList(currentServiceOffering.getTags());
2410+
List<String> newTags = _configMgr.csvTagsToList(newServiceOffering.getTags());
2411+
if (!newTags.containsAll(currentTags)) {
2412+
throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering " +
2413+
"does not have all the tags of the "
2414+
+ "current service offering. Current service offering tags: " + currentTags + "; " + "new service " +
2415+
"offering tags: " + newTags);
2416+
}
2417+
}
2418+
2419+
@Override
2420+
public boolean upgradeVmDb(long vmId, long serviceOfferingId) {
2421+
VMInstanceVO vmForUpdate = _vmDao.createForUpdate();
2422+
vmForUpdate.setServiceOfferingId(serviceOfferingId);
2423+
ServiceOffering newSvcOff = _configMgr.getServiceOffering(serviceOfferingId);
2424+
vmForUpdate.setHaEnabled(newSvcOff.getOfferHA());
2425+
vmForUpdate.setLimitCpuUse(newSvcOff.getLimitCpuUse());
2426+
vmForUpdate.setServiceOfferingId(newSvcOff.getId());
2427+
return _vmDao.update(vmId, vmForUpdate);
2428+
}
23492429
}

0 commit comments

Comments
 (0)