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, 26 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/83/34983/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..7436682 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 @@ -20,7 +20,13 @@ this.rangeStart = rangeStart; this.rangeEnd = rangeEnd; - int numberOfMacs = (int) (rangeEnd - rangeStart) + 1; + long numberOfMacsLong = (rangeEnd - rangeStart) + 1; + if (numberOfMacsLong > Integer.MAX_VALUE) { + throw new IllegalArgumentException(); + } + + 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..230db1f 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,17 @@ 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)); + + } } 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..917867e 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 @@ -54,7 +54,7 @@ return clipMultiCastsFromRanges(disjointRanges.getRanges()); } - private static Collection<LongRange> clipMultiCastsFromRanges(Collection<LongRange> ranges) { + public static Collection<LongRange> clipMultiCastsFromRanges(Collection<LongRange> ranges) { final Collection<LongRange> result = new ArrayList<>(); for (LongRange range : ranges) { final LongRange clippedRange = clipRange(range); @@ -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/34983 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4bbf719deb47c795862a03deb515283b1eae9a0e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
