Refactored 1-1 NAT feature:

* added new fields to createIpForwardingRule api: startPort/protocol - required, endPort is optional
* updated "firewall_rules" table with a new field "is_one_to_one_nat" (being set when new rule is created)

api/src/com/cloud/agent/api/to/FirewallRuleTO.java

@@ -47,21 +47,23 @@ public class FirewallRuleTO {
     int[] srcPortRange;
     boolean revoked;
     boolean alreadyAdded;
+    boolean isOneToOneNat;
     String vlanNetmask;    // FIXME: Get rid of this!
 
     protected FirewallRuleTO() {
     }
     
-    public FirewallRuleTO(long id, String srcIp, String protocol, int srcPortStart, int srcPortEnd, boolean revoked, boolean alreadyAdded) {
+    public FirewallRuleTO(long id, String srcIp, String protocol, int srcPortStart, int srcPortEnd, boolean revoked, boolean alreadyAdded, boolean isOneToOneNat) {
         this.srcIp = srcIp;
         this.protocol = protocol;
         this.srcPortRange = new int[] {srcPortStart, srcPortEnd};
         this.revoked = revoked;
         this.alreadyAdded = alreadyAdded;
+        this.isOneToOneNat = isOneToOneNat;
     }
     
     public FirewallRuleTO(FirewallRule rule) {
-        this(rule.getId(), rule.getSourceIpAddress().addr(), rule.getProtocol(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getState()==State.Revoke, rule.getState()==State.Active);
+        this(rule.getId(), rule.getSourceIpAddress().addr(), rule.getProtocol(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getState()==State.Revoke, rule.getState()==State.Active, rule.isOneToOneNat());
     }
     
     public long getId() {
@@ -91,4 +93,8 @@ public class FirewallRuleTO {
     public boolean isAlreadyAdded() {
         return alreadyAdded;
     }
+    
+    public boolean isOneToOneNat() {
+        return isOneToOneNat;
+    }
 }

api/src/com/cloud/agent/api/to/PortForwardingRuleTO.java

@@ -40,7 +40,7 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
     }
     
     protected PortForwardingRuleTO(long id, String srcIp, int srcPortStart, int srcPortEnd, String dstIp, int dstPortStart, int dstPortEnd, String protocol, boolean revoked, boolean brandNew) {
-        super(id, srcIp, protocol, srcPortStart, srcPortEnd, revoked, brandNew);
+        super(id, srcIp, protocol, srcPortStart, srcPortEnd, revoked, brandNew, false);
         this.dstIp = dstIp;
         this.dstPortRange = new int[] { dstPortStart, dstPortEnd };
     }

api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java

@@ -33,7 +33,6 @@ import com.cloud.network.rules.PortForwardingRule;
 import com.cloud.user.Account;
 import com.cloud.user.UserContext;
 import com.cloud.utils.net.Ip;
-import com.cloud.utils.net.NetUtils;
 
 @Implementation(description="Creates an ip forwarding rule", responseObject=FirewallRuleResponse.class)
 public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements PortForwardingRule {
@@ -50,7 +49,15 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Por
 
     @Parameter(name=ApiConstants.VIRTUAL_MACHINE_ID, type=CommandType.LONG, required=true, description="the ID of the virtual machine for the forwarding rule")
     private Long virtualMachineId;
+    
+    @Parameter(name=ApiConstants.START_PORT, type=CommandType.INTEGER, required=true, description="the start port for the rule")
+    private Integer startPort;
 
+    @Parameter(name=ApiConstants.END_PORT, type=CommandType.INTEGER, description="the end port for the rule")
+    private Integer endPort;
+    
+    @Parameter(name=ApiConstants.PROTOCOL, type=CommandType.STRING, required=true, description="the protocol for the rule. Valid values are TCP or UDP.")
+    private String protocol;
 
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
@@ -63,7 +70,14 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Por
     public long getVirtualMachineId() {
         return virtualMachineId;
     }
-
+    
+    public int getStartPort() {
+        return startPort;
+    }
+    
+    public int getEndPort() {
+        return endPort;
+    }
 
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
@@ -99,7 +113,7 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Por
 	public void create() {
 		PortForwardingRule rule;
         try {
-            rule = _rulesService.createPortForwardingRule(this, virtualMachineId);
+            rule = _rulesService.createPortForwardingRule(this, virtualMachineId, true);
         } catch (NetworkRuleConflictException e) {
             s_logger.info("Unable to create Port Forwarding Rule due to " + e.getMessage());
             throw new ServerApiException(BaseCmd.NETWORK_RULE_CONFLICT_ERROR, e.getMessage());
@@ -146,17 +160,21 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Por
 
     @Override
     public int getSourcePortStart() {
-        return -1;
+        return startPort;
     }
 
     @Override
     public int getSourcePortEnd() {
-        return -1;
+        if (endPort == null) {
+            return startPort;
+        } else {
+            return endPort;
+        }
     }
 
     @Override
     public String getProtocol() {
-        return NetUtils.NAT_PROTO;
+        return protocol;
     }
 
     @Override
@@ -186,17 +204,26 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Por
 
     @Override
     public int getDestinationPortStart() {
-        return -1;
+        return startPort;
     }
 
     @Override
     public int getDestinationPortEnd() {
-        return -1;
+        if (endPort == null) {
+            return startPort;
+        } else {
+            return endPort;
+        }
     }
 
     @Override
     public long getAccountId() {
         throw new UnsupportedOperationException("Get the account id from network");
     }
+    
+    @Override
+    public boolean isOneToOneNat() {
+        return true;
+    }
 
 }

api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java

@@ -183,5 +183,10 @@ public class CreateLoadBalancerRuleCmd extends BaseCmd  implements LoadBalancer
     public int getDefaultPortEnd() {
         return privatePort.intValue();
     }
+    
+    @Override
+    public boolean isOneToOneNat() {
+        return false;
+    }
 
 }

api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java

@@ -196,7 +196,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd  implements
     @Override
     public void create() {
         try {
-            PortForwardingRule result = _rulesService.createPortForwardingRule(this, virtualMachineId);
+            PortForwardingRule result = _rulesService.createPortForwardingRule(this, virtualMachineId, false);
             setEntityId(result.getId());
         } catch (NetworkRuleConflictException ex) {
             s_logger.info("Network rule conflict: " + ex.getMessage());
@@ -219,5 +219,10 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd  implements
     public long getAccountId() {
         throw new UnsupportedOperationException("Get the account id from network");
     }
+    
+    @Override
+    public boolean isOneToOneNat() {
+        return false;
+    }
 
 }

api/src/com/cloud/api/response/IpForwardingRuleResponse.java

@@ -40,6 +40,12 @@ public class IpForwardingRuleResponse extends BaseResponse {
     @SerializedName("ipaddress") @Param(description="the public ip address for the port forwarding rule")
     private String publicIpAddress;
     
+    @SerializedName(ApiConstants.START_PORT) @Param(description="the start port of the rule")
+    private Integer startPort;
+    
+    @SerializedName(ApiConstants.END_PORT) @Param(description="the end port of the rule")
+    private Integer endPort;
+    
     @SerializedName("state") @Param(description="state of the ip forwarding rule")
     private String state;
     
@@ -98,4 +104,20 @@ public class IpForwardingRuleResponse extends BaseResponse {
     public void setState(String state) {
         this.state = state;
     }
+
+    public Integer getStartPort() {
+        return startPort;
+    }
+
+    public void setStartPort(Integer startPort) {
+        this.startPort = startPort;
+    }
+
+    public Integer getEndPort() {
+        return endPort;
+    }
+
+    public void setEndPort(Integer endPort) {
+        this.endPort = endPort;
+    }
 }

api/src/com/cloud/network/lb/LoadBalancingRule.java

@@ -136,4 +136,9 @@ public class LoadBalancingRule implements FirewallRule, LoadBalancer{
             return revoked;
         }
     }
+    
+    @Override
+    public boolean isOneToOneNat() {
+        return false;
+    }
 }

api/src/com/cloud/network/rules/FirewallRule.java

@@ -70,4 +70,6 @@ public interface FirewallRule extends ControlledEntity {
     State getState();
     
     long getNetworkId();
+    
+    boolean isOneToOneNat();
 }

api/src/com/cloud/network/rules/PortForwardingRule.java

@@ -42,4 +42,5 @@ public interface PortForwardingRule extends FirewallRule {
      * @return destination ip address.
      */
     long getVirtualMachineId();
+    
 }

api/src/com/cloud/network/rules/RulesService.java

@@ -33,10 +33,11 @@ public interface RulesService {
      * an ip address and a virtual machine.
      * @param rule rule to be created.
      * @param vmId vm to be linked to.  If specified the destination ip address is ignored.
+     * @param isNat TODO
      * @return PortForwardingRule if created.
      * @throws NetworkRuleConflictException if conflicts in the network rules are detected.
      */
-    PortForwardingRule createPortForwardingRule(PortForwardingRule rule, Long vmId) throws NetworkRuleConflictException;
+    PortForwardingRule createPortForwardingRule(PortForwardingRule rule, Long vmId, boolean isNat) throws NetworkRuleConflictException;
     
     /**
      * Revokes a port forwarding rule 

core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java

@@ -127,7 +127,7 @@ public class VirtualRoutingResource implements Manager {
         int i = 0;
         for (PortForwardingRuleTO rule : cmd.getRules()) {
             String result = null;
-            if (rule.getProtocol().toLowerCase().equalsIgnoreCase(NetUtils.NAT_PROTO)){
+            if (rule.isOneToOneNat()){
                 setStaticNat(!rule.revoked(), rule.getProtocol(), routerIp, rule.getSrcIp(), rule.getDstIp());
             } else {
 

core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java

@@ -1077,7 +1077,7 @@ public abstract class CitrixResourceBase implements ServerResource {
         String[] results = new String[cmd.getRules().length];
         int i = 0;
         for (PortForwardingRuleTO rule : cmd.getRules()) {
-            if (rule.getProtocol().toLowerCase().equals(NetUtils.NAT_PROTO)){
+            if (rule.isOneToOneNat()){
             	//1:1 NAT needs instanceip;publicip;domrip;op
             	args = rule.revoked() ? "-D" : "-A";
             	

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

@@ -972,6 +972,10 @@ public class ApiResponseHelper implements ResponseGenerator {
         if (state.equals(FirewallRule.State.Revoke)) {
             stateToSet = "Deleting";
         }
+        
+        response.setStartPort(fwRule.getSourcePortStart());
+        response.setEndPort(fwRule.getSourcePortEnd());
+        response.setProtocol(fwRule.getProtocol());
         response.setState(stateToSet);
         response.setObjectName("ipforwardingrule");
         return response;

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

@@ -54,7 +54,7 @@ public class LoadBalancerVO extends FirewallRuleVO implements LoadBalancer {
     }
 
     public LoadBalancerVO(String xId, String name, String description, Ip srcIp, int srcPort, int dstPort, String algorithm, long networkId, long accountId, long domainId) {
-        super(xId, srcIp, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing);
+        super(xId, srcIp, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, false);
         this.name = name;
         this.description = description;
         this.algorithm = algorithm;

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

@@ -20,7 +20,6 @@ package com.cloud.network.dao;
 
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
-import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 

server/src/com/cloud/network/rules/FirewallRuleVO.java

@@ -80,6 +80,9 @@ public class FirewallRuleVO implements FirewallRule {
     @Column(name=GenericDao.CREATED_COLUMN)
     Date created;
     
+    @Column(name="is_static_nat", updatable=false)
+    boolean oneToOneNat;
+    
     @Column(name="network_id")
     long networkId;
 
@@ -149,7 +152,7 @@ public class FirewallRuleVO implements FirewallRule {
     protected FirewallRuleVO() {
     }
     
-    public FirewallRuleVO(String xId, Ip srcIp, int portStart, int portEnd, String protocol, long networkId, long accountId, long domainId, Purpose purpose) {
+    public FirewallRuleVO(String xId, Ip srcIp, int portStart, int portEnd, String protocol, long networkId, long accountId, long domainId, Purpose purpose, boolean isOneToOneNat) {
         this.xId = xId;
         if (xId == null) {
             this.xId = UUID.randomUUID().toString();
@@ -163,14 +166,20 @@ public class FirewallRuleVO implements FirewallRule {
         this.purpose = purpose;
         this.networkId = networkId;
         this.state = State.Staged;
+        this.oneToOneNat = isOneToOneNat;
     }
     
-    public FirewallRuleVO(String xId, Ip srcIp, int port, String protocol, long networkId, long accountId, long domainId, Purpose purpose) {
+    public FirewallRuleVO(String xId, Ip srcIp, int port, String protocol, long networkId, long accountId, long domainId, Purpose purpose, boolean isOneToOneNat) {
-        this(xId, srcIp, port, port, protocol, networkId, accountId, domainId, purpose);
+        this(xId, srcIp, port, port, protocol, networkId, accountId, domainId, purpose, isOneToOneNat);
     }
     
     @Override
     public String toString() {
         return new StringBuilder("Rule[").append(id).append("-").append(purpose).append("-").append(state).append("]").toString();
     }
+    
+    @Override
+    public boolean isOneToOneNat() {
+        return oneToOneNat;
+    }
 }

server/src/com/cloud/network/rules/PortForwardingRuleVO.java

@@ -45,21 +45,23 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi
     private int destinationPortEnd;
     
     @Column(name="instance_id")
-    private long virtualMachineId;
+    private long virtualMachineId;
+    
+   
 
     public PortForwardingRuleVO() {
     }
 
-    public PortForwardingRuleVO(String xId, Ip srcIp, int srcPortStart, int srcPortEnd, Ip dstIp, int dstPortStart, int dstPortEnd, String protocol, long networkId, long accountId, long domainId, long instanceId) {
+    public PortForwardingRuleVO(String xId, Ip srcIp, int srcPortStart, int srcPortEnd, Ip dstIp, int dstPortStart, int dstPortEnd, String protocol, long networkId, long accountId, long domainId, long instanceId, boolean isOneToOneNat) {
-        super(xId, srcIp, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding);
+        super(xId, srcIp, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding, isOneToOneNat);
         this.destinationIpAddress = dstIp;
         this.virtualMachineId = instanceId;
         this.destinationPortStart = dstPortStart;
         this.destinationPortEnd = dstPortEnd;
     }
     
-    public PortForwardingRuleVO(String xId, Ip srcIp, int srcPort, Ip dstIp, int dstPort, String protocol, long networkId, long accountId, long domainId, long instanceId) {
+    public PortForwardingRuleVO(String xId, Ip srcIp, int srcPort, Ip dstIp, int dstPort, String protocol, long networkId, long accountId, long domainId, long instanceId, boolean isOneToOneNat) {
-        this(xId, srcIp, srcPort, srcPort, dstIp, dstPort, dstPort, protocol, networkId, accountId, domainId, instanceId);
+        this(xId, srcIp, srcPort, srcPort, dstIp, dstPort, dstPort, protocol, networkId, accountId, domainId, instanceId, isOneToOneNat);
     }
 
     @Override

server/src/com/cloud/network/rules/RulesManagerImpl.java

@@ -27,10 +27,7 @@ import javax.naming.ConfigurationException;
 import org.apache.log4j.Logger;
 
 import com.cloud.api.commands.ListPortForwardingRulesCmd;
-import com.cloud.domain.Domain;
-import com.cloud.domain.DomainVO;
 import com.cloud.event.EventTypes;
-import com.cloud.event.EventVO;
 import com.cloud.event.UsageEventVO;
 import com.cloud.event.dao.EventDao;
 import com.cloud.event.dao.UsageEventDao;
@@ -101,7 +98,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
             if (rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) {
                 throw new NetworkRuleConflictException("New rule is for a different network than what's specified in rule " + rule.getXid());
             }
-            if (rule.getProtocol().equals(NetUtils.NAT_PROTO)) {
+            if (rule.isOneToOneNat()) {
                 throw new NetworkRuleConflictException("There is already a one to one NAT specified for " + newRule.getSourceIpAddress());
             }
             if ((rule.getSourcePortStart() <= newRule.getSourcePortStart() && rule.getSourcePortEnd() >= newRule.getSourcePortStart()) || 
@@ -152,7 +149,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
     }
 
     @Override @DB
-    public PortForwardingRule createPortForwardingRule(PortForwardingRule rule, Long vmId) throws NetworkRuleConflictException {
+    public PortForwardingRule createPortForwardingRule(PortForwardingRule rule, Long vmId, boolean isNat) throws NetworkRuleConflictException {
         UserContext ctx = UserContext.current();
         Account caller = ctx.getCaller();
         
@@ -198,7 +195,6 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
         long domainId = network.getDomainId();
         
         checkIpAndUserVm(ipAddress, vm, caller);
-        boolean isNat = NetUtils.NAT_PROTO.equals(rule.getProtocol());
         if (isNat && (ipAddress.isSourceNat() || ipAddress.isOneToOneNat())) {
             throw new NetworkRuleConflictException("Can't do one to one NAT on ip address: " + ipAddress.getAddress());
         }
@@ -216,7 +212,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
                     rule.getProtocol(), 
                     networkId,
                     accountId,
-                    domainId, vmId);
+                    domainId, vmId, isNat);
         newRule = _forwardingDao.persist(newRule);
         
         if (isNat) {
@@ -281,7 +277,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
             rule.setState(State.Revoke);
             _firewallDao.update(rule.getId(), rule);
         }
-        if (NetUtils.NAT_PROTO.equals(rule.protocol) && rule.getSourcePortStart() == -1) {
+        if (rule.isOneToOneNat()) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Removing one to one nat so setting the ip back to one to one nat is false: "  + rule.getSourceIpAddress());
             }
@@ -291,7 +287,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
         }
         
         // Save and create the event
-        String ruleName = rule.getPurpose() == Purpose.Firewall ? "Firewall" : (rule.getProtocol().equals(NetUtils.NAT_PROTO) ? "ip forwarding" : "port forwarding");
+        String ruleName = rule.getPurpose() == Purpose.Firewall ? "Firewall" : (rule.isOneToOneNat() ? "ip forwarding" : "port forwarding");
         StringBuilder description = new StringBuilder("deleted ").append(ruleName).append(" rule [").append(rule.getSourceIpAddress()).append(":").append(rule.getSourcePortStart()).append("-").append(rule.getSourcePortEnd()).append("]");
         if (rule.getPurpose() == Purpose.PortForwarding) {
             PortForwardingRuleVO pfRule = (PortForwardingRuleVO)rule;
@@ -375,6 +371,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
         sb.and("ip", sb.entity().getSourceIpAddress(), Op.EQ);
         sb.and("accountId", sb.entity().getAccountId(), Op.EQ);
         sb.and("domainId", sb.entity().getDomainId(), Op.EQ);
+        sb.and("oneToOneNat", sb.entity().isOneToOneNat(), Op.EQ);
         
         SearchCriteria<PortForwardingRuleVO> sc = sb.create();
         
@@ -389,6 +386,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
                 sc.setParameters("accountId", account.getId());
             }
         }
+        
+        sc.setParameters("oneToOneNat", false);
        
         return _forwardingDao.search(sc, filter);
     }
@@ -510,7 +509,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
                         ip.getAssociatedWithNetworkId(),
                         ip.getAllocatedToAccountId(),
                         ip.getAllocatedInDomainId(),
-                        purpose);
+                        purpose, ip.isOneToOneNat());
             rules[i] = _firewallDao.persist(rules[i]);
         }
         txn.commit();

server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java

@@ -29,7 +29,6 @@ import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.net.Ip;
-import com.cloud.utils.net.NetUtils;
 
 @Local(value=PortForwardingRulesDao.class)
 public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRuleVO, Long> implements PortForwardingRulesDao {
@@ -49,6 +48,7 @@ public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRul
         AllFieldsSearch.and("protocol", AllFieldsSearch.entity().getProtocol(), Op.EQ);
         AllFieldsSearch.and("networkId", AllFieldsSearch.entity().getNetworkId(), Op.EQ);
         AllFieldsSearch.and("vmId", AllFieldsSearch.entity().getVirtualMachineId(), Op.EQ);
+        AllFieldsSearch.and("oneToOneNat", AllFieldsSearch.entity().isOneToOneNat(), Op.EQ);
         AllFieldsSearch.done();
         
         ApplicationSearch = createSearchBuilder();
@@ -120,8 +120,7 @@ public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRul
             sc.setParameters("vmId", vmId);
         }
         
-        //search for rules with protocol = nat
+        sc.setParameters("oneToOneNat", true);
-        sc.setParameters("protocol", NetUtils.NAT_PROTO);
 
         return listBy(sc, searchFilter);
     }

server/src/com/cloud/vm/UserVmManager.java

@@ -81,7 +81,7 @@ public interface UserVmManager extends VirtualMachineGuru<UserVmVO>{
 
     InstanceGroupVO getGroupForVm(long vmId);
     
-    void removeInstanceFromGroup(long vmId);
+    void removeInstanceFromInstanceGroup(long vmId);
 
 	UserVm startUserVm(long vmId) throws StorageUnavailableException,
 			ConcurrentOperationException, ExecutionException, ResourceUnavailableException, InsufficientCapacityException;

server/src/com/cloud/vm/UserVmManagerImpl.java

@@ -1135,7 +1135,8 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
             }
 	        
             _networkGroupMgr.removeInstanceFromGroups(vm.getId());
-            removeInstanceFromGroup(vm.getId());
+            
+            removeInstanceFromInstanceGroup(vm.getId());
             
             //Cleanup LB/PF rules before expunging the vm
             long vmId = vm.getId();
@@ -1811,7 +1812,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
 	}
 	
 	@Override
-	public void removeInstanceFromGroup(long vmId) {
+	public void removeInstanceFromInstanceGroup(long vmId) {
 		try {
 			List<InstanceGroupVMMapVO> groupVmMaps = _groupVMMapDao.listByInstanceId(vmId);
 	        for (InstanceGroupVMMapVO groupMap : groupVmMaps) {

setup/db/create-schema.sql

@@ -481,14 +481,15 @@ CREATE TABLE `cloud`.`op_dc_vnet_alloc` (
 CREATE TABLE `cloud`.`firewall_rules` (
   `id` bigint unsigned NOT NULL auto_increment COMMENT 'id',
   `ip_address` bigint unsigned NOT NULL COMMENT 'ip address',
-  `start_port` int(10) NOT NULL default -1 COMMENT 'starting port of a port range',
+  `start_port` int(10) NOT NULL COMMENT 'starting port of a port range',
-  `end_port` int(10) NOT NULL default -1 COMMENT 'end port of a port range',
+  `end_port` int(10) NOT NULL COMMENT 'end port of a port range',
   `state` char(32) NOT NULL COMMENT 'current state of this rule',
   `protocol` char(16) NOT NULL default 'TCP' COMMENT 'protocol to open these ports for',
   `purpose` char(32) NOT NULL COMMENT 'why are these ports opened?',
   `account_id` bigint unsigned NOT NULL COMMENT 'owner id',
   `domain_id` bigint unsigned NOT NULL COMMENT 'domain id',
   `network_id` bigint unsigned NOT NULL COMMENT 'network id',
+  `is_static_nat` int(1) unsigned NOT NULL DEFAULT 0 COMMENT '1 if firewall rule is one to one nat rule',
   `xid` char(40) NOT NULL COMMENT 'external id',
   `created` datetime COMMENT 'Date created',
   PRIMARY KEY  (`id`),

utils/src/com/cloud/utils/net/NetUtils.java

@@ -52,7 +52,6 @@ public class NetUtils {
     public final static String UDP_PROTO = "udp";
     public final static String TCP_PROTO = "tcp";
     public final static String ICMP_PROTO = "icmp";
-    public final static String NAT_PROTO = "nat"; //special value for one-to-one NAT
     
     private final static Random _rand = new Random(System.currentTimeMillis());