You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2013/10/23 21:43:39 UTC

[22/47] New Transaction API

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());
     }