Cloudstack-701 Support for non contiguous vlan ranges.

Signed-off-by: Abhinandan Prateek <aprateek@apache.org>

api/src/com/cloud/network/NetworkService.java

@@ -79,7 +79,7 @@ public interface NetworkService {
             Long startIndex, Long pageSize, String name);
 
     PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags,
-            String newVnetRangeString, String state);
+                                          String newVnetRangeString, String state, String removeVlan);
 
     boolean deletePhysicalNetwork(Long id);
 

api/src/com/cloud/network/PhysicalNetwork.java

@@ -18,6 +18,7 @@ package com.cloud.network;
 
 import java.util.List;
 
+import com.cloud.utils.Pair;
 import org.apache.cloudstack.api.Identity;
 import org.apache.cloudstack.api.InternalIdentity;
 
@@ -59,7 +60,9 @@ public interface PhysicalNetwork extends Identity, InternalIdentity {
 
     Long getDomainId();
 
-    String getVnet();
+    List<Pair<Integer,Integer>> getVnet();
+
+    String getVnetString();
 
     String getSpeed();
 

api/src/org/apache/cloudstack/api/ApiConstants.java

@@ -221,6 +221,7 @@ public class ApiConstants {
     public static final String VIRTUAL_MACHINE_ID = "virtualmachineid";
     public static final String VIRTUAL_MACHINE_IDS = "virtualmachineids";
     public static final String VLAN = "vlan";
+    public static final String REMOVE_VLAN="removevlan";
     public static final String VLAN_ID = "vlanid";
     public static final String VM_AVAILABLE = "vmavailable";
     public static final String VM_LIMIT = "vmlimit";

api/src/org/apache/cloudstack/api/command/admin/network/UpdatePhysicalNetworkCmd.java

@@ -54,6 +54,8 @@ public class UpdatePhysicalNetworkCmd extends BaseAsyncCmd {
 
     @Parameter(name=ApiConstants.VLAN, type=CommandType.STRING, description="the VLAN for the physical network")
     private String vlan;
+    @Parameter(name=ApiConstants.REMOVE_VLAN, type = CommandType.STRING, description ="The vlan range we want to remove")
+    private String removevlan;
 
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
@@ -79,6 +81,10 @@ public class UpdatePhysicalNetworkCmd extends BaseAsyncCmd {
         return vlan;
     }
 
+    public String getRemoveVlan(){
+        return removevlan;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
@@ -95,7 +101,7 @@ public class UpdatePhysicalNetworkCmd extends BaseAsyncCmd {
 
     @Override
     public void execute(){
-        PhysicalNetwork result = _networkService.updatePhysicalNetwork(getId(),getNetworkSpeed(), getTags(), getVlan(), getState());
+        PhysicalNetwork result = _networkService.updatePhysicalNetwork(getId(),getNetworkSpeed(), getTags(), getVlan(), getState(), getRemoveVlan());
         PhysicalNetworkResponse response = _responseGenerator.createPhysicalNetworkResponse(result);
         response.setResponseName(getCommandName());
         this.setResponseObject(response);

server/src/com/cloud/api/ApiResponseHelper.java

@@ -2667,7 +2667,7 @@ public class ApiResponseHelper implements ResponseGenerator {
             response.setZoneId(zone.getUuid());
         }
         response.setNetworkSpeed(result.getSpeed());
-        response.setVlan(result.getVnet());
+        response.setVlan(result.getVnetString());
         if (result.getDomainId() != null) {
             Domain domain = ApiDBUtils.findDomainById(result.getDomainId());
             if (domain != null) {

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -2633,7 +2633,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                     if (vlan == null) {
                         throw new CloudRuntimeException("Unable to acquire vlan configuration: " + vlanDbId);
                     }
-                    
+
                     if (s_logger.isDebugEnabled()) {
                         s_logger.debug("lock vlan " + vlanDbId + " is acquired");
                     }

server/src/com/cloud/dc/dao/DataCenterVnetDao.java

@@ -20,9 +20,11 @@ import java.util.List;
 
 import com.cloud.dc.DataCenterVnetVO;
 import com.cloud.utils.db.GenericDao;
+import com.cloud.utils.db.Transaction;
 
 public interface DataCenterVnetDao extends GenericDao<DataCenterVnetVO, Long> {
     public List<DataCenterVnetVO> listAllocatedVnets(long physicalNetworkId);
+    public List<DataCenterVnetVO> listAllocatedVnetsInRange(long dcId, long physicalNetworkId, Integer start, Integer end);
     public List<DataCenterVnetVO> findVnet(long dcId, String vnet);   
     public int countZoneVlans(long dcId, boolean onlyCountAllocated);    
     public List<DataCenterVnetVO> findVnet(long dcId, long physicalNetworkId, String vnet);
@@ -31,6 +33,10 @@ public interface DataCenterVnetDao extends GenericDao<DataCenterVnetVO, Long> {
     
     public void delete(long physicalNetworkId);
 
+    public void deleteRange(Transaction txn, long dcId, long physicalNetworkId, int start, int end);
+
+    public void lockRange(long dcId, long physicalNetworkId, Integer start, Integer end);
+
     public DataCenterVnetVO take(long physicalNetworkId, long accountId, String reservationId);
 
     public void release(String vnet, long physicalNetworkId, long accountId, String reservationId);

server/src/com/cloud/dc/dao/DataCenterVnetDaoImpl.java

@@ -21,6 +21,7 @@ import java.sql.SQLException;
 import java.util.Date;
 import java.util.List;
 
+import com.cloud.exception.InvalidParameterValueException;
 import org.springframework.stereotype.Component;
 
 import com.cloud.dc.DataCenterVnetVO;
@@ -46,8 +47,10 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long
     private final SearchBuilder<DataCenterVnetVO> VnetDcSearch;
     private final SearchBuilder<DataCenterVnetVO> VnetDcSearchAllocated;
     private final SearchBuilder<DataCenterVnetVO> DcSearchAllocated;
+    private final SearchBuilder<DataCenterVnetVO> DcSearchAllocatedInRange;
     private final GenericSearchBuilder<DataCenterVnetVO, Integer> countZoneVlans;
     private final GenericSearchBuilder<DataCenterVnetVO, Integer> countAllocatedZoneVlans;
+    private final SearchBuilder<DataCenterVnetVO> SearchRange;
     
     public List<DataCenterVnetVO> listAllocatedVnets(long physicalNetworkId) {
         SearchCriteria<DataCenterVnetVO> sc = DcSearchAllocated.create();
@@ -55,6 +58,22 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long
         return listBy(sc);
     }
 
+    public List<DataCenterVnetVO> listAllocatedVnetsInRange(long dcId, long physicalNetworkId, Integer start, Integer end) {
+        SearchCriteria<DataCenterVnetVO> sc = DcSearchAllocatedInRange.create();
+        sc.setParameters("dc",dcId);
+        sc.setParameters("physicalNetworkId", physicalNetworkId);
+        sc.setParameters("vnetRange", start.toString(), end.toString());
+        return listBy(sc);
+    }
+
+    public void lockRange(long dcId, long physicalNetworkId, Integer start, Integer end) {
+        SearchCriteria<DataCenterVnetVO> sc = SearchRange.create();
+        sc.setParameters("dc",dcId);
+        sc.setParameters("physicalNetworkId", physicalNetworkId);
+        sc.setParameters("vnetRange", start.toString(), end.toString());
+        lockRows(sc,null,true);
+    }
+
     public List<DataCenterVnetVO> findVnet(long dcId, String vnet) {
     	SearchCriteria<DataCenterVnetVO> sc = VnetDcSearch.create();;
     	sc.setParameters("dc", dcId);
@@ -93,11 +112,28 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long
             }
             stmt.executeBatch();
             txn.commit();
+        } catch (SQLException e) {
+           if (!e.getMessage().contains("Duplicate")){
+               txn.rollback();
+               txn.close();
+               throw new CloudRuntimeException("Exception caught adding vnet ", e);
+           }
+        }
+    }
+
+    public void deleteRange(Transaction txn, long dcId, long physicalNetworkId, int start, int end) {
+        String deleteVnet = "DELETE FROM `cloud`.`op_dc_vnet_alloc` WHERE data_center_id=? AND physical_network_id=? AND taken IS NULL AND vnet BETWEEN ? AND ?";
+        try {
+            PreparedStatement stmt = txn.prepareAutoCloseStatement(deleteVnet);
+            stmt.setLong(1,dcId);
+            stmt.setLong(2,physicalNetworkId);
+            stmt.setString(3,((Integer)start).toString());
+            stmt.setString(4,((Integer)end).toString());
+            stmt.execute();
         } catch (SQLException e) {
             throw new CloudRuntimeException("Exception caught adding vnet ", e);
         }
     }
-    
     public void delete(long physicalNetworkId) {
         SearchCriteria<DataCenterVnetVO> sc = VnetDcSearch.create();
         sc.setParameters("physicalNetworkId", physicalNetworkId);
@@ -149,6 +185,18 @@ public class DataCenterVnetDaoImpl extends GenericDaoBase<DataCenterVnetVO, Long
         DcSearchAllocated.and("physicalNetworkId", DcSearchAllocated.entity().getPhysicalNetworkId(), SearchCriteria.Op.EQ);
         DcSearchAllocated.and("allocated", DcSearchAllocated.entity().getTakenAt(), SearchCriteria.Op.NNULL);
         DcSearchAllocated.done();
+
+        DcSearchAllocatedInRange = createSearchBuilder();
+        DcSearchAllocatedInRange.and("dc",DcSearchAllocatedInRange.entity().getDataCenterId(), Op.EQ);
+        DcSearchAllocatedInRange.and("physicalNetworkId", DcSearchAllocatedInRange.entity().getPhysicalNetworkId(), Op.EQ);
+        DcSearchAllocatedInRange.and("allocated", DcSearchAllocatedInRange.entity().getTakenAt(), Op.NNULL);
+        DcSearchAllocatedInRange.and("vnetRange", DcSearchAllocatedInRange.entity().getVnet(), Op.BETWEEN);
+        DcSearchAllocatedInRange.done();
+
+        SearchRange = createSearchBuilder();
+        SearchRange.and("dc", SearchRange.entity().getDataCenterId(), Op.EQ);
+        SearchRange.and("physicalNetworkId", SearchRange.entity().getPhysicalNetworkId(), Op.EQ);
+        SearchRange.and("vnetRange", SearchRange.entity().getVnet(), Op.BETWEEN);
         
         FreeVnetSearch = createSearchBuilder();
         FreeVnetSearch.and("dc", FreeVnetSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ);

server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java

@@ -29,6 +29,7 @@ import javax.naming.ConfigurationException;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.response.ExternalFirewallResponse;
 import org.apache.cloudstack.network.ExternalNetworkDeviceManager.NetworkDevice;
+import com.cloud.utils.Pair;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
@@ -715,8 +716,17 @@ public abstract class ExternalFirewallDeviceManagerImpl extends AdapterBase impl
         if (pNetwork.getVnet() == null) {
             throw new CloudRuntimeException("Could not find vlan range for physical Network " + physicalNetworkId + ".");
         }
-        String vlanRange[] = pNetwork.getVnet().split("-");
+        Integer lowestVlanTag = null;
-        int lowestVlanTag = Integer.valueOf(vlanRange[0]);
+        List<Pair<Integer, Integer>> vnetList = pNetwork.getVnet();
+        //finding the vlanrange in which the vlanTag lies.
+        for (Pair <Integer,Integer> vnet : vnetList){
+            if (vlanTag >= vnet.first() && vlanTag <= vnet.second()){
+                lowestVlanTag = vnet.first();
+            }
+        }
+        if (lowestVlanTag == null) {
+            throw new InvalidParameterValueException ("The vlan tag dose not belong to any of the existing vlan ranges");
+        }
         return vlanTag - lowestVlanTag;
     }
     

server/src/com/cloud/network/NetworkServiceImpl.java

@@ -19,16 +19,10 @@ package com.cloud.network;
 import com.cloud.configuration.Config;
 import com.cloud.configuration.ConfigurationManager;
 import com.cloud.configuration.dao.ConfigurationDao;
-import com.cloud.dc.DataCenter;
+import com.cloud.dc.*;
 import com.cloud.dc.DataCenter.NetworkType;
-import com.cloud.dc.DataCenterVO;
-import com.cloud.dc.Pod;
 import com.cloud.dc.Vlan.VlanType;
-import com.cloud.dc.VlanVO;
+import com.cloud.dc.dao.*;
-import com.cloud.dc.dao.AccountVlanMapDao;
-import com.cloud.dc.dao.DataCenterDao;
-import com.cloud.dc.dao.HostPodDao;
-import com.cloud.dc.dao.VlanDao;
 import com.cloud.deploy.DeployDestination;
 import com.cloud.domain.Domain;
 import com.cloud.domain.DomainVO;
@@ -89,6 +83,7 @@ import com.cloud.utils.net.NetUtils;
 import com.cloud.vm.*;
 import com.cloud.vm.dao.*;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
+import org.apache.cloudstack.acl.SecurityChecker;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypeImplementorsCmd;
 import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd;
@@ -206,6 +201,8 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
     HostDao _hostDao;
     @Inject
     HostPodDao _hostPodDao;
+    @Inject
+    DataCenterVnetDao _datacneter_vnet;
 
     int _cidrLimit;
     boolean _allowSubdomainNetworkAccess;
@@ -538,7 +535,7 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
         } else if (dc.getNetworkType() == NetworkType.Basic || ntwkOff.getGuestType()  == Network.GuestType.Shared) {
             Account caller = UserContext.current().getCaller();
             long callerUserId = UserContext.current().getCallerUserId();
-            _accountMgr.checkAccess(caller, AccessType.UseNetwork, false, network);
+            _accountMgr.checkAccess(caller, SecurityChecker.AccessType.UseNetwork, false, network);
             //handle the basic networks here
             VirtualMachine vm = _userVmDao.findById(nicVO.getInstanceId());
             if (vm == null) {
@@ -758,18 +755,20 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
         // in the zone when using external networking
         PhysicalNetworkVO pNetwork = _physicalNetworkDao.findById(physicalNetworkId);
         if (pNetwork.getVnet() != null) {
-            String vlanRange[] = pNetwork.getVnet().split("-");
+            List <Pair<Integer,Integer>> vlanList = pNetwork.getVnet();
-            int lowestVlanTag = Integer.valueOf(vlanRange[0]);
+            for (Pair<Integer,Integer> vlanRange : vlanList){
-            int highestVlanTag = Integer.valueOf(vlanRange[1]);
+                Integer lowestVlanTag = vlanRange.first();
-            for (int vlan=lowestVlanTag; vlan <= highestVlanTag; ++vlan) {
+                Integer highestVlanTag = vlanRange.second();
-                int offset = vlan - lowestVlanTag;
+                for (int vlan=lowestVlanTag; vlan <= highestVlanTag; ++vlan) {
-                String globalVlanBits = _configDao.getValue(Config.GuestVlanBits.key());
+                    int offset = vlan - lowestVlanTag;
-                int cidrSize = 8 + Integer.parseInt(globalVlanBits);
+                    String globalVlanBits = _configDao.getValue(Config.GuestVlanBits.key());
-                String guestNetworkCidr = zone.getGuestNetworkCidr();
+                    int cidrSize = 8 + Integer.parseInt(globalVlanBits);
-                String[] cidrTuple = guestNetworkCidr.split("\\/");
+                    String guestNetworkCidr = zone.getGuestNetworkCidr();
-                long newCidrAddress = (NetUtils.ip2Long(cidrTuple[0]) & 0xff000000) | (offset << (32 - cidrSize));
+                    String[] cidrTuple = guestNetworkCidr.split("\\/");
-                if (NetUtils.isNetworksOverlap(NetUtils.long2Ip(newCidrAddress), cidr)) {
+                    long newCidrAddress = (NetUtils.ip2Long(cidrTuple[0]) & 0xff000000) | (offset << (32 - cidrSize));
-                    throw new InvalidParameterValueException("Specified CIDR for shared network conflict with CIDR that is reserved for zone vlan " + vlan);
+                    if (NetUtils.isNetworksOverlap(NetUtils.long2Ip(newCidrAddress), cidr)) {
+                        throw new InvalidParameterValueException("Specified CIDR for shared network conflict with CIDR that is reserved for zone vlan " + vlan);
+                    }
                 }
             }
         }
@@ -2198,7 +2197,6 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
             } catch (NumberFormatException e) {
                 throw new InvalidParameterValueException("Please specify valid integers for the vlan range.");
             }
-            
             if ((vnetStart > vnetEnd) || (vnetStart < 0) || (vnetEnd > 4096)) {
                 s_logger.warn("Invalid vnet range: start range:" + vnetStart + " end range:" + vnetEnd);
                 throw new InvalidParameterValueException("Vnet range should be between 0-4096 and start range should be lesser than or equal to end range");
@@ -2289,7 +2287,7 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
     @Override
     @DB
     @ActionEvent(eventType = EventTypes.EVENT_PHYSICAL_NETWORK_UPDATE, eventDescription = "updating physical network", async = true)
-    public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state) {
+    public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state, String removeVlan) {
 
         // verify input parameters
         PhysicalNetworkVO network = _physicalNetworkDao.findById(id);
@@ -2314,6 +2312,12 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
             }
         }
 
+        if (removeVlan != null){
+            List<Integer> tokens = processVlanRange(network,removeVlan);
+            boolean result = removeVlanRange(network, tokens.get(0), tokens.get(1));
+
+        }
+
         if (tags != null && tags.size() > 1) {
             throw new InvalidParameterException("Unable to support more than one tag on network yet");
         }
@@ -2340,90 +2344,211 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
         }
 
         // Vnet range can be extended only
-        boolean replaceVnet = false;
+        boolean AddVnet = true;
-        ArrayList<Pair<Integer, Integer>> vnetsToAdd = new ArrayList<Pair<Integer, Integer>>(2);
+        List<Pair<Integer, Integer>> vnetsToAdd = new ArrayList<Pair<Integer, Integer>>();
 
         if (newVnetRangeString != null) {
             Integer newStartVnet = 0;
             Integer newEndVnet = 0;
-            String[] newVnetRange = newVnetRangeString.split("-");
+            List<Integer> tokens = processVlanRange(network, newVnetRangeString);
-            int maxVnet = 4096;
+            newStartVnet = tokens.get(0);
-            // for GRE phynets allow up to 32bits
+            newEndVnet = tokens.get(1);
-            // TODO: Not happy about this test.  
+            Integer j=0;
-            // What about guru-like objects for physical networs?
+            List <Pair <Integer,Integer>> existingRanges = network.getVnet();
-            s_logger.debug("ISOLATION METHODS:" + network.getIsolationMethods());
+            if (!existingRanges.isEmpty()) {
-            // Java does not have unsigned types...
+                for (; j < existingRanges.size(); j++){
-            if (network.getIsolationMethods().contains("GRE")) { 
+                    int existingStartVnet = existingRanges.get(j).first();
-                maxVnet = (int)(Math.pow(2, 32)-1);
+                    int existingEndVnet = existingRanges.get(j).second();
-            }
-            String rangeMessage = " between 0 and " + maxVnet; 
-            if (newVnetRange.length < 2) {
-                throw new InvalidParameterValueException("Please provide valid vnet range" + rangeMessage);
-            }
 
-            if (newVnetRange[0] == null || newVnetRange[1] == null) {
+                    // check if vnet is being extended
-                throw new InvalidParameterValueException("Please provide valid vnet range" + rangeMessage);
+                    if (newStartVnet.intValue() >= existingStartVnet & newEndVnet.intValue() <= existingEndVnet) {
-            }
+                        throw new InvalidParameterValueException("The vlan range you trying to add already exists.");
+                    }
 
-            try {
+                    if (newStartVnet < existingStartVnet & newEndVnet+1 >= existingStartVnet & newEndVnet <= existingEndVnet) {
-                newStartVnet = Integer.parseInt(newVnetRange[0]);
+                        vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet, existingStartVnet - 1));
-                newEndVnet = Integer.parseInt(newVnetRange[1]);
+                        existingRanges.get(j).first(newStartVnet);
-            } catch (NumberFormatException e) {
+                        AddVnet = false;
-                s_logger.warn("Unable to parse vnet range:", e);
+                        break;
-                throw new InvalidParameterValueException("Please provide valid vnet range" + rangeMessage);
+                    }
-            }
-            if (newStartVnet < 0 || newEndVnet > maxVnet) {
-                throw new InvalidParameterValueException("Vnet range has to be" + rangeMessage);
-            }
 
-            if (newStartVnet > newEndVnet) {
+                    else if (newStartVnet > existingStartVnet & newStartVnet-1 <= existingEndVnet & newEndVnet >= existingEndVnet) {
-                throw new InvalidParameterValueException("Vnet range has to be" + rangeMessage + " and start range should be lesser than or equal to stop range");
+                        vnetsToAdd.add(new Pair<Integer, Integer>(existingEndVnet + 1, newEndVnet));
-            }
+                        existingRanges.get(j).second(newEndVnet);
-            
+                        AddVnet = false;
-            if (physicalNetworkHasAllocatedVnets(network.getDataCenterId(), network.getId())) {
+                        break;
-                String[] existingRange = network.getVnet().split("-");
+                    }
-                int existingStartVnet = Integer.parseInt(existingRange[0]);
-                int existingEndVnet = Integer.parseInt(existingRange[1]);
 
-                // check if vnet is being extended
+                    else if (newStartVnet< existingStartVnet & newEndVnet > existingEndVnet){
-                if (newStartVnet.intValue() > existingStartVnet || newEndVnet.intValue() < existingEndVnet) {
+                        vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet,existingStartVnet-1));
-                    throw new InvalidParameterValueException("Can't shrink existing vnet range as it the range has vnets allocated. Only extending existing vnet is supported");
+                        vnetsToAdd.add(new Pair<Integer, Integer>(existingEndVnet+1,newEndVnet));
+                        existingRanges.get(j).first(newStartVnet);
+                        existingRanges.get(j).second(newEndVnet);
+                        break;
+                    }
                 }
 
-                if (newStartVnet < existingStartVnet) {
-                    vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet, existingStartVnet - 1));
-                }
-
-                if (newEndVnet > existingEndVnet) {
-                    vnetsToAdd.add(new Pair<Integer, Integer>(existingEndVnet + 1, newEndVnet));
-                }
-
-            } else {
-                vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet, newEndVnet));
-                replaceVnet = true;
             }
-        }
+            if (AddVnet){
+                    vnetsToAdd.add(new Pair<Integer, Integer>(newStartVnet, newEndVnet));
+                    existingRanges.add(new Pair<Integer, Integer>(newStartVnet,newEndVnet));
+            }
 
-        if (newVnetRangeString != null) {
+            Map <Integer,Integer> vnetMap = new HashMap<Integer, Integer>(existingRanges.size());
-            network.setVnet(newVnetRangeString);
+            Map <Integer, Integer> IndexMap = new HashMap<Integer, Integer>(existingRanges.size());
-        }
+            for (int i=0; i< existingRanges.size(); i++){
+                 vnetMap.put(existingRanges.get(i).first(),existingRanges.get(i).second());
+                 IndexMap.put(existingRanges.get(i).first(),i);
+            }
 
-        _physicalNetworkDao.update(id, network);
+            Integer value;
+            Integer index;
+            String vnetString = "";
+            for (int i=0; i < existingRanges.size(); i++){
+                 value = vnetMap.get((existingRanges.get(i).second()+1));
+                 if (value != null) {
+                     vnetMap.remove((existingRanges.get(i).second()+1));
+                     vnetMap.remove(existingRanges.get(i).first());
+                     vnetMap.put(existingRanges.get(i).first(),value);
+                     existingRanges.add(new Pair<Integer,Integer>(existingRanges.get(i).first(),value));
+                     index = IndexMap.get(existingRanges.get(i).second()+1);
+                     existingRanges.get(index).first(-1);
+                     existingRanges.get(index).second(-1);
+                     existingRanges.get(i).first(-1);
+                     existingRanges.get(i).second(-1);
+                 }
+                value = vnetMap.get((existingRanges.get(i).second()));
+                if (value != null) {
+                    vnetMap.remove((existingRanges.get(i).second()));
+                    vnetMap.remove(existingRanges.get(i).first());
+                    vnetMap.put(existingRanges.get(i).first(),value);
+                    existingRanges.add(new Pair<Integer,Integer>(existingRanges.get(i).first(),value));
+                    index = IndexMap.get(existingRanges.get(i).second());
+                    existingRanges.get(index).first(-1);
+                    existingRanges.get(index).second(-1);
+                    existingRanges.get(i).first(-1);
+                    existingRanges.get(i).second(-1);
+                }
+            }
 
-        if (replaceVnet) {
-            s_logger.debug("Deleting existing vnet range for the physicalNetwork id= " + id + " and zone id=" + network.getDataCenterId() + " as a part of updatePhysicalNetwork call");
-            _dcDao.deleteVnet(network.getId());
-        }
 
-        for (Pair<Integer, Integer> vnetToAdd : vnetsToAdd) {
+
-            s_logger.debug("Adding vnet range " + vnetToAdd.first() + "-" + vnetToAdd.second() + " for the physicalNetwork id= " + id + " and zone id=" + network.getDataCenterId()
+            if (newVnetRangeString != null) {
+               for (Pair<Integer,Integer> vnetRange : existingRanges ){
+                    value=vnetMap.get(vnetRange.first());
+                    if (value != null){
+                        vnetString = vnetString+vnetRange.first().toString()+"-"+value.toString()+";";
+                    }
+               }
+                vnetString = vnetString+"*";
+                vnetString = vnetString.replace(";*","");
+                network.setVnet(vnetString);
+            }
+
+
+
+            _physicalNetworkDao.update(id, network);
+
+            for (Pair<Integer, Integer> vnetToAdd : vnetsToAdd) {
+                s_logger.debug("Adding vnet range " + vnetToAdd.first() + "-" + vnetToAdd.second() + " for the physicalNetwork id= " + id + " and zone id=" + network.getDataCenterId()
                     + " as a part of updatePhysicalNetwork call");
-            _dcDao.addVnet(network.getDataCenterId(), network.getId(), vnetToAdd.first(), vnetToAdd.second());
+                _dcDao.addVnet(network.getDataCenterId(), network.getId(), vnetToAdd.first(), vnetToAdd.second());
+            }
         }
 
         return network;
     }
 
+    private List<Integer> processVlanRange(PhysicalNetworkVO network, String removeVlan) {
+        Integer StartVnet;
+        Integer EndVnet;
+        String[] VnetRange = removeVlan.split("-");
+        int maxVnet = 4096;
+        // for GRE phynets allow up to 32bits
+        // TODO: Not happy about this test.
+        // What about guru-like objects for physical networs?
+        s_logger.debug("ISOLATION METHODS:" + network.getIsolationMethods());
+        // Java does not have unsigned types...
+        if (network.getIsolationMethods().contains("GRE")) {
+            maxVnet = (int)(Math.pow(2, 32)-1);
+        }
+        String rangeMessage = " between 0 and " + maxVnet;
+        if (VnetRange.length < 2) {
+            throw new InvalidParameterValueException("Please provide valid vnet range" + rangeMessage);
+        }
+
+        if (VnetRange[0] == null || VnetRange[1] == null) {
+            throw new InvalidParameterValueException("Please provide valid vnet range" + rangeMessage);
+        }
+
+        try {
+            StartVnet = Integer.parseInt(VnetRange[0]);
+            EndVnet = Integer.parseInt(VnetRange[1]);
+        } catch (NumberFormatException e) {
+            s_logger.warn("Unable to parse vnet range:", e);
+            throw new InvalidParameterValueException("Please provide valid vnet range" + rangeMessage);
+        }
+        if (StartVnet < 0 || EndVnet > maxVnet) {
+            throw new InvalidParameterValueException("Vnet range has to be" + rangeMessage);
+        }
+
+        if (StartVnet > EndVnet) {
+            throw new InvalidParameterValueException("Vnet range has to be" + rangeMessage + " and start range should be lesser than or equal to stop range");
+        }
+        List<Integer> tokens = new ArrayList<Integer>();
+        tokens.add(StartVnet);
+        tokens.add(EndVnet);
+        return tokens;
+
+    }
+
+
+    private boolean removeVlanRange( PhysicalNetworkVO network, Integer start, Integer end) {
+        Integer temp=0;
+        int i;
+        List <Pair <Integer,Integer>> existingRanges = network.getVnet();
+        Transaction txn = Transaction.currentTxn();
+        txn.start();
+        _physicalNetworkDao.acquireInLockTable(network.getId(),10);
+        _datacneter_vnet.lockRange(network.getDataCenterId(), network.getId(), start, end);
+        List<DataCenterVnetVO> result = _datacneter_vnet.listAllocatedVnetsInRange(network.getDataCenterId(), network.getId(), start, end);
+        if (!result.isEmpty()){
+            txn.close();
+            throw new InvalidParameterValueException("Some of the vnets from this range are allocated, can only remove a range which has no allocated vnets");
+        }
+        for (i=0; i<existingRanges.size(); i++){
+            if (existingRanges.get(i).first()<= start & existingRanges.get(i).second()>= end){
+                temp = existingRanges.get(i).second();
+                existingRanges.get(i).second(start - 1);
+                existingRanges.add(new Pair<Integer, Integer>((end+1),temp));
+                break;
+            }
+        }
+
+        if (temp == 0){
+            throw new InvalidParameterValueException("The vlan range you are trying to delete dose not exist.");
+        }
+        if(existingRanges.get(i).first() > existingRanges.get(i).second()){
+            existingRanges.remove(i);
+        }
+        if(existingRanges.get(existingRanges.size()-1).first() > existingRanges.get(existingRanges.size()-1).second()){
+            existingRanges.remove(existingRanges.size()-1);
+        }
+        _datacneter_vnet.deleteRange(txn, network.getDataCenterId(), network.getId(), start, end);
+
+        String vnetString="";
+        for (Pair<Integer,Integer> vnetRange : existingRanges ){
+            vnetString=vnetString+vnetRange.first().toString()+"-"+vnetRange.second().toString()+";";
+        }
+        vnetString = vnetString+"*";
+        vnetString = vnetString.replace(";*","");
+        network.setVnet(vnetString);
+        _physicalNetworkDao.update(network.getId(), network);
+        txn.commit();
+        _physicalNetworkDao.releaseFromLockTable(network.getId());
+        return  true;
+    }
+
     private boolean physicalNetworkHasAllocatedVnets(long zoneId, long physicalNetworkId) {
         return !_dcDao.listAllocatedVnets(physicalNetworkId).isEmpty();
     }

server/src/com/cloud/network/dao/PhysicalNetworkVO.java

@@ -34,11 +34,9 @@ import javax.persistence.Table;
 import javax.persistence.TableGenerator;
 
 import com.cloud.network.PhysicalNetwork;
-import com.cloud.network.PhysicalNetwork.BroadcastDomainRange;
-import com.cloud.network.PhysicalNetwork.State;
 import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.Pair;
 import com.cloud.utils.db.GenericDao;
-import org.apache.cloudstack.api.InternalIdentity;
 
 /**
  * NetworkConfigurationVO contains information about a specific physical network.
@@ -205,7 +203,21 @@ public class PhysicalNetworkVO implements PhysicalNetwork {
     }
 
     @Override
-    public String getVnet() {
+    public List<Pair<Integer, Integer>> getVnet() {
+        List <Pair<Integer,Integer>>  vnetList = new ArrayList<Pair<Integer, Integer>>();
+        if (vnet != null) {
+           String [] Temp = vnet.split(";");
+           String [] vnetSplit = null;
+           for (String vnetRange : Temp){
+               vnetSplit = vnetRange.split("-");
+               vnetList.add(new Pair<Integer,Integer>(Integer.parseInt(vnetSplit[0]),Integer.parseInt(vnetSplit[1])));
+           }
+        }
+        return vnetList;
+    }
+
+    @Override
+    public String getVnetString() {
         return vnet;
     }
 

server/src/com/cloud/network/guru/GuestNetworkGuru.java

@@ -26,6 +26,7 @@ import javax.ejb.Local;
 import javax.inject.Inject;
 
 import com.cloud.event.ActionEventUtils;
+import com.cloud.utils.Pair;
 import org.apache.log4j.Logger;
 
 import com.cloud.configuration.Config;
@@ -274,8 +275,17 @@ public abstract class GuestNetworkGuru extends AdapterBase implements NetworkGur
         if (pNetwork.getVnet() == null) {
             throw new CloudRuntimeException("Could not find vlan range for physical Network " + physicalNetworkId + ".");
         }
-        String vlanRange[] = pNetwork.getVnet().split("-");
+        Integer lowestVlanTag = null;
-        int lowestVlanTag = Integer.valueOf(vlanRange[0]);
+        List<Pair<Integer, Integer>> vnetList = pNetwork.getVnet();
+        //finding the vlanrange in which the vlanTag lies.
+        for (Pair <Integer,Integer> vnet : vnetList){
+            if (vlanTag >= vnet.first() && vlanTag <= vnet.second()){
+                lowestVlanTag = vnet.first();
+            }
+        }
+        if (lowestVlanTag == null) {
+            throw new InvalidParameterValueException ("The vlan tag dose not belong to any of the existing vlan ranges");
+        }
         return vlanTag - lowestVlanTag;
     }
 

server/test/com/cloud/network/MockNetworkManagerImpl.java

@@ -322,7 +322,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage
     }
 
     @Override
-    public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state) {
+    public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state, String removeVlan) {
         // TODO Auto-generated method stub
         return null;
     }

server/test/com/cloud/network/UpdatePhysicalNetworkTest.java

@@ -0,0 +1,68 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.network;
+import com.cloud.capacity.CapacityManagerImpl;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.network.NetworkServiceImpl;
+import com.cloud.network.dao.PhysicalNetworkDao;
+import com.cloud.network.dao.PhysicalNetworkVO;
+import com.cloud.utils.Pair;
+import org.junit.Test;
+import org.junit.*;
+import org.mockito.ArgumentCaptor;
+import org.mockito.MockitoAnnotations.*;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+
+public class UpdatePhysicalNetworkTest {
+    private PhysicalNetworkDao _physicalNetworkDao = mock(PhysicalNetworkDao.class);
+    private DataCenterDao _datacenterDao = mock(DataCenterDao.class);
+    private DataCenterVO datacentervo = mock(DataCenterVO.class);
+    private PhysicalNetworkVO physicalNetworkVO = mock(PhysicalNetworkVO.class);
+    List<Pair<Integer,Integer>> existingRange = new ArrayList<Pair<Integer, Integer>>();
+    ArgumentCaptor<String> argumentCaptor =  ArgumentCaptor.forClass(String.class);
+
+    public NetworkServiceImpl setUp() {
+        NetworkServiceImpl networkService = new NetworkServiceImpl();
+        ((NetworkServiceImpl)networkService)._dcDao= _datacenterDao;
+        networkService._physicalNetworkDao = _physicalNetworkDao;
+        return networkService;
+    }
+
+    @Test
+    public void updatePhysicalNetworkTest(){
+        NetworkServiceImpl networkService = setUp();
+        existingRange.add(new Pair<Integer, Integer>(520, 524));
+        when(_physicalNetworkDao.findById(anyLong())).thenReturn(physicalNetworkVO);
+        when(_datacenterDao.findById(anyLong())).thenReturn(datacentervo);
+        when(_physicalNetworkDao.update(anyLong(), any(physicalNetworkVO.getClass()))).thenReturn(true);
+        when(physicalNetworkVO.getVnet()).thenReturn(existingRange);
+        networkService.updatePhysicalNetwork(1l, null, null, "525-530", null, null);
+        verify(physicalNetworkVO).setVnet(argumentCaptor.capture());
+        assertEquals("520-530", argumentCaptor.getValue());
+    }
+
+}

server/test/com/cloud/vpc/MockNetworkManagerImpl.java

@@ -328,7 +328,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage
      */
     @Override
     public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags,
-            String newVnetRangeString, String state) {
+                                                 String newVnetRangeString, String state, String removeVlan) {
         // TODO Auto-generated method stub
         return null;
     }

test/integration/smoke/test_non_contigiousvlan.py

@@ -0,0 +1,125 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Primary Storage
+"""
+import marvin
+from marvin import cloudstackTestCase
+from marvin.cloudstackTestCase import *
+
+import unittest
+import hashlib
+import random
+
+
+class TestUpdatePhysicalNetwork(cloudstackTestCase):
+    """
+    This test updates the existing physicalnetwork with a new vlan range.
+    """
+    def setUp(self):
+        """
+        CloudStack internally saves its passwords in md5 form and that is how we
+        specify it in the API. Python's hashlib library helps us to quickly hash
+        strings as follows
+        """
+        mdf = hashlib.md5()
+        mdf.update('password')
+        mdf_pass = mdf.hexdigest()
+
+        self.apiClient = self.testClient.getApiClient() #Get ourselves an API client
+
+        self.acct = createAccount.createAccountCmd() #The createAccount command
+        self.acct.accounttype = 0                    #We need a regular user. admins have accounttype=1
+        self.acct.firstname = 'bharat'
+        self.acct.lastname = 'kumar'                 #What's up doc?
+        self.acct.password = mdf_pass                #The md5 hashed password string
+        self.acct.username = 'bharat'
+        self.acct.email = 'bharat@kumar.com'
+        self.acct.account = 'bharat'
+        self.acct.domainid = 1                       #The default ROOT domain
+        self.acctResponse = self.apiClient.createAccount(self.acct)
+        # using the default debug logger of the test framework
+        self.debug("successfully created account: %s, user: %s, id: \
+                   %s"%(self.acctResponse.account.account, \
+                        self.acctResponse.account.username, \
+                        self.acctResponse.account.id))
+
+    def test_UpdatePhysicalNetwork(self):
+        """
+        Let's start by defining the attributes of our VM that we will be
+        deploying on CloudStack. We will be assuming a single zone is available
+        and is configured and all templates are Ready
+
+        The hardcoded values are used only for brevity.
+        """
+        listPhysicalNetworksCmd = listPhysicalNetworks.listPhysicalNetworksCmd()
+        listPhysicalNetworksResponse = self.apiClient.listPhysicalNetworks(listPhysicalNetworksCmd)
+        
+        self.assertNotEqual(len(listPhysicalNetworksResponse), 0, "Check if the list API \
+                           returns a non-empty response")
+        
+        networkid = listPhysicalNetworksResponse[0].id
+        updatePhysicalNetworkCmd = updatePhysicalNetwork.updatePhysicalNetworkCmd()
+        updatePhysicalNetworkCmd.id = networkid
+        updatePhysicalNetworkCmd.vlan = "4090-4091"
+        updatePhysicalNetworkResponse = self.apiClient.updatePhysicalNetwork(updatePhysicalNetworkCmd)
+        self.assertNotEqual((updatePhysicalNetworkResponse.len), 0, "Check if the list API \
+                           returns a non-empty response")
+        
+        updatePhysicalNetworkCmd = updatePhysicalNetwork.updatePhysicalNetworkCmd()
+        updatePhysicalNetworkCmd.id = networkid
+        updatePhysicalNetworkCmd.vlan = "4092-4096"
+        updatePhysicalNetworkResponse = self.apiClient.updatePhysicalNetwork(updatePhysicalNetworkCmd)
+        self.assertNotEqual((updatePhysicalNetworkResponse.len), 0, "Check if the list API \
+                                returns a non-empty response")
+
+        vlanranges= updatePhysicalNetworkResponse.vlan
+        range = ""
+        vlanranges = vlanranges.split(";")
+        for vlan in vlanranges:
+            if (vlan == "4090-4096"):
+               range = vlan
+    
+        self.assertEqual(range, "4090-4096", "check if adding the range is successful")
+
+        updatePhysicalNetworkCmd = updatePhysicalNetwork.updatePhysicalNetworkCmd()
+        updatePhysicalNetworkCmd.id = networkid
+        updatePhysicalNetworkCmd.removevlan = "4090-4096"
+        updatePhysicalNetworkResponse = self.apiClient.updatePhysicalNetwork(updatePhysicalNetworkCmd)
+        self.assertNotEqual((updatePhysicalNetworkResponse.len), 0, "Check if the list API \
+                    returns a non-empty response")
+
+        vlanranges= updatePhysicalNetworkResponse.vlan
+        range = ""
+        vlanranges = vlanranges.split(";")
+
+        for vlan in vlanranges:
+            if (vlan == "4090-4096"):
+               range = vlan
+    
+    
+        self.assertEqual(range, "", "check if removing the range is successful")
+
+    
+    def tearDown(self):                               # Teardown will delete the Account as well as the VM once the VM reaches "Running" state
+        """
+        And finally let us cleanup the resources we created by deleting the
+        account. All good unittests are atomic and rerunnable this way
+        """
+        deleteAcct = deleteAccount.deleteAccountCmd()
+        deleteAcct.id = self.acctResponse.account.id
+        self.apiClient.deleteAccount(deleteAcct)
+