If you see the code above try block, placeholder is getting set only when VmJobEnabled.value() is true.
so, doing a not null check on placeholder will imply that VmJobEnabled.value() is true. Here is the complete code block 755<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l755> VmWorkJobVO placeHolder = null; 756<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l756> if (VmJobEnabled.value()) { 757<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l757> VirtualMachine vm = _vmDao.findByUuid(vmUuid); 758<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l758> placeHolder = createPlaceHolderWork(vm.getId()); 759<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l759> } 760<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l760> try { 761<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l761> orchestrateStart(vmUuid, params, planToDeploy, planner); 762<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l762> } finally { 763<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l763> if (placeHolder != null) { 764<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l764> _workJobDao.expunge(placeHolder.getId()); 765<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l765> } 766<https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blob;f=engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java;h=2fd7a522abfc8a814e0e78e9490b5e510e43f945;hb=85bed5fa#l766> } ~Rajani On 02-Jul-2014, at 7:50 pm, Santhosh Edukulla <santhosh.eduku...@citrix.com<mailto:santhosh.eduku...@citrix.com>> wrote: Rajani, One note though: Here, null check should be for placeHolder value, I believe we still need "VmJobEnabled.value()", otherwise the respective behavior may get changed, here null dereference is happening for placeHolder value, so may be changing like "if ( (VmJobEnabled.value() && (placeHolder!=null)) correct? thoughts? if (VmJobEnabled.value()) + if (placeHolder != null) { Thanks! Santhosh ________________________________________ From: seb...@apache.org<mailto:seb...@apache.org> [seb...@apache.org<mailto:seb...@apache.org>] Sent: Wednesday, July 02, 2014 10:07 AM To: comm...@cloudstack.apache.org<mailto:comm...@cloudstack.apache.org> Subject: git commit: updated refs/heads/4.3 to 85bed5f Repository: cloudstack Updated Branches: refs/heads/4.3 44c8e1f67 -> 85bed5fa2 COVERITY: Fixed issues reported by coverity NPEs, unwritten field access and self assignment Signed-off-by: Sebastien Goasguen <run...@gmail.com<mailto:run...@gmail.com>> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/85bed5fa Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/85bed5fa Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/85bed5fa Branch: refs/heads/4.3 Commit: 85bed5fa2b8d61a867e2d53b2402eced0c4b6704 Parents: 44c8e1f Author: Rajani Karuturi <rajanikarut...@gmail.com<mailto:rajanikarut...@gmail.com>> Authored: Tue Jun 10 13:52:43 2014 +0530 Committer: Sebastien Goasguen <run...@gmail.com<mailto:run...@gmail.com>> Committed: Wed Jul 2 16:07:06 2014 +0200 ---------------------------------------------------------------------- .../com/cloud/vm/VirtualMachineManagerImpl.java | 43 +++++++++++--------- 1 file changed, 24 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/85bed5fa/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---------------------------------------------------------------------- diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index b30fc16..2fd7a52 100755 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -760,8 +760,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateStart(vmUuid, params, planToDeploy, planner); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = startVmThroughJobQueue(vmUuid, params, planToDeploy); @@ -1311,8 +1312,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateStop(vmUuid, cleanUpEvenIfUnableToStop); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { @@ -1405,7 +1407,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (s_logger.isDebugEnabled()) { s_logger.debug("Unable to transition the state but we're moving on because it's forced stop"); } - if (state == State.Starting || state == State.Migrating) { + if ((state == State.Starting) || (state == State.Migrating) || (state == State.Stopping)) { if (work != null) { doCleanup = true; } else { @@ -1414,8 +1416,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } throw new CloudRuntimeException("Work item not found, We cannot stop " + vm + " when it is in state " + vm.getState()); } - } else if (state == State.Stopping) { - doCleanup = true; } if (doCleanup) { @@ -1619,8 +1619,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateStorageMigration(vmUuid, destPool); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = migrateVmStorageThroughJobQueue(vmUuid, destPool); @@ -1711,8 +1712,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateMigrate(vmUuid, srcHostId, dest); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = migrateVmThroughJobQueue(vmUuid, srcHostId, dest); @@ -1993,8 +1995,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateMigrateWithStorage(vmUuid, srcHostId, destHostId, volumeToPool); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { @@ -2289,8 +2292,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateReboot(vmUuid, params); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = rebootVmThroughJobQueue(vmUuid, params); @@ -3104,12 +3108,11 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac public String platform; - @SuppressWarnings("unchecked") public AgentVmInfo(String name, VMInstanceVO vm, State state, String host, String platform) { this.name = name; this.state = state; this.vm = vm; - hostUuid = host; + this.hostUuid = host; this.platform = platform; } @@ -3224,8 +3227,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { return orchestrateAddVmToNetwork(vm, network, requested); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = addVmToNetworkThroughJobQueue(vm, network, requested); @@ -3235,7 +3239,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } catch (InterruptedException e) { throw new RuntimeException("Operation is interrupted", e); } catch (java.util.concurrent.ExecutionException e) { - throw new RuntimeException("Execution excetion", e); + throw new RuntimeException("Execution exception", e); } Object jobException = _jobMgr.unmarshallResultObject(outcome.getJob()); @@ -3336,8 +3340,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { return orchestrateRemoveNicFromVm(vm, nic); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { @@ -3583,8 +3588,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { orchestrateMigrateForScale(vmUuid, srcHostId, dest, oldSvcOfferingId); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = migrateVmForScaleThroughJobQueue(vmUuid, srcHostId, dest, oldSvcOfferingId); @@ -3842,8 +3848,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { return orchestrateReConfigureVm(vmUuid, oldServiceOffering, reconfiguringOnExistingHost); } finally { - if (VmJobEnabled.value()) + if (placeHolder != null) { _workJobDao.expunge(placeHolder.getId()); + } } } else { Outcome<VirtualMachine> outcome = reconfigureVmThroughJobQueue(vmUuid, oldServiceOffering, reconfiguringOnExistingHost); @@ -3894,7 +3901,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac work.setStep(Step.Prepare); work.setResourceType(ItWorkVO.ResourceType.Host); work.setResourceId(vm.getHostId()); - work = _workDao.persist(work); + _workDao.persist(work); boolean success = false; try { if (reconfiguringOnExistingHost) { @@ -3916,8 +3923,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } catch (AgentUnavailableException e) { throw e; } finally { - // work.setStep(Step.Done); - //_workDao.update(work.getId(), work); if (!success) { _capacityMgr.releaseVmCapacity(vm, false, false, vm.getHostId()); // release the new capacity vm.setServiceOfferingId(oldServiceOffering.getId());
