From 24435dd6bc2424da18277ca00229d1d3bb0ec284 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 16 Apr 2015 16:51:02 +0530 Subject: [PATCH] server: NPE checks and improved case checking - pool allocation checks for both root and data disks - NPE checks to not add null object in collection or try to migrate null VM - HA work tries need to increment and be given up when max retries are crossed - VM creation should check IP address format for IPv4 and IPv6 - If userdata is not supported by a network, then fail early if userdata, ssh key, or password enabled template is passed/used Signed-off-by: Rohit Yadav --- .../deploy/DeploymentPlanningManagerImpl.java | 2 +- .../cloud/ha/HighAvailabilityManagerImpl.java | 14 +++++++++++++- server/src/com/cloud/vm/UserVmManagerImpl.java | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java index 54149c5880b..7986b3a2dec 100755 --- a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -1208,7 +1208,7 @@ StateListener { // volume is ready and the pool should be reused. // In this case, also check if rest of the volumes are ready and can // be reused. - if (plan.getPoolId() != null) { + if (plan.getPoolId() != null || (toBeCreated.getVolumeType() == Volume.Type.DATADISK && toBeCreated.getPoolId() != null && toBeCreated.getState() == Volume.State.Ready)) { s_logger.debug("Volume has pool already allocated, checking if pool can be reused, poolId: " + toBeCreated.getPoolId()); List suitablePools = new ArrayList(); StoragePool pool = null; diff --git a/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java index 111b9ce425b..ce6239ae83f 100755 --- a/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -634,6 +634,9 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvai _haDao.update(work.getId(), work); VMInstanceVO vm = _instanceDao.findById(vmId); + if (vm == null) { + return null; + } // First try starting the vm with its original planner, if it doesn't succeed send HAPlanner as its an emergency. _itMgr.migrateAway(vm.getUuid(), srcHostId); return null; @@ -753,7 +756,10 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvai List works = _haDao.findTakenWorkItems(WorkType.Migration); List vms = new ArrayList(works.size()); for (HaWorkVO work : works) { - vms.add(_instanceDao.findById(work.getInstanceId())); + VMInstanceVO vm = _instanceDao.findById(work.getInstanceId()); + if (vm != null) { + vms.add(vm); + } } return vms; } @@ -913,6 +919,7 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvai } else { s_logger.info("Rescheduling " + work + " to try again at " + new Date(nextTime << 10)); work.setTimeToTry(nextTime); + work.setTimesTried(work.getTimesTried() + 1); work.setServerId(null); work.setDateTaken(null); } @@ -923,6 +930,7 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvai s_logger.info("Rescheduling " + work + " to try again at " + new Date(nextTime << 10)); work.setTimeToTry(nextTime); + work.setTimesTried(work.getTimesTried() + 1); work.setServerId(null); work.setDateTaken(null); @@ -931,6 +939,10 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvai VMInstanceVO vm = _instanceDao.findById(work.getInstanceId()); work.setUpdateTime(vm.getUpdated()); work.setPreviousState(vm.getState()); + if (!Step.Done.equals(work.getStep()) && work.getTimesTried() >= _maxRetries) { + s_logger.warn("Giving up, retries max times for work: " + work); + work.setStep(Step.Done); + } } _haDao.update(work.getId(), work); } catch (final Throwable th) { diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index e82a210b779..13ed97ecbca 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -1010,6 +1010,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir NicProfile profile = new NicProfile(null, null); if (ipAddress != null) { + if (!(NetUtils.isValidIp(ipAddress) || NetUtils.isValidIpv6(ipAddress))) { + throw new InvalidParameterValueException("Invalid format for IP address parameter: " + ipAddress); + } profile = new NicProfile(ipAddress, null); } @@ -2874,6 +2877,19 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } profile.setDefaultNic(true); + if (!_networkModel.areServicesSupportedInNetwork(network.getId(), new Service[]{Service.UserData})) { + if ((userData != null) && (!userData.isEmpty())) { + throw new InvalidParameterValueException("Unable to deploy VM as UserData is provided while deploying the VM, but there is no support for " + Network.Service.UserData.getName() + " service in the default network " + network.getId()); + } + + if ((sshPublicKey != null) && (!sshPublicKey.isEmpty())) { + throw new InvalidParameterValueException("Unable to deploy VM as SSH keypair is provided while deploying the VM, but there is no support for " + Network.Service.UserData.getName() + " service in the default network " + network.getId()); + } + + if (template.getEnablePassword()) { + throw new InvalidParameterValueException("Unable to deploy VM as template " + template.getId() + " is password enabled, but there is no support for " + Network.Service.UserData.getName() + " service in the default network " + network.getId()); + } + } } networks.add(new Pair(network, profile));