Shmuel Leib Melamud has uploaded a new change for review.

Change subject: webadmin: do not persist RunOnce boot sequence accidentally
......................................................................

webadmin: do not persist RunOnce boot sequence accidentally

VmDeviceCommonUtils.updateVmDevicesBootOrder() method was using RunOnce
boot sequence if vm.isRunOnce() is true. When called from
VmDeviceUtils.updateBootOrderInVmDeviceAndStoreToDB() this caused
RunOnce boot sequence to be persisted when attaching/detaching disks,
adding/removing NICs etc.

Do avoid this undesired behavior,
VmDeviceCommonUtils.updateVmDevicesBootOrder() is no more using RunOnce
boot sequence. The RunOnce boot sequence is passed to it only from
VmInfoBuilder.buildVmBootSequence() that is called when VM is started.
The result is used to boot the VM and never persisted.

Note that VmDynamic.bootSequence is still persisted, this is done in
another place.

Change-Id: Ibfa685f8d81b67eff41234a450185879f5bbccfd
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1209038
Signed-off-by: Shmuel Melamud <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceCommonUtils.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
3 files changed, 29 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/60/39560/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
index 9c0102b..69b768b 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
@@ -661,7 +661,8 @@
     }
 
     /**
-     * Updates boot order in vm device according to the BootSequence enum 
value.
+     * Updates boot order in all VM devices according to the default boot 
sequence.
+     *
      * @param vmBase
      * @return the updated VmDevice list
      */
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceCommonUtils.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceCommonUtils.java
index 37e0055..e754a2a 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceCommonUtils.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceCommonUtils.java
@@ -83,15 +83,33 @@
     }
 
     /**
-     * updates given devices boot order
+     * Updates given devices boot order in accordance with default boot 
sequence.
      *
+     * @param vm
      * @param devices
-     * @param bootSequence
      * @param isOldCluster
      */
-    public static void updateVmDevicesBootOrder(VM vm,
+    public static void updateVmDevicesBootOrder(
+            VM vm,
             List<VmDevice> devices,
             boolean isOldCluster) {
+        updateVmDevicesBootOrder(vm, vm.getDefaultBootSequence(), devices, 
isOldCluster);
+    }
+
+    /**
+     * Updates given devices boot order in accordance with bootSequence given.
+     *
+     * @param vm
+     * @param bootSequence
+     * @param devices
+     * @param isOldCluster
+     */
+    public static void updateVmDevicesBootOrder(
+            VM vm,
+            BootSequence bootSequence,
+            List<VmDevice> devices,
+            boolean isOldCluster) {
+
         int bootOrder = 0;
 
         // reset current boot order of all relevant devices before recomputing 
it.
@@ -101,8 +119,7 @@
                 device.setBootOrder(0);
             }
         }
-        BootSequence bootSequence =
-                (vm.isRunOnce()) ? vm.getBootSequence() : 
vm.getDefaultBootSequence();
+
         switch (bootSequence) {
         case C:
             bootOrder = setDiskBootOrder(vm, devices, bootOrder, isOldCluster);
@@ -238,9 +255,9 @@
 
     /**
      * updates disk boot order
-     * snapshot disk devices always will have lower priority than regulary 
attached disks.
+     * snapshot disk devices always will have lower priority than regular 
attached disks.
      *
-     * @param vmBase
+     * @param vm
      * @param devices
      * @param bootOrder
      * @param isOldCluster
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
index f51fee1..cde5c4a 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java
@@ -571,7 +571,9 @@
         // Check if boot sequence in parameters is different from default boot 
sequence
         if (managedDevices != null) {
             // recalculate boot order from source devices and set it to target 
devices
-            VmDeviceCommonUtils.updateVmDevicesBootOrder(vm,
+            VmDeviceCommonUtils.updateVmDevicesBootOrder(
+                    vm,
+                    vm.isRunOnce() ? vm.getBootSequence() : 
vm.getDefaultBootSequence(),
                     managedDevices,
                     
VmDeviceCommonUtils.isOldClusterVersion(vm.getVdsGroupCompatibilityVersion()));
             for (VmDevice vmDevice : managedDevices) {


-- 
To view, visit https://gerrit.ovirt.org/39560
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibfa685f8d81b67eff41234a450185879f5bbccfd
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to