CS-14956: Fixing an issue that surfaced while testing rate limiting

policies. An error was getting reported during policy map creation that
config operation was in progress, Added synchronization to make sure
sending and receiving commands are seralized. Also removed the retry logic
as after this change it is not needed.

Reviewed-By: Vijay

Author: Devdeep Singh

utils/src/com/cloud/utils/cisco/n1kv/vsm/NetconfHelper.java

@@ -20,9 +20,6 @@ public class NetconfHelper {
 
     private static final String SSH_NETCONF_TERMINATOR = "]]>]]>";
 
-    // Number of times to retry the command on failure.
-    private static final int s_retryCount = 3;
-
     private Connection _connection;
 
     private Session _session;
@@ -71,28 +68,7 @@ public void addPortProfile(String name, PortProfileType type, BindingType bindin
         String command = VsmCommand.getAddPortProfile(name, type, binding, mode, vlanid);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-
-            // This command occasionally fails. On retry it succeeds. Putting in
-            // retry to handle failures.
-            for (int i = 0; i < s_retryCount; ++i) {
-                send(command);
-                // parse the rpc reply.
-                // parseOkReply(receive());
-                VsmOkResponse response = new VsmOkResponse(receive().trim());
-                if (!response.isResponseOk()) {
-                    if (i >= s_retryCount) {
-                        throw new CloudRuntimeException(response.toString());
-                    }
-
-                    try {
-                        Thread.sleep(1000);
-                    } catch (final InterruptedException e) {
-                        s_logger.debug("Got interrupted while waiting.");
-                    }
-                } else {
-                    break;
-                }
-            }
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for adding port profile.");
         }
@@ -103,9 +79,7 @@ public void updatePortProfile(String name, SwitchPortMode mode,
         String command = VsmCommand.getUpdatePortProfile(name, mode, params);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            parseOkReply(receive());
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for updating port profile.");
         }
@@ -115,9 +89,7 @@ public void deletePortProfile(String name) throws CloudRuntimeException {
         String command = VsmCommand.getDeletePortProfile(name);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            parseOkReply(receive());
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for deleting port profile.");
         }
@@ -128,9 +100,7 @@ public void addPolicyMap(String name, int averageRate, int maxRate, int burstRat
         String command = VsmCommand.getAddPolicyMap(name, averageRate, maxRate, burstRate);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            parseOkReply(receive());
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for adding/updating policy map.");
         }
@@ -140,9 +110,7 @@ public void deletePolicyMap(String name) throws CloudRuntimeException {
         String command = VsmCommand.getDeletePolicyMap(name);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            parseOkReply(receive());
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for deleting policy map.");
         }
@@ -159,9 +127,7 @@ public void attachServicePolicy(String policyMap, String portProfile)
         String command = VsmCommand.getServicePolicy(policyMap, portProfile, true);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            parseOkReply(receive());
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for adding policy map.");
         }
@@ -172,9 +138,7 @@ public void detachServicePolicy(String policyMap, String portProfile)
         String command = VsmCommand.getServicePolicy(policyMap, portProfile, false);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            parseOkReply(receive());
+            parseOkReply(sendAndReceive(command));
         } else {
             throw new CloudRuntimeException("Error generating rpc request for removing policy map.");
         }
@@ -184,12 +148,10 @@ public PortProfile getPortProfileByName(String name) throws CloudRuntimeExceptio
         String command = VsmCommand.getPortProfile(name);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            String received = receive();
+            String received = sendAndReceive(command);
             VsmPortProfileResponse response = new VsmPortProfileResponse(received.trim());
             if (!response.isResponseOk()) {
-                throw new CloudRuntimeException("Error response while getting the port profile details.");
+                throw new CloudRuntimeException(response.toString());
             } else {
                 return response.getPortProfile();
             }
@@ -202,12 +164,10 @@ public PolicyMap getPolicyMapByName(String name) throws CloudRuntimeException {
         String command = VsmCommand.getPolicyMap(name);
         if (command != null) {
             command = command.concat(SSH_NETCONF_TERMINATOR);
-            send(command);
-            // parse the rpc reply.
-            String received = receive();
+            String received = sendAndReceive(command);
             VsmPolicyMapResponse response = new VsmPolicyMapResponse(received.trim());
             if (!response.isResponseOk()) {
-                throw new CloudRuntimeException("Error response while getting the port profile details.");
+                throw new CloudRuntimeException(response.toString());
             } else {
                 return response.getPolicyMap();
             }
@@ -222,6 +182,15 @@ private void exchangeHello() {
         send(hello);
     }
 
+    private String sendAndReceive(String command) {
+        String received;
+        synchronized (NetconfHelper.class) {
+            send(command);
+            received = receive();
+        }
+        return received;
+    }
+
     private void send(String message) {
         try {
             OutputStream outputStream = _session.getStdin();

utils/src/com/cloud/utils/cisco/n1kv/vsm/VsmCommand.java

@@ -527,7 +527,7 @@ private static Element policyMapDetails(Document doc, String name,
         Element policyMapMode = doc.createElement(s_policymapmode);
         policyDetails.appendChild(policyMapMode);
 
-        // Create the default class to match all trafic.
+        // Create the default class to match all traffic.
         Element classRoot = doc.createElement("class");
         Element classDefault = doc.createElement("class-default");
         policyMapMode.appendChild(classRoot);
@@ -544,11 +544,13 @@ private static Element policyMapDetails(Document doc, String name,
         // Set the committed information rate and its value in mbps.
         Element cir = doc.createElement("cir");
         police.appendChild(cir);
-        Element cirValue = doc.createElement(s_paramvalue);
-        Element mbps = doc.createElement("mbps");
-        cirValue.setTextContent(Integer.toString(averageRate));
+        Element cirValue = doc.createElement("cir-val");
         cir.appendChild(cirValue);
-        cir.appendChild(mbps);
+        Element value2 = doc.createElement(s_paramvalue);
+        Element mbps = doc.createElement("mbps");
+        value2.setTextContent(Integer.toString(averageRate));
+        cirValue.appendChild(value2);
+        cirValue.appendChild(mbps);
 
         // Persist the configuration across reboots.
         modeConfigure.appendChild(persistConfiguration(doc));