From b38dbc64dbbf868db1887cfae1ddd3d9dbcff950 Mon Sep 17 00:00:00 2001 From: Edison Su Date: Thu, 27 Mar 2014 15:39:23 -0700 Subject: [PATCH] CLOUDSTACK-6245: the security group rule is lagging behind the rules in DB, due to there is a worker thread launched inside a transaction Reviewed-by: Alex (cherry picked from commit d4fdc184fe9c6717d2ed4e4fe4c39d9759a90608) Signed-off-by: Animesh Chaturvedi Conflicts: server/src/com/cloud/network/security/SecurityGroupManagerImpl.java --- .../security/SecurityGroupManagerImpl.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index 9c1b967c6ef..f60a746e68c 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -716,7 +716,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro final Integer startPortOrTypeFinal = startPortOrType; final Integer endPortOrCodeFinal = endPortOrCode; final String protocolFinal = protocol; - return Transaction.execute(new TransactionCallback>() { + List newRules = Transaction.execute(new TransactionCallback>() { @Override public List doInTransaction(TransactionStatus status) { // Prevents other threads/management servers from creating duplicate security rules @@ -761,9 +761,6 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro if (s_logger.isDebugEnabled()) { s_logger.debug("Added " + newRules.size() + " rules to security group " + securityGroup.getName()); } - final ArrayList affectedVms = new ArrayList(); - 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); @@ -776,6 +773,15 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } }); + try { + final ArrayList affectedVms = new ArrayList(); + affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroup.getId())); + scheduleRulesetUpdateToHosts(affectedVms, true, null); + } catch (Exception e) { + s_logger.debug("can't update rules on host, ignore", e); + } + + return newRules; } @Override @@ -815,7 +821,8 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro SecurityGroup securityGroup = _securityGroupDao.findById(rule.getSecurityGroupId()); _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, securityGroup); - return Transaction.execute(new TransactionCallback() { + long securityGroupId = rule.getSecurityGroupId(); + Boolean result = Transaction.execute(new TransactionCallback() { @Override public Boolean doInTransaction(TransactionStatus status) { SecurityGroupVO groupHandle = null; @@ -831,10 +838,6 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro _securityGroupRuleDao.remove(id); s_logger.debug("revokeSecurityGroupRule succeeded for security rule id: " + id); - final ArrayList affectedVms = new ArrayList(); - 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); @@ -846,6 +849,16 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro } } }); + + try { + final ArrayList affectedVms = new ArrayList(); + affectedVms.addAll(_securityGroupVMMapDao.listVmIdsBySecurityGroup(securityGroupId)); + scheduleRulesetUpdateToHosts(affectedVms, true, null); + } catch (Exception e) { + s_logger.debug("Can't update rules for host, ignore", e); + } + + return result; } @Override