http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f62e28c1/server/src/com/cloud/network/rules/RulesManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 6e326b0..83f0493 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -26,7 +26,6 @@ import javax.ejb.Local; import javax.inject.Inject; import org.apache.log4j.Logger; - import org.apache.cloudstack.api.command.user.firewall.ListPortForwardingRulesCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -76,8 +75,11 @@ import com.cloud.utils.db.Filter; import com.cloud.utils.db.JoinBuilder; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; +import com.cloud.utils.db.TransactionCallbackNoReturn; +import com.cloud.utils.db.TransactionCallbackWithException; import com.cloud.utils.db.SearchCriteria.Op; import com.cloud.utils.db.Transaction; +import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.utils.net.NetUtils; @@ -197,12 +199,12 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules @Override @DB @ActionEvent(eventType = EventTypes.EVENT_NET_RULE_ADD, eventDescription = "creating forwarding rule", create = true) - public PortForwardingRule createPortForwardingRule(PortForwardingRule rule, Long vmId, Ip vmIp, boolean openFirewall) + public PortForwardingRule createPortForwardingRule(final PortForwardingRule rule, final Long vmId, Ip vmIp, final boolean openFirewall) throws NetworkRuleConflictException { CallContext ctx = CallContext.current(); - Account caller = ctx.getCallingAccount(); + final Account caller = ctx.getCallingAccount(); - Long ipAddrId = rule.getSourceIpAddressId(); + final Long ipAddrId = rule.getSourceIpAddressId(); IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); @@ -213,7 +215,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); } - Long networkId = rule.getNetworkId(); + final Long networkId = rule.getNetworkId(); Network network = _networkModel.getNetwork(networkId); //associate ip address to network (if needed) boolean performedIpAssoc = false; @@ -245,8 +247,8 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding, FirewallRuleType.User, networkId, rule.getTrafficType()); - Long accountId = ipAddress.getAllocatedToAccountId(); - 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()) { @@ -308,46 +310,48 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } } - Transaction txn = Transaction.currentTxn(); - txn.start(); - - PortForwardingRuleVO newRule = new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), - rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIp, rule.getDestinationPortStart(), - rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); - 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); - } - - try { - _firewallMgr.detectRulesConflict(newRule); - if (!_firewallDao.setStateToAdd(newRule)) { - throw new CloudRuntimeException("Unable to update the state to add for " + newRule); - } - CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, newRule.getAccountId(), - ipAddress.getDataCenterId(), newRule.getId(), null, PortForwardingRule.class.getName(), - newRule.getUuid()); - txn.commit(); - return newRule; - } catch (Exception e) { - if (newRule != null) { - txn.start(); - // no need to apply the rule as it wasn't programmed on the backend yet - _firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false); - removePFRule(newRule); - txn.commit(); - } - - if (e instanceof NetworkRuleConflictException) { - throw (NetworkRuleConflictException) e; + final Ip dstIpFinal = dstIp; + final IPAddressVO ipAddressFinal = ipAddress; + return Transaction.executeWithException(new TransactionCallbackWithException<PortForwardingRuleVO>() { + @Override + public PortForwardingRuleVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + PortForwardingRuleVO newRule = new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), + rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal, rule.getDestinationPortStart(), + rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); + 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); + } + + try { + _firewallMgr.detectRulesConflict(newRule); + if (!_firewallDao.setStateToAdd(newRule)) { + throw new CloudRuntimeException("Unable to update the state to add for " + newRule); + } + 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()); + return newRule; + } catch (Exception e) { + if (newRule != null) { + // no need to apply the rule as it wasn't programmed on the backend yet + _firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false); + removePFRule(newRule); + } + + if (e instanceof NetworkRuleConflictException) { + throw (NetworkRuleConflictException) e; + } + + throw new CloudRuntimeException("Unable to add rule for the ip id=" + ipAddrId, e); + } } + }, NetworkRuleConflictException.class); - throw new CloudRuntimeException("Unable to add rule for the ip id=" + ipAddrId, e); - } } finally { // release ip address if ipassoc was perfored if (performedIpAssoc) { @@ -361,10 +365,10 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules @Override @DB @ActionEvent(eventType = EventTypes.EVENT_NET_RULE_ADD, eventDescription = "creating static nat rule", create = true) - public StaticNatRule createStaticNatRule(StaticNatRule rule, boolean openFirewall) throws NetworkRuleConflictException { - Account caller = CallContext.current().getCallingAccount(); + public StaticNatRule createStaticNatRule(final StaticNatRule rule, final boolean openFirewall) throws NetworkRuleConflictException { + final Account caller = CallContext.current().getCallingAccount(); - Long ipAddrId = rule.getSourceIpAddressId(); + final Long ipAddrId = rule.getSourceIpAddressId(); IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); @@ -377,9 +381,9 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat, FirewallRuleType.User,null, rule.getTrafficType() ); - Long networkId = ipAddress.getAssociatedWithNetworkId(); - Long accountId = ipAddress.getAllocatedToAccountId(); - Long domainId = ipAddress.getAllocatedInDomainId(); + final Long networkId = ipAddress.getAssociatedWithNetworkId(); + final Long accountId = ipAddress.getAllocatedToAccountId(); + final Long domainId = ipAddress.getAllocatedInDomainId(); _networkModel.checkIpForService(ipAddress, Service.StaticNat, null); @@ -390,48 +394,48 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } //String dstIp = _networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); - String dstIp = ipAddress.getVmIp(); - Transaction txn = Transaction.currentTxn(); - txn.start(); + final String dstIp = ipAddress.getVmIp(); + return Transaction.executeWithException(new TransactionCallbackWithException<StaticNatRule>() { + @Override + public StaticNatRule doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { - 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); + 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); - newRule = _firewallDao.persist(newRule); + 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); - } + // 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); + } - try { - _firewallMgr.detectRulesConflict(newRule); - if (!_firewallDao.setStateToAdd(newRule)) { - throw new CloudRuntimeException("Unable to update the state to add for " + newRule); - } - 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()); - - txn.commit(); - StaticNatRule staticNatRule = new StaticNatRuleImpl(newRule, dstIp); - - return staticNatRule; - } catch (Exception e) { - - if (newRule != null) { - txn.start(); - // no need to apply the rule as it wasn't programmed on the backend yet - _firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false); - _firewallMgr.removeRule(newRule); - txn.commit(); - } + try { + _firewallMgr.detectRulesConflict(newRule); + if (!_firewallDao.setStateToAdd(newRule)) { + throw new CloudRuntimeException("Unable to update the state to add for " + newRule); + } + 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()); + + StaticNatRule staticNatRule = new StaticNatRuleImpl(newRule, dstIp); + + return staticNatRule; + } catch (Exception e) { + if (newRule != null) { + // no need to apply the rule as it wasn't programmed on the backend yet + _firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false); + _firewallMgr.removeRule(newRule); + } - if (e instanceof NetworkRuleConflictException) { - throw (NetworkRuleConflictException) e; + if (e instanceof NetworkRuleConflictException) { + throw (NetworkRuleConflictException) e; + } + throw new CloudRuntimeException("Unable to add static nat rule for the ip id=" + newRule.getSourceIpAddressId(), e); + } } - throw new CloudRuntimeException("Unable to add static nat rule for the ip id=" + newRule.getSourceIpAddressId(), e); - } + }, NetworkRuleConflictException.class); + } @Override @@ -1146,23 +1150,27 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules @Override @DB - public FirewallRuleVO[] reservePorts(IpAddress ip, String protocol, FirewallRule.Purpose purpose, - boolean openFirewall, Account caller, int... ports) throws NetworkRuleConflictException { - FirewallRuleVO[] rules = new FirewallRuleVO[ports.length]; - - Transaction txn = Transaction.currentTxn(); - txn.start(); - for (int i = 0; i < ports.length; i++) { - - rules[i] = new FirewallRuleVO(null, ip.getId(), ports[i], protocol, ip.getAssociatedWithNetworkId(), ip.getAllocatedToAccountId(), ip.getAllocatedInDomainId(), purpose, null, null, null, null); - rules[i] = _firewallDao.persist(rules[i]); - - if (openFirewall) { - _firewallMgr.createRuleForAllCidrs(ip.getId(), caller, ports[i], ports[i], protocol, null, null, - rules[i].getId(), ip.getAssociatedWithNetworkId()); + public FirewallRuleVO[] reservePorts(final IpAddress ip, final String protocol, final FirewallRule.Purpose purpose, + final boolean openFirewall, final Account caller, final int... ports) throws NetworkRuleConflictException { + final FirewallRuleVO[] rules = new FirewallRuleVO[ports.length]; + + Transaction.executeWithException(new TransactionCallbackWithException<Object>() { + @Override + public Object doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + for (int i = 0; i < ports.length; i++) { + + rules[i] = new FirewallRuleVO(null, ip.getId(), ports[i], protocol, ip.getAssociatedWithNetworkId(), ip.getAllocatedToAccountId(), ip.getAllocatedInDomainId(), purpose, null, null, null, null); + rules[i] = _firewallDao.persist(rules[i]); + + if (openFirewall) { + _firewallMgr.createRuleForAllCidrs(ip.getId(), caller, ports[i], ports[i], protocol, null, null, + rules[i].getId(), ip.getAssociatedWithNetworkId()); + } + } + + return null; } - } - txn.commit(); + }, NetworkRuleConflictException.class); boolean success = false; try { @@ -1173,12 +1181,14 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules return rules; } finally { if (!success) { - txn.start(); - - for (FirewallRuleVO newRule : rules) { - _firewallMgr.removeRule(newRule); - } - txn.commit(); + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + for (FirewallRuleVO newRule : rules) { + _firewallMgr.removeRule(newRule); + } + } + }); } } }
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f62e28c1/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index 8b2db9d..85b01b3 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.ConcurrentModificationException; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -34,6 +35,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import javax.ejb.ConcurrentAccessException; import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -96,6 +98,10 @@ import com.cloud.utils.db.DB; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GlobalLock; import com.cloud.utils.db.Transaction; +import com.cloud.utils.db.TransactionCallback; +import com.cloud.utils.db.TransactionCallbackNoReturn; +import com.cloud.utils.db.TransactionCallbackWithException; +import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.fsm.StateListener; import com.cloud.utils.net.NetUtils; @@ -191,12 +197,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro @Override protected void runInContext() { try { - Transaction txn = Transaction.open("SG Work"); - try { - work(); - } finally { - txn.close("SG Work"); - } + work(); } catch (Throwable th) { try { s_logger.error("Problem with SG work", th); @@ -204,24 +205,15 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } } } - - WorkerThread() { - - } } public class CleanupThread extends ManagedContextRunnable { @Override protected void runInContext() { try { - Transaction txn = Transaction.open("SG Cleanup"); - try { - cleanupFinishedWork(); - cleanupUnfinishedWork(); - //processScheduledWork(); - } finally { - txn.close("SG Cleanup"); - } + cleanupFinishedWork(); + cleanupUnfinishedWork(); + //processScheduledWork(); } catch (Throwable th) { try { s_logger.error("Problem with SG Cleanup", th); @@ -229,10 +221,6 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } } } - - CleanupThread() { - - } } public static class PortAndProto implements Comparable<PortAndProto> { @@ -400,7 +388,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } @DB - public void scheduleRulesetUpdateToHosts(List<Long> affectedVms, boolean updateSeqno, Long delayMs) { + public void scheduleRulesetUpdateToHosts(final List<Long> affectedVms, final boolean updateSeqno, Long delayMs) { if (affectedVms.size() == 0) { return; } @@ -422,39 +410,43 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro if (s_logger.isTraceEnabled()) { s_logger.trace("Security Group Mgr: acquired global work lock"); } - Transaction txn = Transaction.currentTxn(); + try { - txn.start(); - for (Long vmId : affectedVms) { - if (s_logger.isTraceEnabled()) { - s_logger.trace("Security Group Mgr: scheduling ruleset update for " + vmId); - } - VmRulesetLogVO log = null; - SecurityGroupWorkVO work = null; - - log = _rulesetLogDao.findByVmId(vmId); - if (log == null) { - log = new VmRulesetLogVO(vmId); - log = _rulesetLogDao.persist(log); - } - - if (log != null && updateSeqno) { - log.incrLogsequence(); - _rulesetLogDao.update(log.getId(), log); - } - work = _workDao.findByVmIdStep(vmId, Step.Scheduled); - if (work == null) { - work = new SecurityGroupWorkVO(vmId, null, null, SecurityGroupWork.Step.Scheduled, null); - work = _workDao.persist(work); - if (s_logger.isTraceEnabled()) { - s_logger.trace("Security Group Mgr: created new work item for " + vmId + "; id = " + work.getId()); + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + for (Long vmId : affectedVms) { + if (s_logger.isTraceEnabled()) { + s_logger.trace("Security Group Mgr: scheduling ruleset update for " + vmId); + } + VmRulesetLogVO log = null; + SecurityGroupWorkVO work = null; + + log = _rulesetLogDao.findByVmId(vmId); + if (log == null) { + log = new VmRulesetLogVO(vmId); + log = _rulesetLogDao.persist(log); + } + + if (log != null && updateSeqno) { + log.incrLogsequence(); + _rulesetLogDao.update(log.getId(), log); + } + work = _workDao.findByVmIdStep(vmId, Step.Scheduled); + if (work == null) { + work = new SecurityGroupWorkVO(vmId, null, null, SecurityGroupWork.Step.Scheduled, null); + work = _workDao.persist(work); + if (s_logger.isTraceEnabled()) { + s_logger.trace("Security Group Mgr: created new work item for " + vmId + "; id = " + work.getId()); + } + } + + work.setLogsequenceNumber(log.getLogsequence()); + _workDao.update(work.getId(), work); } } + }); - work.setLogsequenceNumber(log.getLogsequence()); - _workDao.update(work.getId(), work); - } - txn.commit(); for (Long vmId : affectedVms) { _executorPool.schedule(new WorkerThread(), delayMs, TimeUnit.MILLISECONDS); } @@ -595,7 +587,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro return authorizeSecurityGroupRule(securityGroupId,protocol,startPort,endPort,icmpType,icmpCode,cidrList,groupList,SecurityRuleType.IngressRule); } - private List<SecurityGroupRuleVO> authorizeSecurityGroupRule(Long securityGroupId,String protocol,Integer startPort,Integer endPort,Integer icmpType,Integer icmpCode,List<String> cidrList,Map groupList,SecurityRuleType ruleType) { + private List<SecurityGroupRuleVO> authorizeSecurityGroupRule(final Long securityGroupId, String protocol,Integer startPort,Integer endPort,Integer icmpType,Integer icmpCode,final List<String> cidrList,Map groupList, final SecurityRuleType ruleType) { Integer startPortOrType = null; Integer endPortOrCode = null; @@ -713,66 +705,71 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } } - final Transaction txn = Transaction.currentTxn(); final Set<SecurityGroupVO> authorizedGroups2 = new TreeSet<SecurityGroupVO>(new SecurityGroupVOComparator()); authorizedGroups2.addAll(authorizedGroups); // Ensure we don't re-lock the same row - txn.start(); - // Prevents other threads/management servers from creating duplicate security rules - securityGroup = _securityGroupDao.acquireInLockTable(securityGroupId); - if (securityGroup == null) { - s_logger.warn("Could not acquire lock on network security group: id= " + securityGroupId); - return null; - } - List<SecurityGroupRuleVO> newRules = new ArrayList<SecurityGroupRuleVO>(); - try { - for (final SecurityGroupVO ngVO : authorizedGroups2) { - final Long ngId = ngVO.getId(); - // Don't delete the referenced group from under us - if (ngVO.getId() != securityGroup.getId()) { - final SecurityGroupVO tmpGrp = _securityGroupDao.lockRow(ngId, false); - if (tmpGrp == null) { - s_logger.warn("Failed to acquire lock on security group: " + ngId); - txn.rollback(); - return null; - } + final Integer startPortOrTypeFinal = startPortOrType; + final Integer endPortOrCodeFinal = endPortOrCode; + final String protocolFinal = protocol; + return Transaction.execute(new TransactionCallback<List<SecurityGroupRuleVO>>() { + @Override + public List<SecurityGroupRuleVO> doInTransaction(TransactionStatus status) { + // Prevents other threads/management servers from creating duplicate security rules + SecurityGroup securityGroup = _securityGroupDao.acquireInLockTable(securityGroupId); + if (securityGroup == null) { + s_logger.warn("Could not acquire lock on network security group: id= " + securityGroupId); + return null; } - SecurityGroupRuleVO securityGroupRule = _securityGroupRuleDao.findByProtoPortsAndAllowedGroupId(securityGroup.getId(), protocol, startPortOrType, endPortOrCode, ngVO.getId()); - if ((securityGroupRule != null) && (securityGroupRule.getRuleType() == ruleType)) { - continue; // rule already exists. - } - securityGroupRule = new SecurityGroupRuleVO(ruleType, securityGroup.getId(), startPortOrType, endPortOrCode, protocol, ngVO.getId()); - securityGroupRule = _securityGroupRuleDao.persist(securityGroupRule); - newRules.add(securityGroupRule); - } - if (cidrList != null) { - for (String cidr : cidrList) { - SecurityGroupRuleVO securityGroupRule = _securityGroupRuleDao.findByProtoPortsAndCidr(securityGroup.getId(), protocol, startPortOrType, endPortOrCode, cidr); - if ((securityGroupRule != null) && (securityGroupRule.getRuleType() == ruleType)) { - continue; + List<SecurityGroupRuleVO> newRules = new ArrayList<SecurityGroupRuleVO>(); + try { + for (final SecurityGroupVO ngVO : authorizedGroups2) { + final Long ngId = ngVO.getId(); + // Don't delete the referenced group from under us + if (ngVO.getId() != securityGroup.getId()) { + final SecurityGroupVO tmpGrp = _securityGroupDao.lockRow(ngId, false); + if (tmpGrp == null) { + s_logger.warn("Failed to acquire lock on security group: " + ngId); + throw new ConcurrentAccessException("Failed to acquire lock on security group: " + ngId); + } + } + SecurityGroupRuleVO securityGroupRule = _securityGroupRuleDao.findByProtoPortsAndAllowedGroupId(securityGroup.getId(), protocolFinal, startPortOrTypeFinal, endPortOrCodeFinal, ngVO.getId()); + if ((securityGroupRule != null) && (securityGroupRule.getRuleType() == ruleType)) { + continue; // rule already exists. + } + securityGroupRule = new SecurityGroupRuleVO(ruleType, securityGroup.getId(), startPortOrTypeFinal, endPortOrCodeFinal, protocolFinal, ngVO.getId()); + securityGroupRule = _securityGroupRuleDao.persist(securityGroupRule); + newRules.add(securityGroupRule); + } + if (cidrList != null) { + for (String cidr : cidrList) { + SecurityGroupRuleVO securityGroupRule = _securityGroupRuleDao.findByProtoPortsAndCidr(securityGroup.getId(), protocolFinal, startPortOrTypeFinal, endPortOrCodeFinal, cidr); + if ((securityGroupRule != null) && (securityGroupRule.getRuleType() == ruleType)) { + continue; + } + securityGroupRule = new SecurityGroupRuleVO(ruleType, securityGroup.getId(), startPortOrTypeFinal, endPortOrCodeFinal, protocolFinal, cidr); + securityGroupRule = _securityGroupRuleDao.persist(securityGroupRule); + newRules.add(securityGroupRule); + } + } + if (s_logger.isDebugEnabled()) { + s_logger.debug("Added " + newRules.size() + " rules to security group " + securityGroup.getName()); + } + final ArrayList<Long> affectedVms = new ArrayList<Long>(); + affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroup.getId())); + scheduleRulesetUpdateToHosts(affectedVms, true, null); + return newRules; + } catch (Exception e) { + s_logger.warn("Exception caught when adding security group rules ", e); + throw new CloudRuntimeException("Exception caught when adding security group rules", e); + } finally { + if (securityGroup != null) { + _securityGroupDao.releaseFromLockTable(securityGroup.getId()); } - securityGroupRule = new SecurityGroupRuleVO(ruleType, securityGroup.getId(), startPortOrType, endPortOrCode, protocol, cidr); - securityGroupRule = _securityGroupRuleDao.persist(securityGroupRule); - newRules.add(securityGroupRule); } } - if (s_logger.isDebugEnabled()) { - s_logger.debug("Added " + newRules.size() + " rules to security group " + securityGroup.getName()); - } - txn.commit(); - final ArrayList<Long> affectedVms = new ArrayList<Long>(); - affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroup.getId())); - scheduleRulesetUpdateToHosts(affectedVms, true, null); - return newRules; - } catch (Exception e) { - s_logger.warn("Exception caught when adding security group rules ", e); - throw new CloudRuntimeException("Exception caught when adding security group rules", e); - } finally { - if (securityGroup != null) { - _securityGroupDao.releaseFromLockTable(securityGroup.getId()); - } - } + }); + } @Override @@ -792,11 +789,11 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro return revokeSecurityGroupRule(id, SecurityRuleType.IngressRule); } - private boolean revokeSecurityGroupRule(Long id, SecurityRuleType type) { + private boolean revokeSecurityGroupRule(final Long id, SecurityRuleType type) { // input validation Account caller = CallContext.current().getCallingAccount(); - SecurityGroupRuleVO rule = _securityGroupRuleDao.findById(id); + final SecurityGroupRuleVO rule = _securityGroupRuleDao.findById(id); if (rule == null) { s_logger.debug("Unable to find security rule with id " + id); throw new InvalidParameterValueException("Unable to find security rule with id " + id); @@ -812,36 +809,37 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro SecurityGroup securityGroup = _securityGroupDao.findById(rule.getSecurityGroupId()); _accountMgr.checkAccess(caller, null, true, securityGroup); - SecurityGroupVO groupHandle = null; - final Transaction txn = Transaction.currentTxn(); - - try { - txn.start(); - // acquire lock on parent group (preserving this logic) - groupHandle = _securityGroupDao.acquireInLockTable(rule.getSecurityGroupId()); - if (groupHandle == null) { - s_logger.warn("Could not acquire lock on security group id: " + rule.getSecurityGroupId()); - return false; - } - - _securityGroupRuleDao.remove(id); - s_logger.debug("revokeSecurityGroupRule succeeded for security rule id: " + id); - - final ArrayList<Long> affectedVms = new ArrayList<Long>(); - affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(groupHandle.getId())); - scheduleRulesetUpdateToHosts(affectedVms, true, null); + return Transaction.execute(new TransactionCallback<Boolean>() { + @Override + public Boolean doInTransaction(TransactionStatus status) { + SecurityGroupVO groupHandle = null; - return true; - } catch (Exception e) { - s_logger.warn("Exception caught when deleting security rules ", e); - throw new CloudRuntimeException("Exception caught when deleting security rules", e); - } finally { - if (groupHandle != null) { - _securityGroupDao.releaseFromLockTable(groupHandle.getId()); + try { + // acquire lock on parent group (preserving this logic) + groupHandle = _securityGroupDao.acquireInLockTable(rule.getSecurityGroupId()); + if (groupHandle == null) { + s_logger.warn("Could not acquire lock on security group id: " + rule.getSecurityGroupId()); + return false; + } + + _securityGroupRuleDao.remove(id); + s_logger.debug("revokeSecurityGroupRule succeeded for security rule id: " + id); + + final ArrayList<Long> affectedVms = new ArrayList<Long>(); + affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(groupHandle.getId())); + scheduleRulesetUpdateToHosts(affectedVms, true, null); + + return true; + } catch (Exception e) { + s_logger.warn("Exception caught when deleting security rules ", e); + throw new CloudRuntimeException("Exception caught when deleting security rules", e); + } finally { + if (groupHandle != null) { + _securityGroupDao.releaseFromLockTable(groupHandle.getId()); + } + } } - txn.commit(); - } - + }); } @Override @@ -939,7 +937,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } return; } - Long userVmId = work.getInstanceId(); + final Long userVmId = work.getInstanceId(); if (work.getStep() == Step.Done) { if (s_logger.isDebugEnabled()) { s_logger.debug("Security Group work: found a job in done state, rescheduling for vm: " + userVmId); @@ -949,68 +947,73 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro scheduleRulesetUpdateToHosts(affectedVms, false, _timeBetweenCleanups*1000l); return; } - UserVm vm = null; - Long seqnum = null; s_logger.debug("Working on " + work); - final Transaction txn = Transaction.currentTxn(); - txn.start(); - boolean locked = false; - try { - vm = _userVMDao.acquireInLockTable(work.getInstanceId()); - if (vm == null) { - vm = _userVMDao.findById(work.getInstanceId()); - if (vm == null) { - s_logger.info("VM " + work.getInstanceId() + " is removed"); + + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + UserVm vm = null; + Long seqnum = null; + + boolean locked = false; + try { + vm = _userVMDao.acquireInLockTable(work.getInstanceId()); + if (vm == null) { + vm = _userVMDao.findById(work.getInstanceId()); + if (vm == null) { + s_logger.info("VM " + work.getInstanceId() + " is removed"); + locked = true; + return; + } + s_logger.warn("Unable to acquire lock on vm id=" + userVmId); + return; + } locked = true; - return; - } - s_logger.warn("Unable to acquire lock on vm id=" + userVmId); - return; - } - locked = true; - Long agentId = null; - VmRulesetLogVO log = _rulesetLogDao.findByVmId(userVmId); - if (log == null) { - s_logger.warn("Cannot find log record for vm id=" + userVmId); - return; - } - seqnum = log.getLogsequence(); - - if (vm != null && vm.getState() == State.Running) { - Map<PortAndProto, Set<String>> ingressRules = generateRulesForVM(userVmId, SecurityRuleType.IngressRule); - Map<PortAndProto, Set<String>> egressRules = generateRulesForVM(userVmId, SecurityRuleType.EgressRule); - agentId = vm.getHostId(); - if (agentId != null) { - // get nic secondary ip address - String privateIp = vm.getPrivateIpAddress(); - NicVO nic = _nicDao.findByIp4AddressAndVmId(privateIp, vm.getId()); - List<String> nicSecIps = null; - if (nic != null) { - if (nic.getSecondaryIp()) { - //get secondary ips of the vm - long networkId = nic.getNetworkId(); - nicSecIps = _nicSecIpDao.getSecondaryIpAddressesForNic(nic.getId()); + Long agentId = null; + VmRulesetLogVO log = _rulesetLogDao.findByVmId(userVmId); + if (log == null) { + s_logger.warn("Cannot find log record for vm id=" + userVmId); + return; + } + seqnum = log.getLogsequence(); + + if (vm != null && vm.getState() == State.Running) { + Map<PortAndProto, Set<String>> ingressRules = generateRulesForVM(userVmId, SecurityRuleType.IngressRule); + Map<PortAndProto, Set<String>> egressRules = generateRulesForVM(userVmId, SecurityRuleType.EgressRule); + agentId = vm.getHostId(); + if (agentId != null) { + // get nic secondary ip address + String privateIp = vm.getPrivateIpAddress(); + NicVO nic = _nicDao.findByIp4AddressAndVmId(privateIp, vm.getId()); + List<String> nicSecIps = null; + if (nic != null) { + if (nic.getSecondaryIp()) { + //get secondary ips of the vm + long networkId = nic.getNetworkId(); + nicSecIps = _nicSecIpDao.getSecondaryIpAddressesForNic(nic.getId()); + } + } + SecurityGroupRulesCmd cmd = generateRulesetCmd( vm.getInstanceName(), vm.getPrivateIpAddress(), vm.getPrivateMacAddress(), vm.getId(), generateRulesetSignature(ingressRules, egressRules), seqnum, + ingressRules, egressRules, nicSecIps); + Commands cmds = new Commands(cmd); + try { + _agentMgr.send(agentId, cmds, _answerListener); + } catch (AgentUnavailableException e) { + s_logger.debug("Unable to send ingress rules updates for vm: " + userVmId + "(agentid=" + agentId + ")"); + _workDao.updateStep(work.getInstanceId(), seqnum, Step.Done); + } + } } - SecurityGroupRulesCmd cmd = generateRulesetCmd( vm.getInstanceName(), vm.getPrivateIpAddress(), vm.getPrivateMacAddress(), vm.getId(), generateRulesetSignature(ingressRules, egressRules), seqnum, - ingressRules, egressRules, nicSecIps); - Commands cmds = new Commands(cmd); - try { - _agentMgr.send(agentId, cmds, _answerListener); - } catch (AgentUnavailableException e) { - s_logger.debug("Unable to send ingress rules updates for vm: " + userVmId + "(agentid=" + agentId + ")"); - _workDao.updateStep(work.getInstanceId(), seqnum, Step.Done); + } finally { + if (locked) { + _userVMDao.releaseFromLockTable(userVmId); + _workDao.updateStep(work.getId(), Step.Done); } - } } - } finally { - if (locked) { - _userVMDao.releaseFromLockTable(userVmId); - _workDao.updateStep(work.getId(), Step.Done); - } - txn.commit(); - } + }); + } @Override @@ -1021,41 +1024,40 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro return false; } if (groups != null && !groups.isEmpty()) { - - final Transaction txn = Transaction.currentTxn(); - txn.start(); - UserVm userVm = _userVMDao.acquireInLockTable(userVmId); // ensures that duplicate entries are not created. - List<SecurityGroupVO> sgs = new ArrayList<SecurityGroupVO>(); - for (Long sgId : groups) { - sgs.add(_securityGroupDao.findById(sgId)); - } - final Set<SecurityGroupVO> uniqueGroups = new TreeSet<SecurityGroupVO>(new SecurityGroupVOComparator()); - uniqueGroups.addAll(sgs); - if (userVm == null) { - s_logger.warn("Failed to acquire lock on user vm id=" + userVmId); - } - try { - for (SecurityGroupVO securityGroup : uniqueGroups) { - // don't let the group be deleted from under us. - SecurityGroupVO ngrpLock = _securityGroupDao.lockRow(securityGroup.getId(), false); - if (ngrpLock == null) { - s_logger.warn("Failed to acquire lock on network group id=" + securityGroup.getId() + " name=" + securityGroup.getName()); - txn.rollback(); - return false; + return Transaction.execute(new TransactionCallback<Boolean>() { + @Override + public Boolean doInTransaction(TransactionStatus status) { + UserVm userVm = _userVMDao.acquireInLockTable(userVmId); // ensures that duplicate entries are not created. + List<SecurityGroupVO> sgs = new ArrayList<SecurityGroupVO>(); + for (Long sgId : groups) { + sgs.add(_securityGroupDao.findById(sgId)); } - if (_securityGroupVMMapDao.findByVmIdGroupId(userVmId, securityGroup.getId()) == null) { - SecurityGroupVMMapVO groupVmMapVO = new SecurityGroupVMMapVO(securityGroup.getId(), userVmId); - _securityGroupVMMapDao.persist(groupVmMapVO); + final Set<SecurityGroupVO> uniqueGroups = new TreeSet<SecurityGroupVO>(new SecurityGroupVOComparator()); + uniqueGroups.addAll(sgs); + if (userVm == null) { + s_logger.warn("Failed to acquire lock on user vm id=" + userVmId); + } + try { + for (SecurityGroupVO securityGroup : uniqueGroups) { + // don't let the group be deleted from under us. + SecurityGroupVO ngrpLock = _securityGroupDao.lockRow(securityGroup.getId(), false); + if (ngrpLock == null) { + s_logger.warn("Failed to acquire lock on network group id=" + securityGroup.getId() + " name=" + securityGroup.getName()); + throw new ConcurrentModificationException("Failed to acquire lock on network group id=" + securityGroup.getId() + " name=" + securityGroup.getName()); + } + if (_securityGroupVMMapDao.findByVmIdGroupId(userVmId, securityGroup.getId()) == null) { + SecurityGroupVMMapVO groupVmMapVO = new SecurityGroupVMMapVO(securityGroup.getId(), userVmId); + _securityGroupVMMapDao.persist(groupVmMapVO); + } + } + return true; + } finally { + if (userVm != null) { + _userVMDao.releaseFromLockTable(userVmId); + } } } - txn.commit(); - return true; - } finally { - if (userVm != null) { - _userVMDao.releaseFromLockTable(userVmId); - } - } - + }); } return false; @@ -1063,22 +1065,24 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro @Override @DB - public void removeInstanceFromGroups(long userVmId) { + public void removeInstanceFromGroups(final long userVmId) { if (_securityGroupVMMapDao.countSGForVm(userVmId) < 1) { s_logger.trace("No security groups found for vm id=" + userVmId + ", returning"); return; } - final Transaction txn = Transaction.currentTxn(); - txn.start(); - UserVm userVm = _userVMDao.acquireInLockTable(userVmId); // ensures that duplicate entries are not created in - // addInstance - if (userVm == null) { - s_logger.warn("Failed to acquire lock on user vm id=" + userVmId); - } - int n = _securityGroupVMMapDao.deleteVM(userVmId); - s_logger.info("Disassociated " + n + " network groups " + " from uservm " + userVmId); - _userVMDao.releaseFromLockTable(userVmId); - txn.commit(); + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + UserVm userVm = _userVMDao.acquireInLockTable(userVmId); // ensures that duplicate entries are not created in + // addInstance + if (userVm == null) { + s_logger.warn("Failed to acquire lock on user vm id=" + userVmId); + } + int n = _securityGroupVMMapDao.deleteVM(userVmId); + s_logger.info("Disassociated " + n + " network groups " + " from uservm " + userVmId); + _userVMDao.releaseFromLockTable(userVmId); + } + }); s_logger.debug("Security group mappings are removed successfully for vm id=" + userVmId); } @@ -1086,7 +1090,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro @Override @ActionEvent(eventType = EventTypes.EVENT_SECURITY_GROUP_DELETE, eventDescription = "deleting security group") public boolean deleteSecurityGroup(DeleteSecurityGroupCmd cmd) throws ResourceInUseException { - Long groupId = cmd.getId(); + final Long groupId = cmd.getId(); Account caller = CallContext.current().getCallingAccount(); SecurityGroupVO group = _securityGroupDao.findById(groupId); @@ -1097,32 +1101,34 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro // check permissions _accountMgr.checkAccess(caller, null, true, group); - final Transaction txn = Transaction.currentTxn(); - txn.start(); - - group = _securityGroupDao.lockRow(groupId, true); - if (group == null) { - throw new InvalidParameterValueException("Unable to find security group by id " + groupId); - } - - if (group.getName().equalsIgnoreCase(SecurityGroupManager.DEFAULT_GROUP_NAME)) { - throw new InvalidParameterValueException("The network group default is reserved"); - } - - List<SecurityGroupRuleVO> allowingRules = _securityGroupRuleDao.listByAllowedSecurityGroupId(groupId); - List<SecurityGroupVMMapVO> securityGroupVmMap = _securityGroupVMMapDao.listBySecurityGroup(groupId); - if (!allowingRules.isEmpty()) { - throw new ResourceInUseException("Cannot delete group when there are security rules that allow this group"); - } else if (!securityGroupVmMap.isEmpty()) { - throw new ResourceInUseException("Cannot delete group when it's in use by virtual machines"); - } - - _securityGroupDao.expunge(groupId); - txn.commit(); + return Transaction.executeWithException(new TransactionCallbackWithException<Boolean>() { + @Override + public Boolean doInTransaction(TransactionStatus status) throws ResourceInUseException { + SecurityGroupVO group = _securityGroupDao.lockRow(groupId, true); + if (group == null) { + throw new InvalidParameterValueException("Unable to find security group by id " + groupId); + } + + if (group.getName().equalsIgnoreCase(SecurityGroupManager.DEFAULT_GROUP_NAME)) { + throw new InvalidParameterValueException("The network group default is reserved"); + } + + List<SecurityGroupRuleVO> allowingRules = _securityGroupRuleDao.listByAllowedSecurityGroupId(groupId); + List<SecurityGroupVMMapVO> securityGroupVmMap = _securityGroupVMMapDao.listBySecurityGroup(groupId); + if (!allowingRules.isEmpty()) { + throw new ResourceInUseException("Cannot delete group when there are security rules that allow this group"); + } else if (!securityGroupVmMap.isEmpty()) { + throw new ResourceInUseException("Cannot delete group when it's in use by virtual machines"); + } + + _securityGroupDao.expunge(groupId); - s_logger.debug("Deleted security group id=" + groupId); + s_logger.debug("Deleted security group id=" + groupId); + + return true; + } + }, ResourceInUseException.class); - return true; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f62e28c1/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java index 9923db5..30d39e0 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java @@ -23,7 +23,6 @@ import javax.ejb.Local; import javax.inject.Inject; import org.apache.log4j.Logger; - import org.apache.cloudstack.context.CallContext; import com.cloud.configuration.ConfigurationManager; @@ -48,6 +47,8 @@ import com.cloud.utils.component.ManagerBase; import com.cloud.utils.db.DB; import com.cloud.utils.db.EntityManager; import com.cloud.utils.db.Transaction; +import com.cloud.utils.db.TransactionCallback; +import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; @@ -214,30 +215,35 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana @Override @DB @ActionEvent(eventType = EventTypes.EVENT_NETWORK_ACL_ITEM_CREATE, eventDescription = "creating network ACL Item", create = true) - public NetworkACLItem createNetworkACLItem(Integer portStart, Integer portEnd, String protocol, List<String> sourceCidrList, - Integer icmpCode, Integer icmpType, NetworkACLItem.TrafficType trafficType, Long aclId, - String action, Integer number) { - NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow; - if("deny".equalsIgnoreCase(action)){ - ruleAction = NetworkACLItem.Action.Deny; - } + public NetworkACLItem createNetworkACLItem(final Integer portStart, final Integer portEnd, final String protocol, final List<String> sourceCidrList, + final Integer icmpCode, final Integer icmpType, final NetworkACLItem.TrafficType trafficType, final Long aclId, + final String action, Integer number) { // If number is null, set it to currentMax + 1 (for backward compatibility) if(number == null){ number = _networkACLItemDao.getMaxNumberByACL(aclId) + 1; } - Transaction txn = Transaction.currentTxn(); - txn.start(); + final Integer numberFinal = number; + NetworkACLItemVO newRule = Transaction.execute(new TransactionCallback<NetworkACLItemVO>() { + @Override + public NetworkACLItemVO doInTransaction(TransactionStatus status) { + NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow; + if("deny".equalsIgnoreCase(action)){ + ruleAction = NetworkACLItem.Action.Deny; + } - NetworkACLItemVO newRule = new NetworkACLItemVO(portStart, portEnd, protocol.toLowerCase(), aclId, sourceCidrList, icmpCode, icmpType, trafficType, ruleAction, number); - newRule = _networkACLItemDao.persist(newRule); + NetworkACLItemVO newRule = new NetworkACLItemVO(portStart, portEnd, protocol.toLowerCase(), aclId, sourceCidrList, icmpCode, icmpType, trafficType, ruleAction, numberFinal); + newRule = _networkACLItemDao.persist(newRule); - if (!_networkACLItemDao.setStateToAdd(newRule)) { - throw new CloudRuntimeException("Unable to update the state to add for " + newRule); - } - CallContext.current().setEventDetails("ACL Item Id: " + newRule.getId()); + if (!_networkACLItemDao.setStateToAdd(newRule)) { + throw new CloudRuntimeException("Unable to update the state to add for " + newRule); + } + CallContext.current().setEventDetails("ACL Item Id: " + newRule.getId()); + + return newRule; + } + }); - txn.commit(); return getNetworkACLItem(newRule.getId()); }
