This is an automated email from the ASF dual-hosted git repository.

sureshanaparti pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.19 by this push:
     new d79735606bb Handle public IP race conditions (#9234)
d79735606bb is described below

commit d79735606bbe5e80ee1642bdb234028f9e7b5dcd
Author: Henrique Sato <henriquesato2...@gmail.com>
AuthorDate: Sat Jun 29 01:58:01 2024 -0300

    Handle public IP race conditions (#9234)
    
    * Lock public IP
    
    * Release IP if ID is not null
    
    * Fix NPEs
    
    Co-authored-by: Henrique Sato <henrique.s...@scclouds.com.br>
---
 .../com/cloud/network/IpAddressManagerImpl.java    |  77 +++---
 .../java/com/cloud/network/NetworkModelImpl.java   |   4 +
 .../network/firewall/FirewallManagerImpl.java      |  95 ++++----
 .../network/lb/LoadBalancingRulesManagerImpl.java  |  94 +++-----
 .../com/cloud/network/rules/RulesManagerImpl.java  | 262 ++++++++++-----------
 .../network/vpn/RemoteAccessVpnManagerImpl.java    | 130 +++++-----
 6 files changed, 336 insertions(+), 326 deletions(-)

diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java 
b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
index 0178236a21a..c8ac0c1016b 100644
--- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
@@ -725,50 +725,59 @@ public class IpAddressManagerImpl extends ManagerBase 
implements IpAddressManage
     @Override
     @DB
     public boolean disassociatePublicIpAddress(long addrId, long userId, 
Account caller) {
-
         boolean success = true;
-        IPAddressVO ipToBeDisassociated = _ipAddressDao.findById(addrId);
 
-        PublicIpQuarantine publicIpQuarantine = null;
-        // Cleanup all ip address resources - PF/LB/Static nat rules
-        if (!cleanupIpResources(addrId, userId, caller)) {
-            success = false;
-            s_logger.warn("Failed to release resources for ip address id=" + 
addrId);
-        }
+        try {
+            IPAddressVO ipToBeDisassociated = 
_ipAddressDao.acquireInLockTable(addrId);
 
-        IPAddressVO ip = markIpAsUnavailable(addrId);
-        if (ip == null) {
-            return true;
-        }
+            if (ipToBeDisassociated == null) {
+                s_logger.error(String.format("Unable to acquire lock on public 
IP %s.", addrId));
+                throw new CloudRuntimeException("Unable to acquire lock on 
public IP.");
+            }
 
-        if (s_logger.isDebugEnabled()) {
-            s_logger.debug("Releasing ip id=" + addrId + "; sourceNat = " + 
ip.isSourceNat());
-        }
+            PublicIpQuarantine publicIpQuarantine = null;
+            // Cleanup all ip address resources - PF/LB/Static nat rules
+            if (!cleanupIpResources(addrId, userId, caller)) {
+                success = false;
+                s_logger.warn("Failed to release resources for ip address id=" 
+ addrId);
+            }
 
-        if (ip.getAssociatedWithNetworkId() != null) {
-            Network network = 
_networksDao.findById(ip.getAssociatedWithNetworkId());
-            try {
-                if (!applyIpAssociations(network, rulesContinueOnErrFlag)) {
-                    s_logger.warn("Unable to apply ip address associations for 
" + network);
-                    success = false;
+            IPAddressVO ip = markIpAsUnavailable(addrId);
+            if (ip == null) {
+                return true;
+            }
+
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Releasing ip id=" + addrId + "; sourceNat = " 
+ ip.isSourceNat());
+            }
+
+            if (ip.getAssociatedWithNetworkId() != null) {
+                Network network = 
_networksDao.findById(ip.getAssociatedWithNetworkId());
+                try {
+                    if (!applyIpAssociations(network, rulesContinueOnErrFlag)) 
{
+                        s_logger.warn("Unable to apply ip address associations 
for " + network);
+                        success = false;
+                    }
+                } catch (ResourceUnavailableException e) {
+                    throw new CloudRuntimeException("We should never get to 
here because we used true when applyIpAssociations", e);
                 }
-            } catch (ResourceUnavailableException e) {
-                throw new CloudRuntimeException("We should never get to here 
because we used true when applyIpAssociations", e);
+            } else if (ip.getState() == State.Releasing) {
+                publicIpQuarantine = 
addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId());
+                _ipAddressDao.unassignIpAddress(ip.getId());
             }
-        } else if (ip.getState() == State.Releasing) {
-            publicIpQuarantine = 
addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId());
-            _ipAddressDao.unassignIpAddress(ip.getId());
-        }
 
-        
annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(),
 ip.getUuid());
+            
annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(),
 ip.getUuid());
 
-        if (success) {
-            if (ip.isPortable()) {
-                releasePortableIpAddress(addrId);
+            if (success) {
+                if (ip.isPortable()) {
+                    releasePortableIpAddress(addrId);
+                }
+                s_logger.debug("Released a public ip id=" + addrId);
+            } else if (publicIpQuarantine != null) {
+                
removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), "Public IP 
address removed from quarantine as there was an error while disassociating 
it.");
             }
-            s_logger.debug("Released a public ip id=" + addrId);
-        } else if (publicIpQuarantine != null) {
-            removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), 
"Public IP address removed from quarantine as there was an error while 
disassociating it.");
+        } finally {
+            _ipAddressDao.releaseFromLockTable(addrId);
         }
 
         return success;
diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java 
b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
index 2a604796d6e..4088e9539ea 100644
--- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java
+++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
@@ -1612,6 +1612,10 @@ public class NetworkModelImpl extends ManagerBase 
implements NetworkModel, Confi
         }
 
         NetworkVO network = _networksDao.findById(networkId);
+        if (network == null) {
+            throw new CloudRuntimeException("Could not find network associated 
with public IP.");
+        }
+
         NetworkOfferingVO offering = 
_networkOfferingDao.findById(network.getNetworkOfferingId());
         if (offering.getGuestType() != GuestType.Isolated) {
             return true;
diff --git 
a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java 
b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
index b08df5a3d1b..4ed480ea68b 100644
--- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
@@ -194,57 +194,54 @@ public class FirewallManagerImpl extends ManagerBase 
implements FirewallService,
         return createFirewallRule(sourceIpAddressId, caller, rule.getXid(), 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(),
             rule.getSourceCidrList(), null, rule.getIcmpCode(), 
rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), 
rule.getTrafficType(), rule.isDisplay());
     }
+
     //Destination CIDR capability is currently implemented for egress rules 
only. For others, the field is passed as null.
     @DB
-    protected FirewallRule createFirewallRule(final Long ipAddrId, Account 
caller, final String xId, final Integer portStart, final Integer portEnd,
-        final String protocol, final List<String> sourceCidrList, final 
List<String> destCidrList, final Integer icmpCode, final Integer icmpType, 
final Long relatedRuleId,
- final FirewallRule.FirewallRuleType type,
-            final Long networkId, final FirewallRule.TrafficType trafficType, 
final Boolean forDisplay) throws NetworkRuleConflictException {
-
+    protected FirewallRule createFirewallRule(final Long ipAddrId, Account 
caller, final String xId, final Integer portStart, final Integer portEnd, final 
String protocol,
+                                              final List<String> 
sourceCidrList, final List<String> destCidrList, final Integer icmpCode, final 
Integer icmpType, final Long relatedRuleId,
+                                              final 
FirewallRule.FirewallRuleType type, final Long networkId, final 
FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws 
NetworkRuleConflictException {
         IPAddressVO ipAddress = null;
-        if (ipAddrId != null) {
-            // this for ingress firewall rule, for egress id is null
-             ipAddress = _ipAddressDao.findById(ipAddrId);
-        // Validate ip address
-        if (ipAddress == null && type == FirewallRule.FirewallRuleType.User) {
-                throw new InvalidParameterValueException("Unable to create 
firewall rule; " + "couldn't locate IP address by id in the system");
-        }
-        _networkModel.checkIpForService(ipAddress, Service.Firewall, null);
-        }
+        try {
+            // Validate ip address
+            if (ipAddrId != null) {
+                // this for ingress firewall rule, for egress id is null
+                ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId);
+                if (ipAddress == null) {
+                    throw new InvalidParameterValueException("Unable to create 
firewall rule; " + "couldn't locate IP address by id in the system");
+                }
+                _networkModel.checkIpForService(ipAddress, Service.Firewall, 
null);
+            }
 
-        validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, 
Purpose.Firewall, type, networkId, trafficType);
+            validateFirewallRule(caller, ipAddress, portStart, portEnd, 
protocol, Purpose.Firewall, type, networkId, trafficType);
 
-        // icmp code and icmp type can't be passed in for any other protocol 
rather than icmp
-        if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != 
null || icmpType != null)) {
-            throw new InvalidParameterValueException("Can specify icmpCode and 
icmpType for ICMP protocol only");
-        }
+            // icmp code and icmp type can't be passed in for any other 
protocol rather than icmp
+            if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode 
!= null || icmpType != null)) {
+                throw new InvalidParameterValueException("Can specify icmpCode 
and icmpType for ICMP protocol only");
+            }
 
-        if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != 
null || portEnd != null)) {
-            throw new InvalidParameterValueException("Can't specify start/end 
port when protocol is ICMP");
-        }
+            if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart 
!= null || portEnd != null)) {
+                throw new InvalidParameterValueException("Can't specify 
start/end port when protocol is ICMP");
+            }
 
-        Long accountId = null;
-        Long domainId = null;
+            Long accountId = null;
+            Long domainId = null;
 
-        if (ipAddress != null) {
-            //Ingress firewall rule
-            accountId = ipAddress.getAllocatedToAccountId();
-            domainId = ipAddress.getAllocatedInDomainId();
-        } else if (networkId != null) {
-            //egress firewall rule
+            if (ipAddress != null) {
+                //Ingress firewall rule
+                accountId = ipAddress.getAllocatedToAccountId();
+                domainId = ipAddress.getAllocatedInDomainId();
+            } else if (networkId != null) {
+                //egress firewall rule
                 Network network = _networkModel.getNetwork(networkId);
                 accountId = network.getAccountId();
                 domainId = network.getDomainId();
-        }
+            }
 
-        final Long accountIdFinal = accountId;
-        final Long domainIdFinal = domainId;
-        return Transaction.execute(new 
TransactionCallbackWithException<FirewallRuleVO, 
NetworkRuleConflictException>() {
-            @Override
-            public FirewallRuleVO doInTransaction(TransactionStatus status) 
throws NetworkRuleConflictException {
-                FirewallRuleVO newRule =
-                        new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, 
protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, 
Purpose.Firewall,
-                                sourceCidrList, destCidrList, icmpCode, 
icmpType, relatedRuleId, trafficType);
+            final Long accountIdFinal = accountId;
+            final Long domainIdFinal = domainId;
+            return 
Transaction.execute((TransactionCallbackWithException<FirewallRuleVO, 
NetworkRuleConflictException>) status -> {
+                FirewallRuleVO newRule = new FirewallRuleVO(xId, ipAddrId, 
portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, 
domainIdFinal, Purpose.Firewall,
+                        sourceCidrList, destCidrList, icmpCode, icmpType, 
relatedRuleId, trafficType);
                 newRule.setType(type);
                 if (forDisplay != null) {
                     newRule.setDisplay(forDisplay);
@@ -261,8 +258,12 @@ public class FirewallManagerImpl extends ManagerBase 
implements FirewallService,
                 CallContext.current().putContextParameter(FirewallRule.class, 
newRule.getId());
 
                 return newRule;
+            });
+        } finally {
+            if (ipAddrId != null) {
+                _ipAddressDao.releaseFromLockTable(ipAddrId);
             }
-        });
+        }
     }
 
     @Override
@@ -668,9 +669,19 @@ public class FirewallManagerImpl extends ManagerBase 
implements FirewallService,
     }
 
     @Override
+    @DB
     public boolean applyIngressFirewallRules(long ipId, Account caller) throws 
ResourceUnavailableException {
-        List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurpose(ipId, 
Purpose.Firewall);
-        return applyFirewallRules(rules, false, caller);
+        try {
+            IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipId);
+            if (ipAddress == null) {
+                s_logger.error(String.format("Unable to acquire lock for 
public IP [%s].", ipId));
+                throw new CloudRuntimeException("Unable to acquire lock for 
public IP.");
+            }
+            List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurpose(ipId, 
Purpose.Firewall);
+            return applyFirewallRules(rules, false, caller);
+        } finally {
+            _ipAddressDao.releaseFromLockTable(ipId);
+        }
     }
 
     @Override
diff --git 
a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 
b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
index 8cb8972e295..e0d8082c0d9 100644
--- 
a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
+++ 
b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
@@ -1814,13 +1814,12 @@ public class LoadBalancingRulesManagerImpl<Type> 
extends ManagerBase implements
         }
         return cidr;
     }
+
     @DB
     @Override
-    public LoadBalancer createPublicLoadBalancer(final String xId, final 
String name, final String description, final int srcPort, final int destPort,
- final long sourceIpId,
-            final String protocol, final String algorithm, final boolean 
openFirewall, final CallContext caller, final String lbProtocol, final Boolean 
forDisplay, String cidrList)
-            throws NetworkRuleConflictException {
-
+    public LoadBalancer createPublicLoadBalancer(final String xId, final 
String name, final String description, final int srcPort, final int destPort, 
final long sourceIpId,
+                                                 final String protocol, final 
String algorithm, final boolean openFirewall, final CallContext caller, final 
String lbProtocol,
+                                                 final Boolean forDisplay, 
String cidrList) throws NetworkRuleConflictException {
         if (!NetUtils.isValidPort(destPort)) {
             throw new InvalidParameterValueException("privatePort is an 
invalid value: " + destPort);
         }
@@ -1829,55 +1828,41 @@ public class LoadBalancingRulesManagerImpl<Type> 
extends ManagerBase implements
             throw new InvalidParameterValueException("Invalid algorithm: " + 
algorithm);
         }
 
-        final IPAddressVO ipAddr = _ipAddressDao.findById(sourceIpId);
-        // make sure ip address exists
-        if (ipAddr == null || !ipAddr.readyToUse()) {
-            InvalidParameterValueException ex = new 
InvalidParameterValueException("Unable to create load balancer rule, invalid IP 
address id specified");
-            if (ipAddr == null) {
-                ex.addProxyObject(String.valueOf(sourceIpId), "sourceIpId");
-            } else {
+        try {
+            final IPAddressVO ipAddr = 
_ipAddressDao.acquireInLockTable(sourceIpId);
+
+            // make sure ip address exists
+            if (ipAddr == null || !ipAddr.readyToUse()) {
+                InvalidParameterValueException ex = new 
InvalidParameterValueException("Unable to create load balancer rule, invalid IP 
address id specified");
+                if (ipAddr == null) {
+                    ex.addProxyObject(String.valueOf(sourceIpId), 
"sourceIpId");
+                } else {
+                    ex.addProxyObject(ipAddr.getUuid(), "sourceIpId");
+                }
+                throw ex;
+            } else if (ipAddr.isOneToOneNat()) {
+                InvalidParameterValueException ex = new 
InvalidParameterValueException("Unable to create load balancer rule; specified 
sourceip id has static nat enabled");
                 ex.addProxyObject(ipAddr.getUuid(), "sourceIpId");
+                throw ex;
             }
-            throw ex;
-        } else if (ipAddr.isOneToOneNat()) {
-            InvalidParameterValueException ex = new 
InvalidParameterValueException("Unable to create load balancer rule; specified 
sourceip id has static nat enabled");
-            ex.addProxyObject(ipAddr.getUuid(), "sourceIpId");
-            throw ex;
-        }
 
-        _accountMgr.checkAccess(caller.getCallingAccount(), null, true, 
ipAddr);
+            _accountMgr.checkAccess(caller.getCallingAccount(), null, true, 
ipAddr);
 
-        final Long networkId = ipAddr.getAssociatedWithNetworkId();
-        if (networkId == null) {
-            InvalidParameterValueException ex =
-                new InvalidParameterValueException("Unable to create load 
balancer rule ; specified sourceip id is not associated with any network");
-            ex.addProxyObject(ipAddr.getUuid(), "sourceIpId");
-            throw ex;
-        }
-
-        // verify that lb service is supported by the network
-        isLbServiceSupportedInNetwork(networkId, Scheme.Public);
-
-        _firewallMgr.validateFirewallRule(caller.getCallingAccount(), ipAddr, 
srcPort, srcPort, protocol, Purpose.LoadBalancing, FirewallRuleType.User, 
networkId, null);
+            final Long networkId = ipAddr.getAssociatedWithNetworkId();
+            if (networkId == null) {
+                InvalidParameterValueException ex =
+                        new InvalidParameterValueException("Unable to create 
load balancer rule ; specified sourceip id is not associated with any network");
+                ex.addProxyObject(ipAddr.getUuid(), "sourceIpId");
+                throw ex;
+            }
 
-        LoadBalancerVO newRule =
-            new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, 
destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(),
-                ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList);
+            // verify that lb service is supported by the network
+            isLbServiceSupportedInNetwork(networkId, Scheme.Public);
 
-        // verify rule is supported by Lb provider of the network
-        Ip sourceIp = getSourceIp(newRule);
-        LoadBalancingRule loadBalancing =
-            new LoadBalancingRule(newRule, new ArrayList<LbDestination>(), new 
ArrayList<LbStickinessPolicy>(), new ArrayList<LbHealthCheckPolicy>(), 
sourceIp, null,
-                lbProtocol);
-        if (!validateLbRule(loadBalancing)) {
-            throw new InvalidParameterValueException("LB service provider 
cannot support this rule");
-        }
+            _firewallMgr.validateFirewallRule(caller.getCallingAccount(), 
ipAddr, srcPort, srcPort, protocol, Purpose.LoadBalancing, 
FirewallRuleType.User, networkId, null);
 
-        return Transaction.execute(new 
TransactionCallbackWithException<LoadBalancerVO, 
NetworkRuleConflictException>() {
-            @Override
-            public LoadBalancerVO doInTransaction(TransactionStatus status) 
throws NetworkRuleConflictException {
-                LoadBalancerVO newRule =
-                    new LoadBalancerVO(xId, name, description, sourceIpId, 
srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(),
+            return 
Transaction.execute((TransactionCallbackWithException<LoadBalancerVO, 
NetworkRuleConflictException>) status -> {
+                LoadBalancerVO newRule = new LoadBalancerVO(xId, name, 
description, sourceIpId, srcPort, destPort, algorithm, networkId, 
ipAddr.getAllocatedToAccountId(),
                         ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList);
 
                 if (forDisplay != null) {
@@ -1886,9 +1871,7 @@ public class LoadBalancingRulesManagerImpl<Type> extends 
ManagerBase implements
 
                 // verify rule is supported by Lb provider of the network
                 Ip sourceIp = getSourceIp(newRule);
-                LoadBalancingRule loadBalancing =
-                    new LoadBalancingRule(newRule, new 
ArrayList<LbDestination>(), new ArrayList<LbStickinessPolicy>(), new 
ArrayList<LbHealthCheckPolicy>(), sourceIp,
-                        null, lbProtocol);
+                LoadBalancingRule loadBalancing = new 
LoadBalancingRule(newRule, new ArrayList<>(), new ArrayList<>(), new 
ArrayList<>(), sourceIp, null, lbProtocol);
                 if (!validateLbRule(loadBalancing)) {
                     throw new InvalidParameterValueException("LB service 
provider cannot support this rule");
                 }
@@ -1908,10 +1891,10 @@ public class LoadBalancingRulesManagerImpl<Type> 
extends ManagerBase implements
                         throw new CloudRuntimeException("Unable to update the 
state to add for " + newRule);
                     }
                     s_logger.debug("Load balancer " + newRule.getId() + " for 
Ip address id=" + sourceIpId + ", public port " + srcPort + ", private port " + 
destPort +
-                        " is added successfully.");
+                            " is added successfully.");
                     CallContext.current().setEventDetails("Load balancer Id: " 
+ newRule.getId());
                     
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_LOAD_BALANCER_CREATE, 
ipAddr.getAllocatedToAccountId(), ipAddr.getDataCenterId(), newRule.getId(),
-                        null, LoadBalancingRule.class.getName(), 
newRule.getUuid());
+                            null, LoadBalancingRule.class.getName(), 
newRule.getUuid());
 
                     return newRule;
                 } catch (Exception e) {
@@ -1926,9 +1909,10 @@ public class LoadBalancingRulesManagerImpl<Type> extends 
ManagerBase implements
                         removeLBRule(newRule);
                     }
                 }
-            }
-        });
-
+            });
+        } finally {
+            _ipAddressDao.releaseFromLockTable(sourceIpId);
+        }
     }
 
     @Override
diff --git a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java 
b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
index 624fbfb9d24..7686a8d7887 100644
--- a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
@@ -207,124 +207,122 @@ public class RulesManagerImpl extends ManagerBase 
implements RulesManager, Rules
 
         final Long ipAddrId = rule.getSourceIpAddressId();
 
-        IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId);
-
-        // Validate ip address
-        if (ipAddress == null) {
-            throw new InvalidParameterValueException("Unable to create port 
forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system");
-        } else if (ipAddress.isOneToOneNat()) {
-            throw new InvalidParameterValueException("Unable to create port 
forwarding rule; ip id=" + ipAddrId + " has static nat enabled");
-        }
+        try {
+            IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId);
 
-        final Long networkId = rule.getNetworkId();
-        Network network = _networkModel.getNetwork(networkId);
-        //associate ip address to network (if needed)
-        boolean performedIpAssoc = false;
-        Nic guestNic;
-        if (ipAddress.getAssociatedWithNetworkId() == null) {
-            boolean assignToVpcNtwk = network.getVpcId() != null && 
ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == 
network.getVpcId();
-            if (assignToVpcNtwk) {
-                _networkModel.checkIpForService(ipAddress, 
Service.PortForwarding, networkId);
+            // Validate ip address
+            if (ipAddress == null) {
+                throw new InvalidParameterValueException("Unable to create 
port forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system");
+            } else if (ipAddress.isOneToOneNat()) {
+                throw new InvalidParameterValueException("Unable to create 
port forwarding rule; ip id=" + ipAddrId + " has static nat enabled");
+            }
 
-                s_logger.debug("The ip is not associated with the VPC network 
id=" + networkId + ", so assigning");
-                try {
-                    ipAddress = _ipAddrMgr.associateIPToGuestNetwork(ipAddrId, 
networkId, false);
-                    performedIpAssoc = true;
-                } catch (Exception ex) {
-                    throw new CloudRuntimeException("Failed to associate ip to 
VPC network as " + "a part of port forwarding rule creation");
+            final Long networkId = rule.getNetworkId();
+            Network network = _networkModel.getNetwork(networkId);
+            //associate ip address to network (if needed)
+            boolean performedIpAssoc = false;
+            Nic guestNic;
+            if (ipAddress.getAssociatedWithNetworkId() == null) {
+                boolean assignToVpcNtwk = network.getVpcId() != null && 
ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == 
network.getVpcId();
+                if (assignToVpcNtwk) {
+                    _networkModel.checkIpForService(ipAddress, 
Service.PortForwarding, networkId);
+
+                    s_logger.debug("The ip is not associated with the VPC 
network id=" + networkId + ", so assigning");
+                    try {
+                        ipAddress = 
_ipAddrMgr.associateIPToGuestNetwork(ipAddrId, networkId, false);
+                        performedIpAssoc = true;
+                    } catch (Exception ex) {
+                        throw new CloudRuntimeException("Failed to associate 
ip to VPC network as " + "a part of port forwarding rule creation");
+                    }
                 }
+            } else {
+                _networkModel.checkIpForService(ipAddress, 
Service.PortForwarding, null);
             }
-        } else {
-            _networkModel.checkIpForService(ipAddress, Service.PortForwarding, 
null);
-        }
 
-        if (ipAddress.getAssociatedWithNetworkId() == null) {
-            throw new InvalidParameterValueException("Ip address " + ipAddress 
+ " is not assigned to the network " + network);
-        }
+            if (ipAddress.getAssociatedWithNetworkId() == null) {
+                throw new InvalidParameterValueException("Ip address " + 
ipAddress + " is not assigned to the network " + network);
+            }
 
-        try {
-            _firewallMgr.validateFirewallRule(caller, ipAddress, 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), 
Purpose.PortForwarding,
-                FirewallRuleType.User, networkId, rule.getTrafficType());
+            try {
+                _firewallMgr.validateFirewallRule(caller, ipAddress, 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), 
Purpose.PortForwarding,
+                        FirewallRuleType.User, networkId, 
rule.getTrafficType());
 
-            final Long accountId = ipAddress.getAllocatedToAccountId();
-            final Long domainId = ipAddress.getAllocatedInDomainId();
+                final Long accountId = ipAddress.getAllocatedToAccountId();
+                final Long domainId = ipAddress.getAllocatedInDomainId();
 
-            // start port can't be bigger than end port
-            if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) 
{
-                throw new InvalidParameterValueException("Start port can't be 
bigger than end port");
-            }
+                // start port can't be bigger than end port
+                if (rule.getDestinationPortStart() > 
rule.getDestinationPortEnd()) {
+                    throw new InvalidParameterValueException("Start port can't 
be bigger than end port");
+                }
 
-            // check that the port ranges are of equal size
-            if ((rule.getDestinationPortEnd() - 
rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - 
rule.getSourcePortStart())) {
-                throw new InvalidParameterValueException("Source port and 
destination port ranges should be of equal sizes.");
-            }
+                // check that the port ranges are of equal size
+                if ((rule.getDestinationPortEnd() - 
rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - 
rule.getSourcePortStart())) {
+                    throw new InvalidParameterValueException("Source port and 
destination port ranges should be of equal sizes.");
+                }
 
-            // validate user VM exists
-            UserVm vm = _vmDao.findById(vmId);
-            if (vm == null) {
-                throw new InvalidParameterValueException("Unable to create 
port forwarding rule on address " + ipAddress + ", invalid virtual machine id 
specified (" +
-                    vmId + ").");
-            } else if (vm.getState() == VirtualMachine.State.Destroyed || 
vm.getState() == VirtualMachine.State.Expunging) {
-                throw new InvalidParameterValueException("Invalid user vm: " + 
vm.getId());
-            }
+                // validate user VM exists
+                UserVm vm = _vmDao.findById(vmId);
+                if (vm == null) {
+                    throw new InvalidParameterValueException("Unable to create 
port forwarding rule on address " + ipAddress + ", invalid virtual machine id 
specified (" +
+                            vmId + ").");
+                } else if (vm.getState() == VirtualMachine.State.Destroyed || 
vm.getState() == VirtualMachine.State.Expunging) {
+                    throw new InvalidParameterValueException("Invalid user vm: 
" + vm.getId());
+                }
 
-            // Verify that vm has nic in the network
-            Ip dstIp = rule.getDestinationIpAddress();
-            guestNic = _networkModel.getNicInNetwork(vmId, networkId);
-            if (guestNic == null || guestNic.getIPv4Address() == null) {
-                throw new InvalidParameterValueException("Vm doesn't belong to 
network associated with ipAddress");
-            } else {
-                dstIp = new Ip(guestNic.getIPv4Address());
-            }
+                // Verify that vm has nic in the network
+                Ip dstIp = rule.getDestinationIpAddress();
+                guestNic = _networkModel.getNicInNetwork(vmId, networkId);
+                if (guestNic == null || guestNic.getIPv4Address() == null) {
+                    throw new InvalidParameterValueException("Vm doesn't 
belong to network associated with ipAddress");
+                } else {
+                    dstIp = new Ip(guestNic.getIPv4Address());
+                }
 
-            if (vmIp != null) {
-                //vm ip is passed so it can be primary or secondary ip 
addreess.
-                if (!dstIp.equals(vmIp)) {
-                    //the vm ip is secondary ip to the nic.
-                    // is vmIp is secondary ip or not
-                    NicSecondaryIp secondaryIp = 
_nicSecondaryDao.findByIp4AddressAndNicId(vmIp.toString(), guestNic.getId());
-                    if (secondaryIp == null) {
-                        throw new InvalidParameterValueException("IP Address 
is not in the VM nic's network ");
+                if (vmIp != null) {
+                    //vm ip is passed so it can be primary or secondary ip 
addreess.
+                    if (!dstIp.equals(vmIp)) {
+                        //the vm ip is secondary ip to the nic.
+                        // is vmIp is secondary ip or not
+                        NicSecondaryIp secondaryIp = 
_nicSecondaryDao.findByIp4AddressAndNicId(vmIp.toString(), guestNic.getId());
+                        if (secondaryIp == null) {
+                            throw new InvalidParameterValueException("IP 
Address is not in the VM nic's network ");
+                        }
+                        dstIp = vmIp;
                     }
-                    dstIp = vmIp;
                 }
-            }
-
-            //if start port and end port are passed in, and they are not equal 
to each other, perform the validation
-            boolean validatePortRange = false;
-            if (rule.getSourcePortStart().intValue() != 
rule.getSourcePortEnd().intValue() || rule.getDestinationPortStart() != 
rule.getDestinationPortEnd()) {
-                validatePortRange = true;
-            }
 
-            if (validatePortRange) {
-                //source start port and source dest port should be the same. 
The same applies to dest ports
-                if (rule.getSourcePortStart().intValue() != 
rule.getDestinationPortStart()) {
-                    throw new InvalidParameterValueException("Private port 
start should be equal to public port start");
+                //if start port and end port are passed in, and they are not 
equal to each other, perform the validation
+                boolean validatePortRange = false;
+                if (rule.getSourcePortStart().intValue() != 
rule.getSourcePortEnd().intValue() || rule.getDestinationPortStart() != 
rule.getDestinationPortEnd()) {
+                    validatePortRange = true;
                 }
 
-                if (rule.getSourcePortEnd().intValue() != 
rule.getDestinationPortEnd()) {
-                    throw new InvalidParameterValueException("Private port end 
should be equal to public port end");
+                if (validatePortRange) {
+                    //source start port and source dest port should be the 
same. The same applies to dest ports
+                    if (rule.getSourcePortStart().intValue() != 
rule.getDestinationPortStart()) {
+                        throw new InvalidParameterValueException("Private port 
start should be equal to public port start");
+                    }
+
+                    if (rule.getSourcePortEnd().intValue() != 
rule.getDestinationPortEnd()) {
+                        throw new InvalidParameterValueException("Private port 
end should be equal to public port end");
+                    }
                 }
-            }
 
-            final Ip dstIpFinal = dstIp;
-            final IPAddressVO ipAddressFinal = ipAddress;
-            return Transaction.execute(new 
TransactionCallbackWithException<PortForwardingRuleVO, 
NetworkRuleConflictException>() {
-                @Override
-                public PortForwardingRuleVO doInTransaction(TransactionStatus 
status) throws NetworkRuleConflictException {
+                final Ip dstIpFinal = dstIp;
+                final IPAddressVO ipAddressFinal = ipAddress;
+                return 
Transaction.execute((TransactionCallbackWithException<PortForwardingRuleVO, 
NetworkRuleConflictException>) status -> {
                     PortForwardingRuleVO newRule =
-                        new PortForwardingRuleVO(rule.getXid(), 
rule.getSourceIpAddressId(), rule.getSourcePortStart(), 
rule.getSourcePortEnd(), dstIpFinal,
-                            rule.getDestinationPortStart(), 
rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, 
accountId, domainId, vmId);
+                            new PortForwardingRuleVO(rule.getXid(), 
rule.getSourceIpAddressId(), rule.getSourcePortStart(), 
rule.getSourcePortEnd(), dstIpFinal,
+                                    rule.getDestinationPortStart(), 
rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, 
accountId, domainId, vmId);
 
                     if (forDisplay != null) {
                         newRule.setDisplay(forDisplay);
                     }
                     newRule = _portForwardingDao.persist(newRule);
-
                     // create firewallRule for 0.0.0.0/0 cidr
                     if (openFirewall) {
                         _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, 
null,
-                            newRule.getId(), networkId);
+                                newRule.getId(), networkId);
                     }
 
                     try {
@@ -334,7 +332,7 @@ public class RulesManagerImpl extends ManagerBase 
implements RulesManager, Rules
                         }
                         CallContext.current().setEventDetails("Rule Id: " + 
newRule.getId());
                         
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, 
newRule.getAccountId(), ipAddressFinal.getDataCenterId(), newRule.getId(), null,
-                            PortForwardingRule.class.getName(), 
newRule.getUuid());
+                                PortForwardingRule.class.getName(), 
newRule.getUuid());
                         return newRule;
                     } catch (Exception e) {
                         if (newRule != null) {
@@ -349,16 +347,17 @@ public class RulesManagerImpl extends ManagerBase 
implements RulesManager, Rules
 
                         throw new CloudRuntimeException("Unable to add rule 
for the ip id=" + ipAddrId, e);
                     }
+                });
+            } finally {
+                // release ip address if ipassoc was perfored
+                if (performedIpAssoc) {
+                    //if the rule is the last one for the ip address assigned 
to VPC, unassign it from the network
+                    IpAddress ip = _ipAddressDao.findById(ipAddress.getId());
+                    _vpcMgr.unassignIPFromVpcNetwork(ip.getId(), networkId);
                 }
-            });
-
-        } finally {
-            // release ip address if ipassoc was perfored
-            if (performedIpAssoc) {
-                //if the rule is the last one for the ip address assigned to 
VPC, unassign it from the network
-                IpAddress ip = _ipAddressDao.findById(ipAddress.getId());
-                _vpcMgr.unassignIPFromVpcNetwork(ip.getId(), networkId);
             }
+        } finally {
+            _ipAddressDao.releaseFromLockTable(ipAddrId);
         }
     }
 
@@ -370,46 +369,44 @@ public class RulesManagerImpl extends ManagerBase 
implements RulesManager, Rules
 
         final Long ipAddrId = rule.getSourceIpAddressId();
 
-        IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId);
-
-        // Validate ip address
-        if (ipAddress == null) {
-            throw new InvalidParameterValueException("Unable to create static 
nat rule; ip id=" + ipAddrId + " doesn't exist in the system");
-        } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || 
ipAddress.getAssociatedWithVmId() == null) {
-            throw new NetworkRuleConflictException("Can't do static nat on ip 
address: " + ipAddress.getAddress());
-        }
+        try {
+            IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId);
 
-        _firewallMgr.validateFirewallRule(caller, ipAddress, 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), 
Purpose.StaticNat,
-            FirewallRuleType.User, null, rule.getTrafficType());
+            // Validate ip address
+            if (ipAddress == null) {
+                throw new InvalidParameterValueException("Unable to create 
static nat rule; ip id=" + ipAddrId + " doesn't exist in the system");
+            } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() 
|| ipAddress.getAssociatedWithVmId() == null) {
+                throw new NetworkRuleConflictException("Can't do static nat on 
ip address: " + ipAddress.getAddress());
+            }
 
-        final Long networkId = ipAddress.getAssociatedWithNetworkId();
-        final Long accountId = ipAddress.getAllocatedToAccountId();
-        final Long domainId = ipAddress.getAllocatedInDomainId();
+            _firewallMgr.validateFirewallRule(caller, ipAddress, 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), 
Purpose.StaticNat,
+                    FirewallRuleType.User, null, rule.getTrafficType());
 
-        _networkModel.checkIpForService(ipAddress, Service.StaticNat, null);
+            final Long networkId = ipAddress.getAssociatedWithNetworkId();
+            final Long accountId = ipAddress.getAllocatedToAccountId();
+            final Long domainId = ipAddress.getAllocatedInDomainId();
 
-        Network network = _networkModel.getNetwork(networkId);
-        NetworkOffering off = _entityMgr.findById(NetworkOffering.class, 
network.getNetworkOfferingId());
-        if (off.isElasticIp()) {
-            throw new InvalidParameterValueException("Can't create ip 
forwarding rules for the network where elasticIP service is enabled");
-        }
+            _networkModel.checkIpForService(ipAddress, Service.StaticNat, 
null);
 
-        //String dstIp = 
_networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId);
-        final String dstIp = ipAddress.getVmIp();
-        return Transaction.execute(new 
TransactionCallbackWithException<StaticNatRule, NetworkRuleConflictException>() 
{
-            @Override
-            public StaticNatRule doInTransaction(TransactionStatus status) 
throws NetworkRuleConflictException {
+            Network network = _networkModel.getNetwork(networkId);
+            NetworkOffering off = _entityMgr.findById(NetworkOffering.class, 
network.getNetworkOfferingId());
+            if (off.isElasticIp()) {
+                throw new InvalidParameterValueException("Can't create ip 
forwarding rules for the network where elasticIP service is enabled");
+            }
 
+            //String dstIp = 
_networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId);
+            final String dstIp = ipAddress.getVmIp();
+            return 
Transaction.execute((TransactionCallbackWithException<StaticNatRule, 
NetworkRuleConflictException>) status -> {
                 FirewallRuleVO newRule =
-                    new FirewallRuleVO(rule.getXid(), 
rule.getSourceIpAddressId(), rule.getSourcePortStart(), 
rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(),
-                        networkId, accountId, domainId, rule.getPurpose(), 
null, null, null, null, null);
+                        new FirewallRuleVO(rule.getXid(), 
rule.getSourceIpAddressId(), rule.getSourcePortStart(), 
rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(),
+                                networkId, accountId, domainId, 
rule.getPurpose(), null, null, null, null, null);
 
                 newRule = _firewallDao.persist(newRule);
 
                 // create firewallRule for 0.0.0.0/0 cidr
                 if (openFirewall) {
                     _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, 
rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, 
null,
-                        newRule.getId(), networkId);
+                            newRule.getId(), networkId);
                 }
 
                 try {
@@ -419,11 +416,9 @@ public class RulesManagerImpl extends ManagerBase 
implements RulesManager, Rules
                     }
                     CallContext.current().setEventDetails("Rule Id: " + 
newRule.getId());
                     
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, 
newRule.getAccountId(), 0, newRule.getId(), null, FirewallRule.class.getName(),
-                        newRule.getUuid());
+                            newRule.getUuid());
 
-                    StaticNatRule staticNatRule = new 
StaticNatRuleImpl(newRule, dstIp);
-
-                    return staticNatRule;
+                    return new StaticNatRuleImpl(newRule, dstIp);
                 } catch (Exception e) {
                     if (newRule != null) {
                         // no need to apply the rule as it wasn't programmed 
on the backend yet
@@ -436,9 +431,10 @@ public class RulesManagerImpl extends ManagerBase 
implements RulesManager, Rules
                     }
                     throw new CloudRuntimeException("Unable to add static nat 
rule for the ip id=" + newRule.getSourceIpAddressId(), e);
                 }
-            }
-        });
-
+            });
+        } finally {
+            _ipAddressDao.releaseFromLockTable(ipAddrId);
+        }
     }
 
     @Override
diff --git 
a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java 
b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java
index 61d247d7b8a..9ff599b47c6 100644
--- a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java
@@ -155,6 +155,7 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase 
implements RemoteAcc
         return vpns;
     }
 
+
     @Override
     @DB
     public RemoteAccessVpn createRemoteAccessVpn(final long publicIpId, String 
ipRange, boolean openFirewall, final Boolean forDisplay) throws 
NetworkRuleConflictException {
@@ -172,92 +173,97 @@ public class RemoteAccessVpnManagerImpl extends 
ManagerBase implements RemoteAcc
             throw new InvalidParameterValueException("The Ip address is not 
ready to be used yet: " + ipAddr.getAddress());
         }
 
-        IPAddressVO ipAddress = _ipAddressDao.findById(publicIpId);
-
-        Long networkId = ipAddress.getAssociatedWithNetworkId();
-        if (networkId != null) {
-            _networkMgr.checkIpForService(ipAddress, Service.Vpn, null);
-        }
-
-        final Long vpcId = ipAddress.getVpcId();
-        if (vpcId != null && ipAddress.isSourceNat()) {
-            assert networkId == null;
-            openFirewall = false;
-        }
-
-        final boolean openFirewallFinal = openFirewall;
-
-        if (networkId == null && vpcId == null) {
-            throw new InvalidParameterValueException("Unable to create remote 
access vpn for the ipAddress: " + ipAddr.getAddress().addr() +
-                    " as ip is not associated with any network or VPC");
-        }
-
-        RemoteAccessVpnVO vpnVO = 
_remoteAccessVpnDao.findByPublicIpAddress(publicIpId);
+        try {
+            IPAddressVO ipAddress = 
_ipAddressDao.acquireInLockTable(publicIpId);
 
-        if (vpnVO != null) {
-            if (vpnVO.getState() == RemoteAccessVpn.State.Added) {
-                return vpnVO;
+            if (ipAddress == null) {
+                s_logger.error(String.format("Unable to acquire lock on public 
IP %s.", publicIpId));
+                throw new CloudRuntimeException("Unable to acquire lock on 
public IP.");
             }
 
-            throw new InvalidParameterValueException(String.format("A remote 
Access VPN already exists for the public IP address [%s].", 
ipAddr.getAddress().toString()));
-        }
+            Long networkId = ipAddress.getAssociatedWithNetworkId();
+            if (networkId != null) {
+                _networkMgr.checkIpForService(ipAddress, Service.Vpn, null);
+            }
 
-        if (ipRange == null) {
-            ipRange = 
RemoteAccessVpnClientIpRange.valueIn(ipAddr.getAccountId());
-        }
+            final Long vpcId = ipAddress.getVpcId();
+            if (vpcId != null && ipAddress.isSourceNat()) {
+                assert networkId == null;
+                openFirewall = false;
+            }
 
-        validateIpRange(ipRange, InvalidParameterValueException.class);
+            final boolean openFirewallFinal = openFirewall;
 
-        String[] range = ipRange.split("-");
+            if (networkId == null && vpcId == null) {
+                throw new InvalidParameterValueException("Unable to create 
remote access vpn for the ipAddress: " + ipAddr.getAddress().addr() +
+                        " as ip is not associated with any network or VPC");
+            }
 
-        Pair<String, Integer> cidr = null;
+            RemoteAccessVpnVO vpnVO = 
_remoteAccessVpnDao.findByPublicIpAddress(publicIpId);
 
-        if (networkId != null) {
-            long ipAddressOwner = ipAddr.getAccountId();
-            vpnVO = 
_remoteAccessVpnDao.findByAccountAndNetwork(ipAddressOwner, networkId);
             if (vpnVO != null) {
                 if (vpnVO.getState() == RemoteAccessVpn.State.Added) {
                     return vpnVO;
                 }
 
-                throw new InvalidParameterValueException(String.format("A 
remote access VPN already exists for the account [%s].", ipAddressOwner));
+                throw new InvalidParameterValueException(String.format("A 
remote Access VPN already exists for the public IP address [%s].", 
ipAddr.getAddress().toString()));
             }
-            Network network = _networkMgr.getNetwork(networkId);
-            if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), 
Service.Vpn)) {
-                throw new InvalidParameterValueException("Vpn service is not 
supported in network id=" + ipAddr.getAssociatedWithNetworkId());
+
+            if (ipRange == null) {
+                ipRange = 
RemoteAccessVpnClientIpRange.valueIn(ipAddr.getAccountId());
             }
-            cidr = NetUtils.getCidr(network.getCidr());
-        } else {
-            Vpc vpc = _vpcDao.findById(vpcId);
-            cidr = NetUtils.getCidr(vpc.getCidr());
-        }
 
-        String[] guestIpRange = NetUtils.getIpRangeFromCidr(cidr.first(), 
cidr.second());
-        if (NetUtils.ipRangesOverlap(range[0], range[1], guestIpRange[0], 
guestIpRange[1])) {
-            throw new InvalidParameterValueException("Invalid ip range: " + 
ipRange + " overlaps with guest ip range " + guestIpRange[0] + "-" + 
guestIpRange[1]);
-        }
+            validateIpRange(ipRange, InvalidParameterValueException.class);
 
-        long startIp = NetUtils.ip2Long(range[0]);
-        final String newIpRange = NetUtils.long2Ip(++startIp) + "-" + range[1];
-        final String sharedSecret = 
PasswordGenerator.generatePresharedKey(_pskLength);
+            String[] range = ipRange.split("-");
 
-        return Transaction.execute(new 
TransactionCallbackWithException<RemoteAccessVpn, 
NetworkRuleConflictException>() {
-            @Override
-            public RemoteAccessVpn doInTransaction(TransactionStatus status) 
throws NetworkRuleConflictException {
+            Pair<String, Integer> cidr = null;
+
+            if (networkId != null) {
+                long ipAddressOwner = ipAddr.getAccountId();
+                vpnVO = 
_remoteAccessVpnDao.findByAccountAndNetwork(ipAddressOwner, networkId);
+                if (vpnVO != null) {
+                    if (vpnVO.getState() == RemoteAccessVpn.State.Added) {
+                        return vpnVO;
+                    }
+
+                    throw new InvalidParameterValueException(String.format("A 
remote access VPN already exists for the account [%s].", ipAddressOwner));
+                }
+                Network network = _networkMgr.getNetwork(networkId);
+                if 
(!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.Vpn)) {
+                    throw new InvalidParameterValueException("Vpn service is 
not supported in network id=" + ipAddr.getAssociatedWithNetworkId());
+                }
+                cidr = NetUtils.getCidr(network.getCidr());
+            } else {
+                Vpc vpc = _vpcDao.findById(vpcId);
+                cidr = NetUtils.getCidr(vpc.getCidr());
+            }
+
+            String[] guestIpRange = NetUtils.getIpRangeFromCidr(cidr.first(), 
cidr.second());
+            if (NetUtils.ipRangesOverlap(range[0], range[1], guestIpRange[0], 
guestIpRange[1])) {
+                throw new InvalidParameterValueException("Invalid ip range: " 
+ ipRange + " overlaps with guest ip range " + guestIpRange[0] + "-" + 
guestIpRange[1]);
+            }
+
+            long startIp = NetUtils.ip2Long(range[0]);
+            final String newIpRange = NetUtils.long2Ip(++startIp) + "-" + 
range[1];
+            final String sharedSecret = 
PasswordGenerator.generatePresharedKey(_pskLength);
+
+            return 
Transaction.execute((TransactionCallbackWithException<RemoteAccessVpn, 
NetworkRuleConflictException>) status -> {
                 if (vpcId == null) {
                     _rulesMgr.reservePorts(ipAddr, NetUtils.UDP_PROTO, 
Purpose.Vpn, openFirewallFinal, caller, NetUtils.VPN_PORT, 
NetUtils.VPN_L2TP_PORT,
-                        NetUtils.VPN_NATT_PORT);
+                            NetUtils.VPN_NATT_PORT);
                 }
-                RemoteAccessVpnVO vpnVO =
-                    new RemoteAccessVpnVO(ipAddr.getAccountId(), 
ipAddr.getDomainId(), ipAddr.getAssociatedWithNetworkId(), publicIpId, vpcId, 
range[0], newIpRange,
-                        sharedSecret);
+                RemoteAccessVpnVO remoteAccessVpnVO = new 
RemoteAccessVpnVO(ipAddr.getAccountId(), ipAddr.getDomainId(), 
ipAddr.getAssociatedWithNetworkId(),
+                        publicIpId, vpcId, range[0], newIpRange, sharedSecret);
 
                 if (forDisplay != null) {
-                    vpnVO.setDisplay(forDisplay);
+                    remoteAccessVpnVO.setDisplay(forDisplay);
                 }
-                return _remoteAccessVpnDao.persist(vpnVO);
-            }
-        });
+                return _remoteAccessVpnDao.persist(remoteAccessVpnVO);
+            });
+        } finally {
+            _ipAddressDao.releaseFromLockTable(publicIpId);
+        }
     }
 
     private void validateRemoteAccessVpnConfiguration() throws 
ConfigurationException {

Reply via email to