[
https://issues.apache.org/jira/browse/CLOUDSTACK-10356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16442338#comment-16442338
]
ASF GitHub Bot commented on CLOUDSTACK-10356:
---------------------------------------------
DaanHoogland closed pull request #2573: [CLOUDSTACK-10356] Fix NPE in
Cloudstack found with NPEDetector
URL: https://github.com/apache/cloudstack/pull/2573
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 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/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 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
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.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
index dd039e54263..bd639fb76ba 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
@@ -2337,7 +2337,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);
}
@@ -2391,7 +2394,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
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 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
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 @@ 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
index d22dcbafbba..eabfb4337f4 100644
---
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
index a8dd225c54c..efbba165428 100644
--- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
+++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
@@ -479,7 +479,7 @@ 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.getId() != futureOwnerAccount.getId()) {
+ if (currentOwnerAccount !=null && currentOwnerAccount.getId() !=
futureOwnerAccount.getId()) {
ProjectAccountVO futureOwner =
_projectAccountDao.findByProjectIdAccountId(projectId,
futureOwnerAccount.getAccountId());
if (futureOwner == null) {
throw new InvalidParameterValueException("Account
" + newOwnerName +
diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
index 42bdd72af63..c862adae61f 100755
--- 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
index 583dcfc0ef5..baa3ba02562 100644
---
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);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Fix Some Potential NPE
> -----------------------
>
> Key: CLOUDSTACK-10356
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10356
> Project: CloudStack
> Issue Type: Bug
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Affects Versions: 4.12.0.0
> Reporter: lujie
> Priority: Major
> Attachments: CLOUDSTACK-10356_1.patch
>
>
> We have developed a static analysis tool
> [NPEDetector|https://github.com/lujiefsi/NPEDetector] to find some potential
> NPE. Our analysis shows that some callees may return null in corner case(e.g.
> node crash , IO exception), some of their callers have _!=null_ check but
> some do not have. In this issue we post a patch which can add !=null based
> on existed !=null check. For example:
> Callee GlobalLoadBalancingRulesServiceImpl#lookupGslbServiceProvider:
> {code:java}
> protected GslbServiceProvider lookupGslbServiceProvider() {
> return _gslbProviders.size() == 0 ? null : _gslbProviders.get(0);// may
> return null;
> }
> {code}
> Caller GlobalLoadBalancingRulesServiceImpl#checkGslbServiceEnabledInZone have
> _!=null_:
> {code:java}
> private boolean checkGslbServiceEnabledInZone(long zoneId, long
> physicalNetworkId) {
> GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
> if (gslbProvider == null) {
> throw new CloudRuntimeException("No GSLB provider is available");
> }
> return gslbProvider.isServiceEnabledInZone(zoneId, physicalNetworkId);
> }
> {code}
> but another
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig does
> not have !=null check:
> {code:java}
> GslbServiceProvider gslbProvider = lookupGslbServiceProvider();
> siteLb.setGslbProviderPublicIp(gslbProvider.getZoneGslbProviderPublicIp(dataCenterId,physicalNetworkId));
> .........{code}
> So we will add below code in non-(!=null) caller
> GlobalLoadBalancingRulesServiceImpl#applyGlobalLoadBalancerRuleConfig
> {code:java}
> if (gslbProvider == null) {
> throw new CloudRuntimeException("No GSLB provider is available");
> }
> {code}
> But due to we are not very familiar with CLOUDSTACK, hope some expert can
> review it.
> Thanks!!!!
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
