CLOUDSTACK-8395: vmops plugin should work on both XS 6.5 and 6.2 ✊

This fixes the issue of Security Groups not working in case of XenServer 6.5;
- Uses nethash ipset data-structure to store CIDRs (efficient than iphash and
  avoids overflow errors in case users add /8 /4 ingress/egress cidrs)
- Support for ipset versions both on 6.2 and 6.5, both have different outputs. This
  fixes the issue of destroy_network_rules_for_vm failing
- Implements defensive filtering of list, instead of popping last item without
  checking if it's None or empty
- Greps using names that are 'quoted' to avoid bash errors
- Before setting up new network rule, tries to clean and remove old ipset entry
- Idents, whitespace and naming fixes

PS. This is my 1000th commit to the 🐵 project :)

This closes #186

(cherry picked from commit d91d161107e20ffc8ec088ee55f99d6743330946)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Conflicts:
	scripts/vm/hypervisor/xenserver/vmops

scripts/vm/hypervisor/xenserver/vmops

@@ -357,17 +357,17 @@ def allow_egress_traffic(session):
     return 'true'
 
 
-def ipset(ipsetname, proto, start, end, ips):
+def ipset(ipsetname, proto, start, end, cidrs):
     try:
-        util.pread2(['ipset', '-N', ipsetname, 'iphash'])
+        util.pread2(['ipset', '-N', ipsetname, 'nethash'])
     except:
-        logging.debug("ipset chain already exists" + ipsetname)
+        logging.debug("ipset chain already exists: " + ipsetname)
 
     result = True
     ipsettmp = ''.join(''.join(ipsetname.split('-')).split('_')) + str(int(time.time()) % 1000)
 
     try:
-        util.pread2(['ipset', '-N', ipsettmp, 'iphash']) 
+        util.pread2(['ipset', '-N', ipsettmp, 'nethash'])
     except:
         logging.debug("Failed to create temp ipset, reusing old name= " + ipsettmp)
         try:
@@ -375,12 +375,12 @@ def ipset(ipsetname, proto, start, end, ips):
         except:
             logging.debug("Failed to clear old temp ipset name=" + ipsettmp)
             return False
-        
     try:
-        for ip in ips:
+        for cidr in cidrs:
             try:
-                util.pread2(['ipset', '-A', ipsettmp, ip])
+                util.pread2(['ipset', '-A', ipsettmp, cidr])
             except CommandException, cex:
+                logging.debug("ipset cidr add failed due to: " + str(cex.reason))
                 if cex.reason.rfind('already in set') == -1:
                    raise
     except:
@@ -392,8 +392,16 @@ def ipset(ipsetname, proto, start, end, ips):
     try:
         util.pread2(['ipset', '-W', ipsettmp, ipsetname])
     except:
-        logging.debug("Failed to swap ipset " + ipsetname)
+        logging.debug("Failed to swap ipset, trying to delete and swap ipset: " + ipsetname)
-        result = False
+        # the old ipset entry could be of iphash type, try to delete and recreate
+        try:
+            util.pread2(['ipset', '-X', ipsetname])
+            util.pread2(['ipset', '-N', ipsetname, 'nethash'])
+            util.pread2(['ipset', '-W', ipsettmp, ipsetname])
+        except:
+            logging.debug("Failed to swap ipset " + ipsetname)
+            result = False
+        logging.debug("Succeeded in re-initializing and swapping ipset")
 
     try:
         util.pread2(['ipset', '-F', ipsettmp])
@@ -417,7 +425,7 @@ def destroy_network_rules_for_vm(session, args):
             util.pread2(['iptables', '-F', vmchain_default])
             util.pread2(['iptables', '-X', vmchain_default])
         except:
-            logging.debug("Ignoring failure to delete  chain " + vmchain_default)
+            logging.debug("Ignoring failure to delete chain " + vmchain_default)
 
     destroy_ebtables_rules(vmchain)
     destroy_arptables_rules(vmchain)
@@ -442,49 +450,42 @@ def destroy_network_rules_for_vm(session, args):
         return 'true'
 
     try:
-        setscmd = "ipset --save | grep " +  vmchain + " | grep '^-N' | awk '{print $2}'"
+        setscmd = "ipset --save | grep '%s' | grep -e '^-N' -e '^create' | awk '{print $2}'" % vmchain
-        setsforvm = util.pread2(['/bin/bash', '-c', setscmd]).split('\n')
+        ipset_names = filter(None, util.pread2(['/bin/bash', '-c', setscmd]).split('\n'))
-        for set in setsforvm:
+        for ipset_name in ipset_names:
-            if set != '':
+            if not ipset_name:
-                util.pread2(['ipset', '-F', set])       
+                continue
-                util.pread2(['ipset', '-X', set])       
+            util.pread2(['ipset', '-F', ipset_name])
+            util.pread2(['ipset', '-X', ipset_name])
     except:
         logging.debug("Failed to destroy ipsets for %" % vm_name)
 
