Arik Hadas has uploaded a new change for review.

Change subject: core: change decrease pending memory & cpu places
......................................................................

core: change decrease pending memory & cpu places

We used to decrease the pending memory & CPU in 3 places:
1. RunVmCommandBase#onPoweringUp: we wanted to decrease the pending
resources as soon as we know the VM is running, i.e when it reaches the
POWERING_UP state
2. RunVmCommandBase#rerun: when the VM fails to run on a host, we
decreased the pending resources on that host
3. MigrateVmCommand#reportCompleted: in 48af8ddc we added it, so the
pending resources will be decreased when migration is done as well.

This patch changes the locations where the pending memory & CPU is
decreased such that locations #1 and #2 that mentioned above will
remain, and instead of the third location we're now decreasing them in
RunVmCommandBase#runningSucceeded & RunVmCommandBase#runningFailed.

reportCompleted is not supposed to be overridden and is not the place
where the reduction should be made. This method should only finish the
monitoring job/step. The runningSucceeded & runningFailed are the more
appropriate places for that.

A positive side-effect of this change is that we're now decreasing the
pending memory & CPU in other flows it should be decreased in, such as
when resume paused VM which was paused for more than 1 minute.

Change-Id: I8a3a10864d64e67a36942e01869b8f41c67f032a
Bug-Url: https://bugzilla.redhat.com/1049321
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
2 files changed, 16 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/21/28121/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
index e17d574..f6ea7b0 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
@@ -372,20 +372,6 @@
         }
     }
 
-    @Override
-    public void reportCompleted() {
-        try {
-            super.reportCompleted();
-        }
-        finally {
-            // Decrement the pending counters here as this is the only place 
which
-            // is called consistently regardless of the migration result.
-            if (getVdsDestinationId() != null) {
-                decreasePendingVms(getVdsDestinationId());
-            }
-        }
-    }
-
     /**
      * Log that the migration had failed with the error code that is in the 
VDS and needs to be retrieved.
      */
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
index d436115..483bc2f 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
@@ -58,6 +58,7 @@
     protected VDS _destinationVds;
     private SnapshotsValidator snapshotsValidator=new SnapshotsValidator();
     private final List<Guid> runVdsList = new ArrayList<Guid>();
+    private Guid lastDecreasedVds;
 
     protected RunVmCommandBase(Guid commandId) {
         super(commandId);
@@ -82,8 +83,7 @@
 
     @Override
     public void rerun() {
-        Guid vdsId = getDestinationVds() != null ? getDestinationVds().getId() 
: getCurrentVdsId();
-        decreasePendingVms(vdsId);
+        decreasePendingVms();
 
         setSucceeded(false);
         setVm(null);
@@ -131,6 +131,7 @@
 
     protected void runningFailed() {
         try {
+            decreasePendingVms();
             
Backend.getInstance().getResourceManager().RemoveAsyncRunningCommand(getVmId());
             _isRerun = false;
             setSucceeded(false);
@@ -161,6 +162,7 @@
     @Override
     public void runningSucceded() {
         try {
+            decreasePendingVms();
             setSucceeded(true);
             setActionReturnValue(VMStatus.Up);
             log();
@@ -232,7 +234,7 @@
     }
 
     @Override
-    public void reportCompleted() {
+    public final void reportCompleted() {
         try {
             ExecutionContext executionContext = getExecutionContext();
             if (executionContext != null && executionContext.isMonitored()
@@ -300,7 +302,16 @@
         return true;
     }
 
-    protected void decreasePendingVms(Guid vdsId) {
+    protected void decreasePendingVms() {
+        decreasePendingVms(getCurrentVdsId());
+    }
+
+    private void decreasePendingVms(Guid vdsId) {
+        if (vdsId == null || vdsId.equals(lastDecreasedVds)) {
+            // do not decrease twice..
+            return;
+        }
+
         VM vm = getVm();
         decreasePendingVms(vdsId, vm.getNumOfCpus(), vm.getMinAllocatedMem(), 
vm.getName());
     }
@@ -309,6 +320,7 @@
         getVdsDynamicDao().updatePartialVdsDynamicCalc(vdsId, 0, -numOfCpus, 
-minAllocatedMem, 0, 0);
         getBlockingQueue(vdsId).offer(Boolean.TRUE);
 
+        lastDecreasedVds = vdsId;
         log.debugFormat("Decreasing vds {0} pending vcpu count by {1} and vmem 
size by {2} (Vm: {3})",
                 vdsId, numOfCpus, minAllocatedMem, vmName);
     }


-- 
To view, visit http://gerrit.ovirt.org/28121
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a3a10864d64e67a36942e01869b8f41c67f032a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Arik Hadas <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to