Arik Hadas has uploaded a new change for review.

Change subject: core: remove RunVmCommandBase#getDestinationVds
......................................................................

core: remove RunVmCommandBase#getDestinationVds

In light of 84fc3c7 RunVmCommandBase#getDestinationVds is now used
only by classes which extend RunVmCommandBase and is used differently:
MigrateVm returns the VDS that the VM is going to migrate to, RunVm &
RunVmOnce return the predefined VDS on which the VM should run on.

This patch removes RunVmCommandBase#getDestinationVds method. In RunVm
it is replaced by getPredefinedDestinationVdsId method which reflect the
result better by its name and returns only the ID of the VDS.

In addition, minor improvement was made in
MigrateVmCommand#getDestinationVds, and calls to it where we only need
the ID of the VDS were replaced by calls to
MigrateVmCommand#getDestinationVdsId.

Change-Id: Ie1d8fb852a88203378b3bcd3f1d6f96e469e5cb9
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
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/RunVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
4 files changed, 24 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/55/24255/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 dc59923..b6344b9 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
@@ -69,11 +69,15 @@
                 .TranslateErrorTextSingle(migrationErrorCode.name(), true);
     }
 
-    @Override
     protected VDS getDestinationVds() {
-        if (cachedDestinationVds == null && destinationVdsId != null) {
+        if (destinationVdsId == null) {
+            return cachedDestinationVds = null;
+        }
+
+        if (cachedDestinationVds == null || 
!cachedDestinationVds.getId().equals(destinationVdsId)) {
             cachedDestinationVds = getVdsDAO().get(destinationVdsId);
         }
+
         return cachedDestinationVds;
     }
 
@@ -101,13 +105,12 @@
 
     protected void initVdss() {
         setVdsIdRef(getVm().getRunOnVds());
-        VDS destVds = getDestinationVds();
         Guid vdsToRunOn =
                 SchedulingManager.getInstance().schedule(getVdsGroup(),
                         getVm(),
                         getVdsBlackList(),
                         getVdsWhiteList(),
-                        destVds == null ? null : destVds.getId(),
+                        getDestinationVdsId(),
                         new ArrayList<String>(),
                         new VdsFreeMemoryChecker(this),
                         getCorrelationId());
@@ -116,9 +119,7 @@
             getRunVdssList().add(vdsToRunOn);
         }
         VmHandler.updateVmGuestAgentVersion(getVm());
-        // make _destinationVds null in order to refresh it from db in case it
-        // changed.
-        cachedDestinationVds = null;
+
         if (destinationVdsId != null && destinationVdsId.equals(Guid.Empty)) {
             throw new 
VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_CANT_ALLOC_VDS_MIGRATION);
         }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
index 883e67d..75bfa46 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
@@ -94,7 +94,6 @@
     /** This flag is used to indicate that the disks might be dirty since the 
memory
      *  from the active snapshot was restored so the memory should not be used 
*/
     private boolean memoryFromSnapshotIrrelevant;
-    private VDS cachedDestinationVds;
 
     public static final String ISO_PREFIX = "iso://";
     public static final String STATELESS_SNAPSHOT_DESCRIPTION = "stateless 
snapshot";
@@ -112,25 +111,8 @@
         initRunVmCommand();
     }
 
-    @Override
-    protected VDS getDestinationVds() {
-        if (cachedDestinationVds == null) {
-            Guid vdsId = null;
-
-            if (getVm().getDedicatedVmForVds() != null) {
-                vdsId = getVm().getDedicatedVmForVds();
-            }
-
-            // destination VDS ID is stronger than the dedicated VDS
-            if (getParameters().getDestinationVdsId() != null) {
-                vdsId = getParameters().getDestinationVdsId();
-            }
-
-            if (vdsId != null) {
-                cachedDestinationVds = getVdsDAO().get(vdsId);
-            }
-        }
-        return cachedDestinationVds;
+    protected Guid getPredefinedDestinationVdsId() {
+        return getVm().getDedicatedVmForVds();
     }
 
     private void initRunVmCommand() {
@@ -624,14 +606,12 @@
     }
 
     protected boolean getVdsToRunOn() {
-        // use destination vds or default vds or none
-        VDS destinationVds = getDestinationVds();
         Guid vdsToRunOn =
                 SchedulingManager.getInstance().schedule(getVdsGroup(),
                         getVm(),
                         getRunVdssList(),
                         getVdsWhiteList(),
-                        destinationVds == null ? null : destinationVds.getId(),
+                        getPredefinedDestinationVdsId(),
                         new ArrayList<String>(),
                         new VdsFreeMemoryChecker(this),
                         getCorrelationId());
@@ -754,7 +734,7 @@
                 getStoragePool(),
                 getRunVdssList(),
                 getVdsWhiteList(),
-                getDestinationVds() != null ? getDestinationVds().getId() : 
null,
+                getPredefinedDestinationVdsId(),
                 getVdsGroup())) {
             return false;
         }
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 9dff5bd..58a47a6 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
@@ -25,7 +25,6 @@
 import org.ovirt.engine.core.common.businessentities.LUNs;
 import org.ovirt.engine.core.common.businessentities.LunDisk;
 import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
-import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.businessentities.VmStatic;
@@ -66,8 +65,6 @@
     public RunVmCommandBase(T parameters) {
         super(parameters);
     }
-
-    protected abstract VDS getDestinationVds();
 
     public SnapshotsValidator getSnapshotsValidator() {
         return snapshotsValidator;
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
index 0d3c32d..effb235 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
@@ -47,6 +47,14 @@
         return true;
     }
 
+    @Override
+    protected Guid getPredefinedDestinationVdsId() {
+        // destination VDS ID is stronger than the dedicated VDS
+        return getParameters().getDestinationVdsId() != null ?
+            getParameters().getDestinationVdsId()
+            : super.getPredefinedDestinationVdsId();
+    }
+
     /**
      * Refresh the associated values of the VM boot parameters with the values 
from the command parameters. The method
      * is used when VM is reloaded from the DB while its parameters hasn't 
been persisted (e.g. when running 'as once')
@@ -112,11 +120,10 @@
 
     @Override
     protected List<Guid> getVdsWhiteList() {
-        VDS destinationVds = getDestinationVds();
-        if (destinationVds != null) {
-            return Arrays.asList(destinationVds.getId());
-        }
-        return super.getVdsWhiteList();
+        Guid predefinedDestinationVdsId = getPredefinedDestinationVdsId();
+        return predefinedDestinationVdsId != null ?
+                Arrays.asList(predefinedDestinationVdsId)
+                : super.getVdsWhiteList();
     }
 
     @Override


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie1d8fb852a88203378b3bcd3f1d6f96e469e5cb9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to