You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by se...@apache.org on 2014/07/02 16:07:26 UTC
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 <ru...@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 <ra...@gmail.com>
Authored: Tue Jun 10 13:52:43 2014 +0530
Committer: Sebastien Goasguen <ru...@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());
RE: git commit: updated refs/heads/4.3 to 85bed5f
Posted by Santhosh Edukulla <sa...@citrix.com>.
Though logically it is correct, but still it makes sense to use expunge based upon the business condition i.e., vmjobenabled.value() , we are using intermediate statement reference to expunge which may do the task, but the logic of expunge when to expunge or when not to expunge is based on vmjobenabled.value() i believe, as well, there is a chance in between placeholder can be null but vmjobenabled.value() can be still be true, in that case we may want to throw some exception.
Santhosh
________________________________________
From: Rajani Karuturi [Rajani.Karuturi@citrix.com]
Sent: Thursday, July 03, 2014 4:39 AM
To: dev@cloudstack.apache.org
Subject: Re: git commit: updated refs/heads/4.3 to 85bed5f
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 <sa...@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: sebgoa@apache.org<ma...@apache.org> [sebgoa@apache.org<ma...@apache.org>]
Sent: Wednesday, July 02, 2014 10:07 AM
To: commits@cloudstack.apache.org<ma...@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 <ru...@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 <ra...@gmail.com>>
Authored: Tue Jun 10 13:52:43 2014 +0530
Committer: Sebastien Goasguen <ru...@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());
Re: git commit: updated refs/heads/4.3 to 85bed5f
Posted by Rajani Karuturi <Ra...@citrix.com>.
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 <sa...@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: sebgoa@apache.org<ma...@apache.org> [sebgoa@apache.org<ma...@apache.org>]
Sent: Wednesday, July 02, 2014 10:07 AM
To: commits@cloudstack.apache.org<ma...@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 <ru...@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 <ra...@gmail.com>>
Authored: Tue Jun 10 13:52:43 2014 +0530
Committer: Sebastien Goasguen <ru...@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());
FW: git commit: updated refs/heads/4.3 to 85bed5f
Posted by Santhosh Edukulla <sa...@citrix.com>.
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: sebgoa@apache.org [sebgoa@apache.org]
Sent: Wednesday, July 02, 2014 10:07 AM
To: commits@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 <ru...@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 <ra...@gmail.com>
Authored: Tue Jun 10 13:52:43 2014 +0530
Committer: Sebastien Goasguen <ru...@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());