From 2027049fd67a792ea0f3264573f77e6e802008eb Mon Sep 17 00:00:00 2001
From: Chiradeep Vittal <chiradeep@cloud.com>
Date: Wed, 31 Aug 2011 22:59:19 -0700
Subject: [PATCH] if the xenserver host cannot do bridge firewalling do not
 attempt to retry the security rule updat

---
 .../agent/api/SecurityIngressRuleAnswer.java  | 24 +++++++++++++++++++
 .../xen/resource/CitrixResourceBase.java      |  6 +++--
 .../security/SecurityGroupListener.java       | 14 ++++++++---
 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/api/src/com/cloud/agent/api/SecurityIngressRuleAnswer.java b/api/src/com/cloud/agent/api/SecurityIngressRuleAnswer.java
index b56048bb28d..caaf00006ce 100644
--- a/api/src/com/cloud/agent/api/SecurityIngressRuleAnswer.java
+++ b/api/src/com/cloud/agent/api/SecurityIngressRuleAnswer.java
@@ -18,8 +18,16 @@
 package com.cloud.agent.api;
 
 public class SecurityIngressRuleAnswer extends Answer {
+    public static enum FailureReason {
+        NONE,
+        UNKNOWN,
+        PROGRAMMING_FAILED,
+        CANNOT_BRIDGE_FIREWALL
+    }
     Long logSequenceNumber = null;
     Long vmId = null;
+    FailureReason reason = FailureReason.NONE;
+   
     
     protected SecurityIngressRuleAnswer() {
     }
@@ -34,6 +42,14 @@ public class SecurityIngressRuleAnswer extends Answer {
         super(cmd, result, detail);
         this.logSequenceNumber = cmd.getSeqNum();
         this.vmId = cmd.getVmId();
+        reason = FailureReason.PROGRAMMING_FAILED;
+    }
+    
+    public SecurityIngressRuleAnswer(SecurityIngressRulesCmd cmd, boolean result, String detail, FailureReason r) {
+        super(cmd, result, detail);
+        this.logSequenceNumber = cmd.getSeqNum();
+        this.vmId = cmd.getVmId();
+        reason = r;
     }
 
 	public Long getLogSequenceNumber() {
@@ -44,4 +60,12 @@ public class SecurityIngressRuleAnswer extends Answer {
 		return vmId;
 	}
 
+    public FailureReason getReason() {
+        return reason;
+    }
+
+    public void setReason(FailureReason reason) {
+        this.reason = reason;
+    }
+
 }
diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
index 87a3a264f1a..09a9f51dd7e 100644
--- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
+++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
@@ -4717,8 +4717,10 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
         }
 
         if (!_canBridgeFirewall) {
-            s_logger.info("Host " + _host.ip + " cannot do bridge firewalling");
-            return new SecurityIngressRuleAnswer(cmd, false, "Host " + _host.ip + " cannot do bridge firewalling");
+            s_logger.warn("Host " + _host.ip + " cannot do bridge firewalling");
+            return new SecurityIngressRuleAnswer(cmd, false, 
+                                                 "Host " + _host.ip + " cannot do bridge firewalling",
+                                                 SecurityIngressRuleAnswer.FailureReason.CANNOT_BRIDGE_FIREWALL);
         }
       
         String result = callHostPlugin(conn, "vmops", "network_rules",
diff --git a/server/src/com/cloud/network/security/SecurityGroupListener.java b/server/src/com/cloud/network/security/SecurityGroupListener.java
index 8118ec444eb..d9dfb928ecc 100755
--- a/server/src/com/cloud/network/security/SecurityGroupListener.java
+++ b/server/src/com/cloud/network/security/SecurityGroupListener.java
@@ -33,6 +33,7 @@ import com.cloud.agent.api.PingRoutingWithNwGroupsCommand;
 import com.cloud.agent.api.SecurityIngressRuleAnswer;
 import com.cloud.agent.api.StartupCommand;
 import com.cloud.agent.api.StartupRoutingCommand;
+import com.cloud.agent.api.SecurityIngressRuleAnswer.FailureReason;
 import com.cloud.agent.manager.Commands;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.host.HostVO;
@@ -85,9 +86,16 @@ public class SecurityGroupListener implements Listener {
                     _workDao.updateStep(ruleAnswer.getVmId(), ruleAnswer.getLogSequenceNumber(), Step.Done);
 
                 } else {
-                    _workDao.updateStep(ruleAnswer.getVmId(), ruleAnswer.getLogSequenceNumber(), Step.Error);
-                    s_logger.debug("Failed to program rule " + ruleAnswer.toString() + " into host " + agentId);
-                    affectedVms.add(ruleAnswer.getVmId());
+                    int deleted = _workDao.deleteWork(ruleAnswer.getVmId(), ruleAnswer.getLogSequenceNumber());
+                    s_logger.debug("Failed to program rule " + ruleAnswer.toString() + " into host " + agentId 
+                            +" due to " + ruleAnswer.getDetails()
+                            +" and deleted " + deleted + " jobs");
+                    if (ruleAnswer.getReason() == FailureReason.CANNOT_BRIDGE_FIREWALL) {
+                        s_logger.debug("Not retrying security group rules for vm " + ruleAnswer.getVmId() + " on failure since host " + agentId + " cannot do bridge firewalling");
+                    } else if (ruleAnswer.getReason() == FailureReason.PROGRAMMING_FAILED){
+                        s_logger.debug("Retrying on failure for vm " + ruleAnswer.getVmId());
+                        affectedVms.add(ruleAnswer.getVmId());
+                    }
                 }
                 commandNum++;
             }