Martin Mucha has uploaded a new change for review. Change subject: core: error in calculating number of available macs / trimming mac pool ranges ......................................................................
core: error in calculating number of available macs / trimming mac pool ranges mac ranges was trimmed to inapproriate size; it was trimmed so that first and last mac representation can 'fit' into int datatype, but that rendered unable to fit *count* of macs in such range into int datatype, since that count would then be max_int+1 and that'd cause overflow. Bug-Url: https://bugzilla.redhat.com/1162077 Change-Id: I4bbf719deb47c795862a03deb515283b1eae9a0e Signed-off-by: Martin Mucha <mmu...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/Range.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/RangeTest.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java M backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/MacAddressRangeUtilsTest.java 4 files changed, 36 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/30/35130/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/Range.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/Range.java index 3a7dea5..6f3adfe 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/Range.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/Range.java @@ -4,6 +4,8 @@ import java.util.BitSet; import java.util.List; +import org.apache.commons.lang.Validate; + class Range { private final long rangeStart; private final long rangeEnd; @@ -20,7 +22,13 @@ this.rangeStart = rangeStart; this.rangeEnd = rangeEnd; - int numberOfMacs = (int) (rangeEnd - rangeStart) + 1; + long numberOfMacsLong = (rangeEnd - rangeStart) + 1; + Validate.isTrue(numberOfMacsLong <= Integer.MAX_VALUE, + String.format("Range too big; Range shouldn't be bigger than %1$s, but passed one " + + "contains %2$s elements.", Integer.MAX_VALUE, numberOfMacsLong)); + + int numberOfMacs = (int) numberOfMacsLong; + this.availableMacsCount = numberOfMacs; this.usedMacs = new BitSet(numberOfMacs); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/RangeTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/RangeTest.java index 77eb526..e8238b8 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/RangeTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/RangeTest.java @@ -5,8 +5,10 @@ import java.util.List; +import org.apache.commons.lang.math.LongRange; import org.junit.Before; import org.junit.Test; +import org.ovirt.engine.core.utils.MacAddressRangeUtils; public class RangeTest { @@ -108,4 +110,26 @@ public void testAllocateMacNoEnoughMacs() throws Exception { rangeOf10Macs.allocateMacs(NUMBER_OF_MACS + 1); } + + @Test + public void testRangeStartAndRangeStopAreInclusive() throws Exception { + assertThat(new Range(MAC_FROM_RANGE, MAC_FROM_RANGE).getAvailableCount(), is(1)); + } + + @Test + public void testRangeCanContainOnlyIntSizeNumberOfElements() throws Exception { + LongRange longRange = MacAddressRangeUtils.clipRange(new LongRange(0, Long.MAX_VALUE)); + Range range = new Range(longRange.getMinimumLong(), longRange.getMaximumLong()); + assertThat(range.getAvailableCount(), is(Integer.MAX_VALUE)); + } + + @Test(expected = IllegalArgumentException.class) + public void testTooBigRange() throws Exception { + new Range(0, Integer.MAX_VALUE); + } + + @Test + public void testMaxSizeRange() throws Exception { + new Range(0, Integer.MAX_VALUE - 1); + } } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java index e43863d..3beb76b 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java @@ -65,7 +65,7 @@ return result; } - private static LongRange clipRange(Range range) { + public static LongRange clipRange(Range range) { long rangeEnd = range.getMaximumLong(); long rangeStart = range.getMinimumLong(); @@ -75,7 +75,7 @@ trimmingOccurred = true; } - final long trimmedRangeEnd = Math.min(rangeStart + Integer.MAX_VALUE, rangeEnd); + final long trimmedRangeEnd = Math.min(rangeStart + Integer.MAX_VALUE - 1, rangeEnd); if (rangeEnd != trimmedRangeEnd) { rangeEnd = trimmedRangeEnd; trimmingOccurred = true; diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/MacAddressRangeUtilsTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/MacAddressRangeUtilsTest.java index 405b0c4..fdcbecf 100644 --- a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/MacAddressRangeUtilsTest.java +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/MacAddressRangeUtilsTest.java @@ -43,7 +43,7 @@ String start = "00:1a:4a:01:00:00"; String end = "00:FF:FF:FF:FF:FF"; final long expectedStart = MacAddressRangeUtils.macToLong(start); - final long expectedEnd = MacAddressRangeUtils.macToLong(start) + Integer.MAX_VALUE; + final long expectedEnd = MacAddressRangeUtils.macToLong(start) + Integer.MAX_VALUE - 1; testRange(start, end, expectedStart, expectedEnd); } -- To view, visit http://gerrit.ovirt.org/35130 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4bbf719deb47c795862a03deb515283b1eae9a0e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Martin Mucha <mmu...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
