Findbugs : Fix a number of potential NPEs and minor findings
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/d4d49578 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/d4d49578 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/d4d49578 Branch: refs/heads/master Commit: d4d49578dc2dfc03aa7247811ba571da20c82cd5 Parents: e0a4b7c Author: Hugo Trippaers <[email protected]> Authored: Thu Feb 13 09:26:51 2014 +0100 Committer: Hugo Trippaers <[email protected]> Committed: Fri Feb 14 18:37:45 2014 +0100 ---------------------------------------------------------------------- .../vmware/VmwareServerDiscoverer.java | 6 -- .../vmware/manager/VmwareManagerImpl.java | 6 +- .../vmware/resource/VmwareResource.java | 23 +++---- .../resource/VmwareStorageProcessor.java | 72 ++++++++++---------- 4 files changed, 51 insertions(+), 56 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d4d49578/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java index e6ecbc9..5ab5af0 100755 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java @@ -42,14 +42,12 @@ import com.cloud.dc.ClusterDetailsDao; import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenter.NetworkType; import com.cloud.dc.DataCenterVO; -import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; import com.cloud.exception.DiscoveredWithErrorException; import com.cloud.exception.DiscoveryException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceInUseException; import com.cloud.host.HostVO; -import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; @@ -88,8 +86,6 @@ public class VmwareServerDiscoverer extends DiscovererBase implements Discoverer private static final Logger s_logger = Logger.getLogger(VmwareServerDiscoverer.class); @Inject - ClusterDao _clusterDao; - @Inject VmwareManager _vmwareMgr; @Inject AlertManager _alertMgr; @@ -98,8 +94,6 @@ public class VmwareServerDiscoverer extends DiscovererBase implements Discoverer @Inject ClusterDetailsDao _clusterDetailsDao; @Inject - HostDao _hostDao; - @Inject DataCenterDao _dcDao; @Inject ResourceManager _resourceMgr; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d4d49578/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index 4f443bb..67d3963 100755 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -382,10 +382,10 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw @Override public List<ManagedObjectReference> addHostToPodCluster(VmwareContext serviceContext, long dcId, Long podId, Long clusterId, String hostInventoryPath) throws Exception { - ManagedObjectReference mor = null; - if (serviceContext != null) { - mor = serviceContext.getHostMorByPath(hostInventoryPath); + if (serviceContext == null) { + throw new CloudRuntimeException("Invalid serviceContext"); } + ManagedObjectReference mor = serviceContext.getHostMorByPath(hostInventoryPath); String privateTrafficLabel = null; privateTrafficLabel = serviceContext.getStockObject("privateTrafficLabel"); if (privateTrafficLabel == null) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d4d49578/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java index c609686..182fe9f 100755 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -663,8 +663,6 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa public ExecutionResult createFileInVR(String routerIp, String filePath, String fileName, String content) { VmwareManager mgr = getServiceContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME); File keyFile = mgr.getSystemVMKeyFile(); - boolean result = true; - try { SshHelper.scpTo(routerIp, 3922, "root", keyFile, null, filePath, content.getBytes(), fileName, null); } catch (Exception e) { @@ -1065,9 +1063,6 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } private ExecutionResult prepareNetworkElementCommand(IpAssocCommand cmd) { - int i = 0; - String[] results = new String[cmd.getIpAddresses().length]; - VmwareContext context = getServiceContext(); try { VmwareHypervisorHost hyperHost = getHyperHost(context); @@ -1142,15 +1137,13 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } private ExecutionResult NetworkElementCommandnup(IpAssocCommand cmd) { - String[] results = new String[cmd.getIpAddresses().length]; - VmwareContext context = getServiceContext(); try { VmwareHypervisorHost hyperHost = getHyperHost(context); IpAddressTO[] ips = cmd.getIpAddresses(); String routerName = cmd.getAccessDetail(NetworkElementCommand.ROUTER_NAME); - String controlIp = VmwareResource.getRouterSshControlIp(cmd); + VmwareResource.getRouterSshControlIp(cmd); VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(routerName); @@ -4477,10 +4470,13 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa null); return new CreateAnswer(cmd, vol); } finally { - vmMo.detachAllDisks(); s_logger.info("Destroy dummy VM after volume creation"); - vmMo.destroy(); + if (vmMo != null) { + s_logger.warn("Unable to destroy a null VM ManagedObjectReference"); + vmMo.detachAllDisks(); + vmMo.destroy(); + } } } else { VirtualMachineMO vmTemplate = VmwareHelper.pickOneVmOnRunningHost(dcMo.findVmByNameAndLabel(cmd.getTemplateUrl()), true); @@ -4537,8 +4533,11 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa return new CreateAnswer(cmd, vol); } finally { s_logger.info("Destroy dummy VM after volume creation"); - vmMo.detachAllDisks(); - vmMo.destroy(); + if (vmMo != null) { + s_logger.warn("Unable to destroy a null VM ManagedObjectReference"); + vmMo.detachAllDisks(); + vmMo.destroy(); + } } } } catch (Throwable e) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d4d49578/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java index 2dbda5d..9d86b16 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -123,17 +123,17 @@ public class VmwareStorageProcessor implements StorageProcessor { } private void copyTemplateFromSecondaryToPrimary(VmwareHypervisorHost hyperHost, DatastoreMO datastoreMo, String secondaryStorageUrl, - String templatePathAtSecondaryStorage, String templateName, String templateUuid) throws Exception { + String templatePathAtSecondaryStorage, String templateName, String templateUuid) throws Exception { s_logger.info("Executing copyTemplateFromSecondaryToPrimary. secondaryStorage: " + secondaryStorageUrl + ", templatePathAtSecondaryStorage: " + - templatePathAtSecondaryStorage + ", templateName: " + templateName); + templatePathAtSecondaryStorage + ", templateName: " + templateName); String secondaryMountPoint = mountService.getMountPoint(secondaryStorageUrl); s_logger.info("Secondary storage mount point: " + secondaryMountPoint); String srcOVAFileName = - VmwareStorageLayoutHelper.getTemplateOnSecStorageFilePath(secondaryMountPoint, templatePathAtSecondaryStorage, templateName, - ImageFormat.OVA.getFileExtension()); + VmwareStorageLayoutHelper.getTemplateOnSecStorageFilePath(secondaryMountPoint, templatePathAtSecondaryStorage, templateName, + ImageFormat.OVA.getFileExtension()); String srcFileName = getOVFFilePath(srcOVAFileName); if (srcFileName == null) { @@ -163,7 +163,7 @@ public class VmwareStorageProcessor implements StorageProcessor { VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(vmName); if (vmMo == null) { String msg = - "Failed to import OVA template. secondaryStorage: " + secondaryStorageUrl + ", templatePathAtSecondaryStorage: " + templatePathAtSecondaryStorage + + "Failed to import OVA template. secondaryStorage: " + secondaryStorageUrl + ", templatePathAtSecondaryStorage: " + templatePathAtSecondaryStorage + ", templateName: " + templateName + ", templateUuid: " + templateUuid; s_logger.error(msg); throw new Exception(msg); @@ -218,7 +218,7 @@ public class VmwareStorageProcessor implements StorageProcessor { DatastoreMO primaryStorageDatastoreMo = new DatastoreMO(context, morDs); copyTemplateFromSecondaryToPrimary(hyperHost, primaryStorageDatastoreMo, secondaryStorageUrl, templateInfo.first(), templateInfo.second(), - templateUuidName); + templateUuidName); } else { s_logger.info("Template " + templateInfo.second() + " has already been setup, skip the template setup process in primary storage"); } @@ -238,7 +238,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } private boolean createVMLinkedClone(VirtualMachineMO vmTemplate, DatacenterMO dcMo, DatastoreMO dsMo, String vmdkName, ManagedObjectReference morDatastore, - ManagedObjectReference morPool) throws Exception { + ManagedObjectReference morPool) throws Exception { ManagedObjectReference morBaseSnapshot = vmTemplate.getSnapshotMor("cloud.template.base"); if (morBaseSnapshot == null) { @@ -257,7 +257,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } private boolean createVMFullClone(VirtualMachineMO vmTemplate, DatacenterMO dcMo, DatastoreMO dsMo, String vmdkName, ManagedObjectReference morDatastore, - ManagedObjectReference morPool) throws Exception { + ManagedObjectReference morPool) throws Exception { s_logger.info("creating full clone from template"); if (!vmTemplate.createFullClone(vmdkName, dcMo.getVmFolder(), morPool, morDatastore)) { @@ -293,7 +293,7 @@ public class VmwareStorageProcessor implements StorageProcessor { String vmdkFileBaseName = null; if (srcStore == null) { // create a root volume for blank VM (created from ISO) - String dummyVmName = this.hostService.getWorkerName(context, cmd, 0); + String dummyVmName = hostService.getWorkerName(context, cmd, 0); try { vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, dsMo, dummyVmName); @@ -312,10 +312,12 @@ public class VmwareStorageProcessor implements StorageProcessor { vmMo.detachDisk(volumeDatastorePath, false); } } finally { - vmMo.detachAllDisks(); - s_logger.info("Destroy dummy VM after volume creation"); - vmMo.destroy(); + if (vmMo != null) { + s_logger.warn("Unable to destroy a null VM ManagedObjectReference"); + vmMo.detachAllDisks(); + vmMo.destroy(); + } } } else { String templatePath = template.getPath(); @@ -459,7 +461,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } private Pair<String, String> copyVolumeToSecStorage(VmwareHostService hostService, VmwareHypervisorHost hyperHost, CopyCommand cmd, String vmName, String poolId, - String volumePath, String destVolumePath, String secStorageUrl, String workerVmName) throws Exception { + String volumePath, String destVolumePath, String secStorageUrl, String workerVmName) throws Exception { VirtualMachineMO workerVm = null; VirtualMachineMO vmMo = null; String exportName = UUID.randomUUID().toString().replace("-", ""); @@ -521,8 +523,8 @@ public class VmwareStorageProcessor implements StorageProcessor { Pair<String, String> result; result = - copyVolumeToSecStorage(hostService, hyperHost, cmd, vmName, primaryStorage.getUuid(), srcVolume.getPath(), destVolume.getPath(), destStore.getUrl(), - hostService.getWorkerName(context, cmd, 0)); + copyVolumeToSecStorage(hostService, hyperHost, cmd, vmName, primaryStorage.getUuid(), srcVolume.getPath(), destVolume.getPath(), destStore.getUrl(), + hostService.getWorkerName(context, cmd, 0)); VolumeObjectTO newVolume = new VolumeObjectTO(); newVolume.setPath(result.first() + File.separator + result.second()); return new CopyCmdAnswer(newVolume); @@ -577,7 +579,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } private Ternary<String, Long, Long> createTemplateFromVolume(VirtualMachineMO vmMo, String installPath, long templateId, String templateUniqueName, - String secStorageUrl, String volumePath, String workerVmName) throws Exception { + String secStorageUrl, String volumePath, String workerVmName) throws Exception { String secondaryMountPoint = mountService.getMountPoint(secStorageUrl); String installFullPath = secondaryMountPoint + "/" + installPath; @@ -611,7 +613,7 @@ public class VmwareStorageProcessor implements StorageProcessor { // 4 MB is the minimum requirement for VM memory in VMware Pair<VirtualMachineMO, String[]> cloneResult = - vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first())); + vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first())); clonedVm = cloneResult.first(); clonedVm.exportVm(secondaryMountPoint + "/" + installPath, templateUniqueName, false, false); @@ -674,7 +676,7 @@ public class VmwareStorageProcessor implements StorageProcessor { if (vmMo == null) { if (s_logger.isDebugEnabled()) { s_logger.debug("Unable to find the owner VM for CreatePrivateTemplateFromVolumeCommand on host " + hyperHost.getHyperHostName() + - ", try within datacenter"); + ", try within datacenter"); } vmMo = hyperHost.findVmOnPeerHyperHost(volume.getVmName()); @@ -694,8 +696,8 @@ public class VmwareStorageProcessor implements StorageProcessor { } Ternary<String, Long, Long> result = - createTemplateFromVolume(vmMo, template.getPath(), template.getId(), template.getName(), secondaryStoragePoolURL, volumePath, - hostService.getWorkerName(context, cmd, 0)); + createTemplateFromVolume(vmMo, template.getPath(), template.getId(), template.getName(), secondaryStoragePoolURL, volumePath, + hostService.getWorkerName(context, cmd, 0)); TemplateObjectTO newTemplate = new TemplateObjectTO(); newTemplate.setPath(result.first()); @@ -742,7 +744,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } private Ternary<String, Long, Long> createTemplateFromSnapshot(String installPath, String templateUniqueName, String secStorageUrl, String snapshotPath, - Long templateId) throws Exception { + Long templateId) throws Exception { //Snapshot path is decoded in this form: /snapshots/account/volumeId/uuid/uuid String backupSSUuid; String snapshotFolder; @@ -751,7 +753,7 @@ public class VmwareStorageProcessor implements StorageProcessor { backupSSUuid = snapshotPath.substring(index + 1).replace(".ova", ""); snapshotFolder = snapshotPath.substring(0, index); } else { - String[] tokens = snapshotPath.split(File.separator); + String[] tokens = snapshotPath.split(File.separatorChar == '\\' ? "\\\\" : File.separator); backupSSUuid = tokens[tokens.length - 1]; snapshotFolder = StringUtils.join(tokens, File.separator, 0, tokens.length - 1); } @@ -909,7 +911,7 @@ public class VmwareStorageProcessor implements StorageProcessor { // return Pair<String(divice bus name), String[](disk chain)> private Pair<String, String[]> exportVolumeToSecondaryStroage(VirtualMachineMO vmMo, String volumePath, String secStorageUrl, String secStorageDir, - String exportName, String workerVmName) throws Exception { + String exportName, String workerVmName) throws Exception { String secondaryMountPoint = mountService.getMountPoint(secStorageUrl); String exportPath = secondaryMountPoint + "/" + secStorageDir + "/" + exportName; @@ -937,7 +939,7 @@ public class VmwareStorageProcessor implements StorageProcessor { // 4 MB is the minimum requirement for VM memory in VMware Pair<VirtualMachineMO, String[]> cloneResult = - vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first())); + vmMo.cloneFromCurrentSnapshot(workerVmName, 0, 4, volumeDeviceInfo.second(), VmwareHelper.getDiskDeviceDatastore(volumeDeviceInfo.first())); clonedVm = cloneResult.first(); String disks[] = cloneResult.second(); @@ -953,7 +955,7 @@ public class VmwareStorageProcessor implements StorageProcessor { // Ternary<String(backup uuid in secondary storage), String(device bus name), String[](original disk chain in the snapshot)> private Ternary<String, String, String[]> backupSnapshotToSecondaryStorage(VirtualMachineMO vmMo, String installPath, String volumePath, String snapshotUuid, - String secStorageUrl, String prevSnapshotUuid, String prevBackupUuid, String workerVmName) throws Exception { + String secStorageUrl, String prevSnapshotUuid, String prevBackupUuid, String workerVmName) throws Exception { String backupUuid = UUID.randomUUID().toString(); Pair<String, String[]> snapshotInfo = exportVolumeToSecondaryStroage(vmMo, volumePath, secStorageUrl, installPath, backupUuid, workerVmName); @@ -1030,8 +1032,8 @@ public class VmwareStorageProcessor implements StorageProcessor { } backupResult = - backupSnapshotToSecondaryStorage(vmMo, destSnapshot.getPath(), srcSnapshot.getVolume().getPath(), snapshotUuid, secondaryStorageUrl, - prevSnapshotUuid, prevBackupUuid, hostService.getWorkerName(context, cmd, 1)); + backupSnapshotToSecondaryStorage(vmMo, destSnapshot.getPath(), srcSnapshot.getVolume().getPath(), snapshotUuid, secondaryStorageUrl, + prevSnapshotUuid, prevBackupUuid, hostService.getWorkerName(context, cmd, 1)); snapshotBackupUuid = backupResult.first(); success = (snapshotBackupUuid != null); @@ -1173,9 +1175,9 @@ public class VmwareStorageProcessor implements StorageProcessor { Map<String, String> details = disk.getDetails(); morDs = hostService.prepareManagedStorage(hyperHost, iScsiName, storageHost, storagePort, - details.get(DiskTO.CHAP_INITIATOR_USERNAME), details.get(DiskTO.CHAP_INITIATOR_SECRET), - details.get(DiskTO.CHAP_TARGET_USERNAME), details.get(DiskTO.CHAP_TARGET_SECRET), - volumeTO.getSize(), cmd); + details.get(DiskTO.CHAP_INITIATOR_USERNAME), details.get(DiskTO.CHAP_INITIATOR_SECRET), + details.get(DiskTO.CHAP_TARGET_USERNAME), details.get(DiskTO.CHAP_TARGET_SECRET), + volumeTO.getSize(), cmd); } else { morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, isManaged ? VmwareResource.getDatastoreName(iScsiName) : primaryStore.getUuid()); @@ -1187,7 +1189,7 @@ public class VmwareStorageProcessor implements StorageProcessor { throw new Exception(msg); } - DatastoreMO dsMo = new DatastoreMO(this.hostService.getServiceContext(null), morDs); + DatastoreMO dsMo = new DatastoreMO(hostService.getServiceContext(null), morDs); String datastoreVolumePath; if (isAttach) { @@ -1219,7 +1221,7 @@ public class VmwareStorageProcessor implements StorageProcessor { vmMo.detachDisk(datastoreVolumePath, false); if (isManaged) { - this.hostService.handleDatastoreAndVmdkDetach(iScsiName, storageHost, storagePort); + hostService.handleDatastoreAndVmdkDetach(iScsiName, storageHost, storagePort); } else { VmwareStorageLayoutHelper.syncVolumeToRootFolder(dsMo.getOwnerDatacenter().first(), dsMo, volumeTO.getPath()); } @@ -1369,7 +1371,7 @@ public class VmwareStorageProcessor implements StorageProcessor { String volumeUuid = UUID.randomUUID().toString().replace("-", ""); String volumeDatastorePath = dsMo.getDatastorePath(volumeUuid + ".vmdk"); - String dummyVmName = this.hostService.getWorkerName(context, cmd, 0); + String dummyVmName = hostService.getWorkerName(context, cmd, 0); try { s_logger.info("Create worker VM " + dummyVmName); vmMo = HypervisorHostHelper.createWorkerVM(hyperHost, dsMo, dummyVmName); @@ -1485,7 +1487,7 @@ public class VmwareStorageProcessor implements StorageProcessor { // this.hostService.handleDatastoreAndVmdkDetach(iScsiName, storageHost, storagePort); if (managedIqns != null && !managedIqns.isEmpty()) { - this.hostService.removeManagedTargetsFromCluster(managedIqns); + hostService.removeManagedTargetsFromCluster(managedIqns); } for (NetworkDetails netDetails : networks) { @@ -1573,7 +1575,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } private Long restoreVolumeFromSecStorage(VmwareHypervisorHost hyperHost, DatastoreMO primaryDsMo, String newVolumeName, String secStorageUrl, String secStorageDir, - String backupName) throws Exception { + String backupName) throws Exception { String secondaryMountPoint = mountService.getMountPoint(secStorageUrl); String srcOVAFileName = null;