-    
     return 'true'
 
 @echo
 def destroy_ebtables_rules(vm_chain):
-    
+    delcmd = "ebtables-save | grep '%s' | sed 's/-A/-D/'" % vm_chain
-    delcmd = "ebtables-save | grep " +  vm_chain + " | sed 's/-A/-D/'"
     delcmds = util.pread2(['/bin/bash', '-c', delcmd]).split('\n')
-    delcmds.pop()
+    for cmd in filter(None, delcmds):
-    for cmd in delcmds:
         try:
-            dc = cmd.split(' ')
+            dc = 'ebtables ' + cmd
-            dc.insert(0, 'ebtables')
+            util.pread2(filter(None, dc.split(' ')))
-            util.pread2(dc)
         except:
             logging.debug("Ignoring failure to delete ebtables rules for vm " + vm_chain)
     try:
         util.pread2(['ebtables', '-F', vm_chain])
         util.pread2(['ebtables', '-X', vm_chain])
     except:
-            logging.debug("Ignoring failure to delete ebtables chain for vm " + vm_chain)   
+        logging.debug("Ignoring failure to delete ebtables chain for vm " + vm_chain)
 
 @echo
 def destroy_arptables_rules(vm_chain):
-    delcmd = "arptables -vL FORWARD | grep " + vm_chain + " | sed 's/-i any//' | sed 's/-o any//' | awk '{print $1,$2,$3,$4}' "
+    delcmd = "arptables -vL FORWARD | grep '%s' | sed 's/-i any//' | sed 's/-o any//' | awk '{print $1,$2,$3,$4}' " % vm_chain
     delcmds = util.pread2(['/bin/bash', '-c', delcmd]).split('\n')
-    delcmds.pop()
+    for cmd in filter(None, delcmds):
-    for cmd in delcmds:
         try:
-            dc = cmd.split(' ')
+            dc = 'arptables -D FORWARD ' + cmd
-            dc.insert(0, 'arptables')
+            util.pread2(filter(None, dc.split(' ')))
-            dc.insert(1, '-D')
-            dc.insert(2, 'FORWARD')
-            util.pread2(dc)
         except:
             logging.debug("Ignoring failure to delete arptables rules for vm " + vm_chain)
 
@@ -662,7 +663,6 @@ def default_network_rules_systemvm(session, args):
             logging.debug("Failed to program default rules")
             return 'false'
 
-	
     util.pread2(['iptables', '-A', vmchain, '-j', 'ACCEPT'])
 
     if write_rule_log_for_vm(vm_name, '-1', '_ignore_', domid, '_initial_', '-1') == False:
@@ -760,7 +760,6 @@ def default_network_rules(session, args):
 
     destroy_ebtables_rules(vmchain)
 
-
     try:
         util.pread2(['iptables', '-N', vmchain])
     except:
@@ -870,19 +869,15 @@ def delete_rules_for_vm_in_bridge_firewall_chain(vmName):
     vm_name = vmName
     vmchain = chain_name_def(vm_name)
 
-    delcmd = "iptables-save | grep '\-A BRIDGE-FIREWALL' | grep " +  vmchain + " | sed 's/-A/-D/'"
+    delcmd = "iptables-save | grep '\-A BRIDGE-FIREWALL' | grep '%s' | sed 's/-A/-D/'" % vmchain
     delcmds = util.pread2(['/bin/bash', '-c', delcmd]).split('\n')
-    delcmds.pop()
+    for cmd in filter(None, delcmds):
-    for cmd in delcmds:
         try:
-            dc = cmd.split(' ')
+            dc = 'iptables ' + cmd
-            dc.insert(0, 'iptables')
+            util.pread2(filter(None, dc.split(' ')))
-            dc.pop()
-            util.pread2(filter(None, dc))
         except:
               logging.debug("Ignoring failure to delete rules for vm " + vmName)
 
-  
 @echo
 def network_rules_for_rebooted_vm(session, vmName):
     vm_name = vmName
@@ -1380,22 +1375,29 @@ def network_rules(session, args):
     logging.debug("Programming network rules for vm  %s seqno=%s numrules=%s signature=%s guestIp=%s,"\
               " update iptables, reason=%s" % (vm_name, seqno, len(lines), signature, vm_ip, reason))
 
+    # Flush iptables rules to clear ipset references and before re-applying iptable rules
+    for chain in [chain_name(vm_name), egress_chain_name(vm_name)]:
+        try:
+            util.pread2(['iptables', '-F', chain])
+        except:
+            logging.debug("Ignoring failure to delete chain " + chain)
+            util.pread2(['iptables', '-N', chain])
+
     cmds = []
     egressrules = 0
     for line in lines:
         tokens = line.split(':')
         if len(tokens) != 5:
           continue
