[CLOUDSTACK-10356] Fix NPE in Cloudstack found with NPEDetector (#2573)

brettKK · rafaelweingartner · commit 4c42aafae0d3 · 2018-04-26T10:03:46.000-03:00
* 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
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
@@ -499,6 +499,9 @@ public VolumeInfo copyVolumeFromSecToPrimary(VolumeInfo volume, VirtualMachine v
 
         // 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<VolumeApiResult> 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
@@ -39,9 +39,6 @@ public interface ResourceCountDao extends GenericDao<ResourceCountVO, Long> {
      */
     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
@@ -120,16 +120,6 @@ public void setResourceCount(long ownerId, ResourceOwnerType ownerType, Resource
         }
     }
 
-    @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
@@ -39,6 +39,7 @@
 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 PlannerResourceUsage getResourceUsage(VirtualMachineProfile vmProfile, De
 
             // Get the list of all the hosts in the given clusters
             List<Long> allHosts = new ArrayList<Long>();
-            for (Long cluster : clusterList) {
-                List<HostVO> hostsInCluster = resourceMgr.listAllHostsInCluster(cluster);
-                for (HostVO hostVO : hostsInCluster) {
+            if (!CollectionUtils.isEmpty(clusterList)) {
+                for (Long cluster : clusterList) {
+                    List<HostVO> 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
@@ -2339,7 +2339,10 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
                     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 @@ private void createVif(final LibvirtVMDef vm, final NicTO nic, final String nicA
                         + ") 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
@@ -522,7 +522,9 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri
                     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
@@ -2139,6 +2139,12 @@ public boolean startRemoteAccessVpn(final Network network, final RemoteAccessVpn
             }
 
             Answer answer = cmds.getAnswer("users");
+            if (answer == null) {
+                s_logger.error("Unable to start vpn: unable add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
+                        + router.getInstanceName() + " due to null answer");
+                throw new ResourceUnavailableException("Unable to start vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
+                        + router.getInstanceName() + " due to null answer", DataCenter.class, router.getDataCenterId());
+            }
             if (!answer.getResult()) {
                 s_logger.error("Unable to start vpn: unable add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
                         + router.getInstanceName() + " due to " + answer.getDetails());
diff --git a/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
@@ -740,18 +740,20 @@ public boolean startRemoteAccessVpn(final RemoteAccessVpn vpn, final VirtualRout
             throw new AgentUnavailableException("Unable to send commands to virtual router ", router.getHostId(), e);
         }
         Answer answer = cmds.getAnswer("users");
-        if (!answer.getResult()) {
+        if (answer == null || !answer.getResult()) {
+            String errorMessage = (answer == null) ? "null answer object" : answer.getDetails();
             s_logger.error("Unable to start vpn: unable add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
-                    + router.getInstanceName() + " due to " + answer.getDetails());
+                    + router.getInstanceName() + " due to " + errorMessage);
             throw new ResourceUnavailableException("Unable to start vpn: Unable to add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId()
-            + " on domR: " + router.getInstanceName() + " due to " + answer.getDetails(), DataCenter.class, router.getDataCenterId());
+            + " on domR: " + router.getInstanceName() + " due to " + errorMessage, DataCenter.class, router.getDataCenterId());
         }
         answer = cmds.getAnswer("startVpn");
-        if (!answer.getResult()) {
+        if (answer == null || !answer.getResult()) {
+            String errorMessage = (answer == null) ? "null answer object" : answer.getDetails();
             s_logger.error("Unable to start vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: " + router.getInstanceName() + " due to "
-                    + answer.getDetails());
+                    + errorMessage);
             throw new ResourceUnavailableException("Unable to start vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
-                    + router.getInstanceName() + " due to " + answer.getDetails(), DataCenter.class, router.getDataCenterId());
+                    + router.getInstanceName() + " due to " + errorMessage, DataCenter.class, router.getDataCenterId());
         }
 
         return true;
diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
@@ -479,6 +479,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Resour
                 throw new InvalidParameterValueException("Unable to find account name=" + newOwnerName + " in domain id=" + project.getDomainId());
             }
             Account currentOwnerAccount = getProjectOwner(projectId);
+            if (currentOwnerAccount == null) {
+                s_logger.error("Unable to find the current owner for the project id=" + projectId);
+                throw new InvalidParameterValueException("Unable to find the current owner for the project id=" + projectId);
+            }
             if (currentOwnerAccount.getId() != futureOwnerAccount.getId()) {
                 ProjectAccountVO futureOwner = _projectAccountDao.findByProjectIdAccountId(projectId, futureOwnerAccount.getAccountId());
                 if (futureOwner == null) {
diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
@@ -1232,7 +1232,10 @@ private boolean attachISOToVM(long vmId, long isoId, boolean attach) {
 
         // prepare ISO ready to mount on hypervisor resource level
         TemplateInfo tmplt = prepareIso(isoId, vm.getDataCenterId(), vm.getHostId(), null);
-
+        if (tmplt == null) {
+            s_logger.error("Failed to prepare ISO ready to mount on hypervisor resource level");
+            throw new CloudRuntimeException("Failed to prepare ISO ready to mount on hypervisor resource level");
+        }
         String vmName = vm.getInstanceName();
 
         HostVO host = _hostDao.findById(vm.getHostId());
diff --git a/server/src/main/java/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImpl.java b/server/src/main/java/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImpl.java
@@ -650,9 +650,12 @@ private boolean applyGlobalLoadBalancerRuleConfig(long gslbRuleId, boolean revok
             SiteLoadBalancerConfig siteLb =
                 new SiteLoadBalancerConfig(gslbLbMapVo.isRevoke(), serviceType, ip.getAddress().addr(), Integer.toString(loadBalancer.getDefaultPortStart()),
                     dataCenterId);
-
-            siteLb.setGslbProviderPublicIp(lookupGslbServiceProvider().getZoneGslbProviderPublicIp(dataCenterId, physicalNetworkId));
-            siteLb.setGslbProviderPrivateIp(lookupGslbServiceProvider().getZoneGslbProviderPrivateIp(dataCenterId, physicalNetworkId));
+            GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
+            if (gslbProvider == null) {
+                throw new CloudRuntimeException("No GSLB provider is available");
+            }
+            siteLb.setGslbProviderPublicIp(gslbProvider.getZoneGslbProviderPublicIp(dataCenterId, physicalNetworkId));
+            siteLb.setGslbProviderPrivateIp(gslbProvider.getZoneGslbProviderPrivateIp(dataCenterId, physicalNetworkId));
             siteLb.setWeight(gslbLbMapVo.getWeight());
 
             zoneSiteLoadbalancerMap.put(network.getDataCenterId(), siteLb);

Original file line number	Diff line number	Diff line change
`@@ -2339,7 +2339,10 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {`
`2339`	`2339`	`disk.setCacheMode(DiskDef.DiskCacheMode.valueOf(volumeObjectTO.getCacheMode().toString().toUpperCase()));`
`2340`	`2340`	`}`
`2341`	`2341`	`}`
`2342`		`-`
	`2342`	`+ if (vm.getDevices() == null) {`
	`2343`	`+ s_logger.error("There is no devices for" + vm);`
	`2344`	`+ throw new RuntimeException("There is no devices for" + vm);`
	`2345`	`+ }`
`2343`	`2346`	`vm.getDevices().addDevice(disk);`
`2344`	`2347`	`}`
`2345`	`2348`
`@@ -2393,7 +2396,10 @@ private void createVif(final LibvirtVMDef vm, final NicTO nic, final String nicA`
`2393`	`2396`	`+ ") is " + nic.getType() + " traffic type. So, vsp-vr-ip " + vrIp + " is set in the metadata");`
`2394`	`2397`	`}`
`2395`	`2398`	`}`
`2396`		`-`
	`2399`	`+ if (vm.getDevices() == null) {`
	`2400`	`+ s_logger.error("LibvirtVMDef object get devices with null result");`
	`2401`	`+ throw new InternalErrorException("LibvirtVMDef object get devices with null result");`
	`2402`	`+ }`
`2397`	`2403`	`vm.getDevices().addDevice(getVifDriver(nic.getType(), nic.getName()).plug(nic, vm.getPlatformEmulator(), nicAdapter));`
`2398`	`2404`	`}`
`2399`	`2405`