From 4c42aafae0d3286a38d796a7c69a6aec6810cf79 Mon Sep 17 00:00:00 2001 From: brett <1099446733@qq.com> Date: Thu, 26 Apr 2018 21:03:46 +0800 Subject: [PATCH] [CLOUDSTACK-10356] Fix NPE in Cloudstack found with NPEDetector (#2573) * fix https://issues.apache.org/jira/browse/CLOUDSTACK-10356 * del patch file * Update ResourceCountDaoImpl.java * fix some format * fix code * fix error message in VolumeOrchestrator * add check null stmt * del import unuse class * use BooleanUtils to check Boolean * fix error message * delete unuse function * delete the deprecated function updateDomainCount * add error log and throw exception in ProjectManagerImpl.java --- .../engine/orchestration/VolumeOrchestrator.java | 3 +++ .../cloud/configuration/dao/ResourceCountDao.java | 3 --- .../configuration/dao/ResourceCountDaoImpl.java | 10 ---------- .../cloud/deploy/ImplicitDedicationPlanner.java | 12 +++++++----- .../kvm/resource/LibvirtComputingResource.java | 10 ++++++++-- .../kvm/storage/LibvirtStorageAdaptor.java | 4 +++- .../router/VirtualNetworkApplianceManagerImpl.java | 6 ++++++ .../VpcVirtualNetworkApplianceManagerImpl.java | 14 ++++++++------ .../com/cloud/projects/ProjectManagerImpl.java | 4 ++++ .../com/cloud/template/TemplateManagerImpl.java | 5 ++++- .../gslb/GlobalLoadBalancingRulesServiceImpl.java | 9 ++++++--- 11 files changed, 49 insertions(+), 31 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index f02fdc495f9..c8279ff3f99 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -499,6 +499,9 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati // Find a suitable storage to create volume on StoragePool destPool = findStoragePool(dskCh, dc, pod, clusterId, null, vm, avoidPools); + if (destPool == null) { + throw new CloudRuntimeException("Failed to find a suitable storage pool to create Volume in the pod/cluster of the provided VM "+ vm.getUuid()); + } DataStore destStore = dataStoreMgr.getDataStore(destPool.getId(), DataStoreRole.Primary); AsyncCallFuture future = volService.copyVolume(volume, destStore); diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java index b5a75d196fa..28f2a536071 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java @@ -39,9 +39,6 @@ public interface ResourceCountDao extends GenericDao { */ void setResourceCount(long ownerId, ResourceOwnerType ownerType, ResourceType type, long count); - @Deprecated - void updateDomainCount(long domainId, ResourceType type, boolean increment, long delta); - boolean updateById(long id, boolean increment, long delta); void createResourceCounts(long ownerId, ResourceOwnerType ownerType); diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index 56261337faf..dbf2228183b 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -120,16 +120,6 @@ public class ResourceCountDaoImpl extends GenericDaoBase } } - @Override - @Deprecated - public void updateDomainCount(long domainId, ResourceType type, boolean increment, long delta) { - delta = increment ? delta : delta * -1; - - ResourceCountVO resourceCountVO = findByOwnerAndType(domainId, ResourceOwnerType.Domain, type); - resourceCountVO.setCount(resourceCountVO.getCount() + delta); - update(resourceCountVO.getId(), resourceCountVO); - } - @Override public boolean updateById(long id, boolean increment, long delta) { delta = increment ? delta : delta * -1; diff --git a/plugins/deployment-planners/implicit-dedication/src/main/java/com/cloud/deploy/ImplicitDedicationPlanner.java b/plugins/deployment-planners/implicit-dedication/src/main/java/com/cloud/deploy/ImplicitDedicationPlanner.java index 5bad9226eed..45f16abd2af 100644 --- a/plugins/deployment-planners/implicit-dedication/src/main/java/com/cloud/deploy/ImplicitDedicationPlanner.java +++ b/plugins/deployment-planners/implicit-dedication/src/main/java/com/cloud/deploy/ImplicitDedicationPlanner.java @@ -39,6 +39,7 @@ import com.cloud.utils.DateUtil; import com.cloud.utils.NumbersUtil; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachineProfile; +import org.springframework.util.CollectionUtils; public class ImplicitDedicationPlanner extends FirstFitPlanner implements DeploymentClusterPlanner { @@ -256,14 +257,15 @@ public class ImplicitDedicationPlanner extends FirstFitPlanner implements Deploy // Get the list of all the hosts in the given clusters List allHosts = new ArrayList(); - for (Long cluster : clusterList) { - List hostsInCluster = resourceMgr.listAllHostsInCluster(cluster); - for (HostVO hostVO : hostsInCluster) { + if (!CollectionUtils.isEmpty(clusterList)) { + for (Long cluster : clusterList) { + List hostsInCluster = resourceMgr.listAllHostsInCluster(cluster); + for (HostVO hostVO : hostsInCluster) { - allHosts.add(hostVO.getId()); + allHosts.add(hostVO.getId()); + } } } - // Go over all the hosts in the cluster and get a list of // 1. All empty hosts, not running any vms. // 2. Hosts running vms for this account and created by a service diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index fc5e5395b87..f26d8ded0a4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2339,7 +2339,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv disk.setCacheMode(DiskDef.DiskCacheMode.valueOf(volumeObjectTO.getCacheMode().toString().toUpperCase())); } } - + if (vm.getDevices() == null) { + s_logger.error("There is no devices for" + vm); + throw new RuntimeException("There is no devices for" + vm); + } vm.getDevices().addDevice(disk); } @@ -2393,7 +2396,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv + ") is " + nic.getType() + " traffic type. So, vsp-vr-ip " + vrIp + " is set in the metadata"); } } - + if (vm.getDevices() == null) { + s_logger.error("LibvirtVMDef object get devices with null result"); + throw new InternalErrorException("LibvirtVMDef object get devices with null result"); + } vm.getDevices().addDevice(getVifDriver(nic.getType(), nic.getName()).plug(nic, vm.getPlatformEmulator(), nicAdapter)); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 792fc6958cd..63f7872d05e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -522,7 +522,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { s_logger.debug("Checking path of existing pool " + poolname + " against pool we want to create"); StoragePool p = conn.storagePoolLookupByName(poolname); LibvirtStoragePoolDef pdef = getStoragePoolDef(conn, p); - + if (pdef == null) { + throw new CloudRuntimeException("Unable to parse the storage pool definition for storage pool " + poolname); + } String targetPath = pdef.getTargetPath(); if (targetPath != null && targetPath.equals(path)) { s_logger.debug("Storage pool utilizing path '" + path + "' already exists as pool " + poolname + diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 1985deaefa8..63587a89835 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -2139,6 +2139,12 @@ Configurable, StateListener