incubator-cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From chirad...@apache.org
Subject git commit: bug CS-16034 getRandomIp can return -1 unexpectedly also fixes unit test failures
Date Thu, 16 Aug 2012 18:39:55 GMT
Updated Branches:
  refs/heads/master d626e29fa -> 5b85edb96


bug CS-16034 getRandomIp can return -1 unexpectedly
also fixes unit test failures


Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/5b85edb9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/5b85edb9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/5b85edb9

Branch: refs/heads/master
Commit: 5b85edb96186583eae08d96e01c4ef4ee31e6222
Parents: d626e29
Author: Chiradeep Vittal <chiradeep@apache.org>
Authored: Thu Aug 16 11:28:11 2012 -0700
Committer: Chiradeep Vittal <chiradeep@apache.org>
Committed: Thu Aug 16 11:42:25 2012 -0700

----------------------------------------------------------------------
 .../com/cloud/network/guru/GuestNetworkGuru.java   |    3 +-
 utils/src/com/cloud/utils/net/NetUtils.java        |   36 +++++++++-----
 utils/test/com/cloud/utils/net/NetUtilsTest.java   |    4 +-
 3 files changed, 28 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5b85edb9/server/src/com/cloud/network/guru/GuestNetworkGuru.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/guru/GuestNetworkGuru.java b/server/src/com/cloud/network/guru/GuestNetworkGuru.java
index d1dc286..212de2f 100755
--- a/server/src/com/cloud/network/guru/GuestNetworkGuru.java
+++ b/server/src/com/cloud/network/guru/GuestNetworkGuru.java
@@ -20,6 +20,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Random;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeSet;
 
 import javax.ejb.Local;
@@ -239,7 +240,7 @@ public abstract class GuestNetworkGuru extends AdapterBase implements
NetworkGur
     public Ip4Address acquireIp4Address(Network network, Ip4Address requestedIp, String reservationId)
{
         List<String> ips = _nicDao.listIpAddressInNetwork(network.getId());
         String[] cidr = network.getCidr().split("/");
-        Set<Long> usedIps = new TreeSet<Long>();
+        SortedSet<Long> usedIps = new TreeSet<Long>();
 
         if (requestedIp != null && requestedIp.equals(network.getGateway())) {
             s_logger.warn("Requested ip address " + requestedIp + " is used as a gateway
address in network " + network);

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5b85edb9/utils/src/com/cloud/utils/net/NetUtils.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/net/NetUtils.java b/utils/src/com/cloud/utils/net/NetUtils.java
index e429022..1341338 100755
--- a/utils/src/com/cloud/utils/net/NetUtils.java
+++ b/utils/src/com/cloud/utils/net/NetUtils.java
@@ -32,6 +32,7 @@ import java.util.Formatter;
 import java.util.List;
 import java.util.Random;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.StringTokenizer;
 import java.util.TreeSet;
 import java.util.regex.Matcher;
@@ -651,39 +652,48 @@ public class NetUtils {
      * @param avoid set of ips to avoid
      * @return ip that is within the cidr range but not in the avoid set.  -1 if unable to
find one.
      */
-    public static long getRandomIpFromCidr(String startIp, int size, Set<Long> avoid)
{
+    public static long getRandomIpFromCidr(String startIp, int size, SortedSet<Long>
avoid) {
         return getRandomIpFromCidr(ip2Long(startIp), size, avoid);
 
     }
 
     /**
      * Given a cidr, this method returns an ip address within the range but
-     * is not in the avoid list.
+     * is not in the avoid list. 
+     * Note: the gateway address has to be specified in the avoid list
      * 
-     * @param startIp ip that the cidr starts with
+     * @param cidr ip that the cidr starts with
      * @param size size of the cidr
      * @param avoid set of ips to avoid
      * @return ip that is within the cidr range but not in the avoid set.  -1 if unable to
find one.
      */
-    public static long getRandomIpFromCidr(long cidr, int size, Set<Long> avoid) {
+    public static long getRandomIpFromCidr(long cidr, int size, SortedSet<Long> avoid)
{
         assert (size < 32) : "You do know this is not for ipv6 right?  Keep it smaller
than 32 but you have " + size;
 
         long startNetMask = ip2Long(getCidrNetmask(size));
-        long startIp = (cidr & startNetMask) + 2;
-        int range = 1 << (32 - size);
+        long startIp = (cidr & startNetMask) + 1; //exclude the first ip since it isnt
valid, e.g., 192.168.10.0
+        int range = 1 << (32 - size); //e.g., /24 = 2^8 = 256
+        range = range -1; //exclude end of the range since that is the broadcast address,
e.g., 192.168.10.255
 
-        if (avoid.size() > range) {
+        if (avoid.size() >= range) {
             return -1;
         }
-
-        for (int i = 0; i < range; i++) {
-            int next = _rand.nextInt(range);
-            if (!avoid.contains(startIp + next)) {
-                return startIp + next;
+                
+        //Reduce the range by the size of the avoid set
+        //e.g., cidr = 192.168.10.0, size = /24, avoid = 192.168.10.1, 192.168.10.20, 192.168.10.254
+        // range = 2^8 - 1 - 3 = 252
+        range = range - avoid.size();
+        int next = _rand.nextInt(range); //note: nextInt excludes last value
+        long ip = startIp + next;
+        for (Long avoidable : avoid) {
+            if (ip >= avoidable) {
+            	ip++;
+            } else {
+            	break;
             }
         }
 
-        return -1;
+        return ip;
     }
 
     public static String getIpRangeStartIpFromCidr(String cidr, long size) {

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/5b85edb9/utils/test/com/cloud/utils/net/NetUtilsTest.java
----------------------------------------------------------------------
diff --git a/utils/test/com/cloud/utils/net/NetUtilsTest.java b/utils/test/com/cloud/utils/net/NetUtilsTest.java
index bab1406..1eccba3 100644
--- a/utils/test/com/cloud/utils/net/NetUtilsTest.java
+++ b/utils/test/com/cloud/utils/net/NetUtilsTest.java
@@ -17,6 +17,7 @@
 package com.cloud.utils.net;
 
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeSet;
 
 import junit.framework.TestCase;
@@ -37,7 +38,7 @@ public class NetUtilsTest extends TestCase {
         ip = NetUtils.getRandomIpFromCidr(cidr, 8, new TreeSet<Long>());
         assertEquals("The ip " + NetUtils.long2Ip(ip) + " retrieved must be within the cidr
" + cidr + "/8", cidr.substring(0, 4), NetUtils.long2Ip(ip).substring(0, 4));
 
-        Set<Long> avoid = new TreeSet<Long>();
+        SortedSet<Long> avoid = new TreeSet<Long>();
         ip = NetUtils.getRandomIpFromCidr(cidr, 30, avoid);
         assertTrue("We should be able to retrieve an ip on the first call.", ip != -1);
         avoid.add(ip);
@@ -49,6 +50,7 @@ public class NetUtilsTest extends TestCase {
         assertTrue("We should be able to retrieve an ip on the third call.", ip != -1);
         assertTrue("ip returned is not in the avoid list", !avoid.contains(ip));
         avoid.add(ip);
+       
         ip = NetUtils.getRandomIpFromCidr(cidr, 30, avoid);
         assertEquals("This should be -1 because we ran out of ip addresses: " + ip, ip, -1);
     }


Mime
View raw message