Tomas Jelinek has uploaded a new change for review. Change subject: engine: vm doesn't boot from cd when CD set in "edit" (#856806) ......................................................................
engine: vm doesn't boot from cd when CD set in "edit" (#856806) https://bugzilla.redhat.com/856806 Virtual Machines tab -> select VM -> Edit -> Boot Options -> select CD Than setup boot order to First device: CD, second NONE. The result is, that the VM will not boot from the CD attached. The problem was in RunVmCommand.AttachCd() which did set up the correct CD path only, when the getVm().getboot_sequence() == BootSequence.CD The problem is, that this condition is only true, when the boot sequence is first the Hard Disk (C), than the CD (D). The condition should be, that when it contains CD (D). Solved by adding the BootSequence.containsSubsequence method (the tests in BootSequenceTest) which checks if the specific sequence is a subsequence of the sequence. Change-Id: I61d39f8559bcffdc154b0b2c01d5303cf2ff1e1a Signed-off-by: Tomas Jelinek <tjeli...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BootSequence.java A backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/BootSequenceTest.java 3 files changed, 132 insertions(+), 16 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/77/8077/1 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 2d8a052..b176d91 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 @@ -287,7 +287,7 @@ /** * Handles the cd attachment. Set the VM CDPath to the ISO Path stored in the database (default) Call - * GuestToolsVersionTreatment to override CDPath by guest tools if needed. If the CD symbol ('C') is contained in + * GuestToolsVersionTreatment to override CDPath by guest tools if needed. If the CD symbol ('D') is contained in * the Boot Sequence (at any place) set again the CDPath to the ISO Path that was stored in the database, and we * assume that this CD is bootable. So , priorities are (from low to high) : 1)Default 2)Tools 3)Boot Sequence */ @@ -300,7 +300,7 @@ getVm().setCdPath(getVm().getiso_path()); GuestToolsVersionTreatment(); refreshBootSequenceParameter(getParameters()); - if (getVm().getboot_sequence() == BootSequence.CD) { + if (getVm().getboot_sequence() != null && getVm().getboot_sequence().containsSubsequence(BootSequence.D)) { getVm().setCdPath(getVm().getiso_path()); } getVm().setCdPath(ImagesHandler.cdPathWindowsToLinux(getVm().getCdPath(), getVm().getstorage_pool_id())); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BootSequence.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BootSequence.java index a752147..a6f2840 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BootSequence.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BootSequence.java @@ -1,6 +1,8 @@ package org.ovirt.engine.core.common.businessentities; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; /** @@ -9,23 +11,24 @@ public enum BootSequence { C(0), - DC(1), - N(2), - CDN(3), - CND(4), - DCN(5), - DNC(6), - NCD(7), - NDC(8), - CD(9), D(10), - CN(11), - DN(12), - NC(13), - ND(14); + N(2), + DC(1, D, C), + CDN(3, C, D, N), + CND(4, C, N, D), + DCN(5, D, C, N), + DNC(6, D, N, C), + NCD(7, N, C, D), + NDC(8, N, D, C), + CD(9, C, D), + CN(11, C, N), + DN(12, D, N), + NC(13, N, C), + ND(14, N, D); private int intValue; private static Map<Integer, BootSequence> mappings; + private final List<BootSequence> subsequences; static { mappings = new HashMap<Integer, BootSequence>(); @@ -34,8 +37,14 @@ } } - private BootSequence(int value) { + private BootSequence(int value, BootSequence... composedOf) { intValue = value; + if (composedOf == null || composedOf.length == 0) { + // leaf contains itself + this.subsequences = Arrays.asList(this); + } else { + this.subsequences = Arrays.asList(composedOf); + } } public int getValue() { @@ -45,4 +54,37 @@ public static BootSequence forValue(int value) { return mappings.get(value); } + + /** + * Returns true if and only if the subsequence is a subsequence of the sequence of this class. + * <p> + * For example, D is a subsequence of D, DN, CD..., DN is a subsequence of DNC etc + * <p> + * This method is implemented the hard way, because the Collections.indexOfSubList is not implemented in the GWT + * part. + */ + public boolean containsSubsequence(BootSequence subsequence) { + if (subsequence == null) { + return false; + } + + boolean subsequenceLonger = subsequence.subsequences.size() > subsequences.size(); + boolean subsequenceIsEmpty = subsequence.subsequences.size() == 0; + + if (subsequenceIsEmpty || subsequenceLonger) { + return false; + } + + int firstElementIndex = subsequences.indexOf(subsequence.subsequences.get(0)); + boolean firstElementNotPresent = firstElementIndex == -1; + boolean subsequenceTooLong = subsequences.size() - firstElementIndex < subsequence.subsequences.size(); + + if (firstElementNotPresent || subsequenceTooLong) { + return false; + } + + List<BootSequence> sublist = + subsequences.subList(firstElementIndex, firstElementIndex + subsequence.subsequences.size()); + return sublist.equals(subsequence.subsequences); + } } diff --git a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/BootSequenceTest.java b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/BootSequenceTest.java new file mode 100644 index 0000000..77f0afb --- /dev/null +++ b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/BootSequenceTest.java @@ -0,0 +1,74 @@ +package org.ovirt.engine.core.common.businessentities; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import org.junit.Test; + +public class BootSequenceTest { + + @Test + public void nothingContainsNull() { + assertFalse(BootSequence.C.containsSubsequence(null)); + } + + @Test + public void leafContainsItself() { + assertTrue(BootSequence.C.containsSubsequence(BootSequence.C)); + } + + @Test + public void threeSequence_oneSubsequence() { + assertTrue(BootSequence.CDN.containsSubsequence(BootSequence.C)); + } + + @Test + public void threeSequence_twoSubsequence() { + assertTrue(BootSequence.CDN.containsSubsequence(BootSequence.CD)); + } + + @Test + public void threeSequence_twoSubsequence_secondPosition() { + assertTrue(BootSequence.CDN.containsSubsequence(BootSequence.DN)); + } + + @Test + public void threeSequence_oneSubsequence_secondPosition() { + assertTrue(BootSequence.CDN.containsSubsequence(BootSequence.D)); + } + + @Test + public void threeSequence_oneSubsequence_thirdPosition() { + assertTrue(BootSequence.CDN.containsSubsequence(BootSequence.N)); + } + + @Test + public void threeSequence_threeSubsequence() { + assertTrue(BootSequence.CDN.containsSubsequence(BootSequence.CDN)); + } + + @Test + public void oneSequence_oneWrongSubsequence() { + assertFalse(BootSequence.C.containsSubsequence(BootSequence.D)); + } + + @Test + public void oneSequence_twoSubsequence() { + assertFalse(BootSequence.C.containsSubsequence(BootSequence.DC)); + } + + @Test + public void oneSequence_threeSubsequence() { + assertFalse(BootSequence.C.containsSubsequence(BootSequence.DCN)); + } + + @Test + public void twoSequence_twoSubsequence_incorrectOrder() { + assertFalse(BootSequence.CD.containsSubsequence(BootSequence.DC)); + } + + @Test + public void twoSequence_oneSubsequence_notInItAtAll() { + assertFalse(BootSequence.CD.containsSubsequence(BootSequence.N)); + } +} -- To view, visit http://gerrit.ovirt.org/8077 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I61d39f8559bcffdc154b0b2c01d5303cf2ff1e1a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