-        type = tokens[0]
+        token_type = tokens[0]
         protocol = tokens[1]
         start = tokens[2]
         end = tokens[3]
-        cidrs = tokens.pop();
+        cidrs = tokens.pop().split(",")
-        ips = cidrs.split(",")
+        cidrs.pop()
-        ips.pop()
         allow_any = False
 
-        if type == 'E':
+        if token_type == 'E':
             vmchain = egress_chain_name(vm_name)
             action = "RETURN"
             direction = "dst"
@@ -1404,61 +1406,48 @@ def network_rules(session, args):
             vmchain = chain_name(vm_name)
             action = "ACCEPT"
             direction = "src"
-        if  '0.0.0.0/0' in ips:
+        if  '0.0.0.0/0' in cidrs:
-            i = ips.index('0.0.0.0/0')
+            i = cidrs.index('0.0.0.0/0')
-            del ips[i]
+            del cidrs[i]
             allow_any = True
-        range = start + ":" + end
+        port_range = start + ":" + end
-        if ips:    
+        if cidrs:
             ipsetname = vmchain + "_" + protocol + "_" + start + "_" + end
             if start == "-1":
                 ipsetname = vmchain + "_" + protocol + "_any"
 
-            if ipset(ipsetname, protocol, start, end, ips) == False:
+            if ipset(ipsetname, protocol, start, end, cidrs) == False:
                 logging.debug(" failed to create ipset for rule " + str(tokens))
 
             if protocol == 'all':
                 iptables = ['iptables', '-I', vmchain, '-m', 'state', '--state', 'NEW', '-m', 'set', keyword, ipsetname, direction, '-j', action]
             elif protocol != 'icmp':
-                iptables = ['iptables', '-I', vmchain, '-p',  protocol, '-m', protocol, '--dport', range, '-m', 'state', '--state', 'NEW', '-m', 'set', keyword, ipsetname, direction, '-j', action]
+                iptables = ['iptables', '-I', vmchain, '-p',  protocol, '-m', protocol, '--dport', port_range, '-m', 'state', '--state', 'NEW', '-m', 'set', keyword, ipsetname, direction, '-j', action]
             else:
-                range = start + "/" + end
+                port_range = start + "/" + end
                 if start == "-1":
-                    range = "any"
+                    port_range = "any"
-                iptables = ['iptables', '-I', vmchain, '-p',  'icmp', '--icmp-type',  range,  '-m', 'set', keyword, ipsetname, direction, '-j', action]
+                iptables = ['iptables', '-I', vmchain, '-p',  'icmp', '--icmp-type',  port_range,  '-m', 'set', keyword, ipsetname, direction, '-j', action]
-                
             cmds.append(iptables)
             logging.debug(iptables)
 
         if allow_any and protocol != 'all':
             if protocol != 'icmp':
-                iptables = ['iptables', '-I', vmchain, '-p',  protocol, '-m', protocol, '--dport', range, '-m', 'state', '--state', 'NEW', '-j', action]
+                iptables = ['iptables', '-I', vmchain, '-p',  protocol, '-m', protocol, '--dport', port_range, '-m', 'state', '--state', 'NEW', '-j', action]
             else:
-                range = start + "/" + end
+                port_range = start + "/" + end
                 if start == "-1":
-                    range = "any"
+                    port_range = "any"
-                iptables = ['iptables', '-I', vmchain, '-p',  'icmp', '--icmp-type',  range, '-j', action]
+                iptables = ['iptables', '-I', vmchain, '-p',  'icmp', '--icmp-type',  port_range, '-j', action]
             cmds.append(iptables)
             logging.debug(iptables)
 
-    vmchain = chain_name(vm_name)
-    try:
-        util.pread2(['iptables', '-F', vmchain])
-    except:
-        logging.debug("Ignoring failure to delete chain " + vmchain)
-        util.pread2(['iptables', '-N', vmchain])
-
-    egress_vmchain = egress_chain_name(vm_name)
-    try:
-        util.pread2(['iptables', '-F', egress_vmchain])
-    except:
-        logging.debug("Ignoring failure to delete chain " + egress_vmchain)
-        util.pread2(['iptables', '-N', egress_vmchain])
-
-    
     for cmd in cmds:
         util.pread2(cmd)
 
+    vmchain = chain_name(vm_name)
+    egress_vmchain = egress_chain_name(vm_name)
+
     if egressrules == 0 :
         util.pread2(['iptables', '-A', egress_vmchain, '-j', 'RETURN'])
     else: