Arik Hadas has uploaded a new change for review.

Change subject: core: change the way run vm once override vm parameters
......................................................................

core: change the way run vm once override vm parameters

Before this change, in order to allow run VM once to override VM
parameters we extracted the relevant parts from RunVmCommand#initVm to
separate methods and override them in RunVmOnceCommand. The problem
with this approach is that we end up with tiny methods in RunVmCommand
and even worse, people are adding things that should be overridden by
RunVmOnceCommand to irrelevant methods (like setting the kernel-url
inside RunVmOnceCommand#refreshBootParameters, and not adding another
method that would be overridden).

A better approach, implemented in this patch, is not to extract things
to separate methods just to allow to override them, but to override the
whole RunVmCommand#initVm in RunVmOnceCommand and then change the things
that should be changed. It is a bit less efficient because first we put
the values in RunVmCommand and then change them, but those operations
are simple and there are just few such VM properties that should be
changed.

Change-Id: If6b578241863ab0a5f2ca16b4b94828cf3e029c1
Signed-off-by: Arik Hadas <[email protected]>
---
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/RunVmOnceCommand.java
2 files changed, 15 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/21/22221/17

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 7e4d280..11fe36a 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
@@ -155,14 +155,6 @@
     }
 
     /**
-     * Sets up the command specific boot parameters. This method is not 
expected to be
-     * extended, however it can be overridden (e.g. the children will not call 
the super)
-     */
-    protected void refreshBootParameters(RunVmParams runVmParameters) {
-        getVm().setBootSequence(getVm().getDefaultBootSequence());
-    }
-
-    /**
      * Returns the full path file name of Iso file. If the Iso file has prefix 
of Iso://, then we set the prefix to the
      * path of the domain on the Iso Domain server<BR/>
      * otherwise, returns the original name.<BR/>
@@ -617,6 +609,7 @@
         VmHandler.updateVmGuestAgentVersion(getVm());
 
         getVm().setRunOnce(false);
+        getVm().setBootSequence(getVm().getDefaultBootSequence());
         getVm().setCpuName(getVdsGroup().getcpu_name());
 
         if (!getVm().getInterfaces().isEmpty()) {
@@ -626,9 +619,6 @@
         if (getFlow() != RunVmFlow.DEHIBERNATE) {
             getVm().setHibernationVolHandle(getMemoryFromSnapshot());
         }
-
-        // reevaluate boot parameters if VM was executed with 'run once'
-        refreshBootParameters(getParameters());
     }
 
     protected void fetchVmDisksFromDb() {
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 c33e2f1..9b90d13 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
@@ -11,7 +11,6 @@
 import org.ovirt.engine.core.bll.quota.QuotaStorageDependent;
 import org.ovirt.engine.core.common.FeatureSupported;
 import org.ovirt.engine.core.common.action.RunVmOnceParams;
-import org.ovirt.engine.core.common.action.RunVmParams;
 import org.ovirt.engine.core.common.action.SysPrepParams;
 import org.ovirt.engine.core.common.businessentities.DiskImage;
 import org.ovirt.engine.core.common.businessentities.VDS;
@@ -22,8 +21,8 @@
 
 @NonTransactiveCommandAttribute
 public class RunVmOnceCommand<T extends RunVmOnceParams> extends 
RunVmCommand<T> implements QuotaStorageDependent {
-    public RunVmOnceCommand(T runVmParams) {
-        super(runVmParams);
+    public RunVmOnceCommand(T parameters) {
+        super(parameters);
     }
 
     @Override
@@ -47,19 +46,21 @@
     }
 
     /**
-     * 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')
+     * Extends {@link RunVmCommand#initVm()} and override the VM attributes
+     * that need to set differently when running it in 'run once' mode
      */
     @Override
-    protected void refreshBootParameters(RunVmParams runVmParameters) {
-        getVm().setInitrdUrl(runVmParameters.getinitrd_url());
-        getVm().setKernelUrl(runVmParameters.getkernel_url());
-        getVm().setKernelParams(runVmParameters.getkernel_params());
-        getVm().setCustomProperties(runVmParameters.getCustomProperties());
+    protected void initVm() {
+        super.initVm();
 
-        getVm().setBootSequence((runVmParameters.getBootSequence() != null) ?
-                runVmParameters.getBootSequence() :
-                getVm().getDefaultBootSequence());
+        getVm().setInitrdUrl(getParameters().getinitrd_url());
+        getVm().setKernelUrl(getParameters().getkernel_url());
+        getVm().setKernelParams(getParameters().getkernel_params());
+        getVm().setCustomProperties(getParameters().getCustomProperties());
+
+        if (getParameters().getBootSequence() != null) {
+            getVm().setBootSequence(getParameters().getBootSequence());
+        }
 
         getVm().setRunOnce(true);
     }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If6b578241863ab0a5f2ca16b4b94828cf3e029c1
Gerrit-PatchSet: 17
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to