Clean up driver interface for sX300

Refactor the interface around setting up general ports in VLANs. Move
all the direct calls to the switch to internal-only functions, and
just expose the top-level wrapper PortSetGeneralVlan(). This makes the
interface easier to match up with that for other drviers.
diff --git a/drivers/cisco-sX300.py b/drivers/cisco-sX300.py
index 5aa1078..a983499 100644
--- a/drivers/cisco-sX300.py
+++ b/drivers/cisco-sX300.py
@@ -38,7 +38,7 @@
     allowed_port_modes = [ "trunk", "general" ]
 
     logfile = sys.stderr
-    #logfile = None
+    logfile = None
 
     def __init__(self, switch_hostname, switch_telnetport=23):
         self.exec_string = "/usr/bin/telnet %s %d" % (switch_hostname, switch_telnetport)
@@ -212,74 +212,41 @@
                 mode = match.group(1)
         return mode.lower()
 
-    # Allow the default VLAN on a port
-    def PortAllowDefaultVlan(self, port):
-        if not self._is_port_name_valid(port):
-            raise IndexError("Port name %s not recognised" % port)
-        self._configure()
-        self._cli("interface %s" % port)
-        self._cli("no switchport forbidden default-vlan")
-        self._end_configure()
-        # Difficult to validate
-
-    # Block the default VLAN on a port
-    def PortBlockDefaultVlan(self, port):
-        if not self._is_port_name_valid(port):
-            raise IndexError("Port name %s not recognised" % port)
-        self._configure()
-        self._cli("interface %s" % port)
-        self._cli("switchport forbidden default-vlan")
-        self._end_configure()        
-        # Difficult to validate
-
-    # Add a general port to a specified VLAN (tag)
-    def PortAddGeneraltoVlan(self, port, tag):
-        logging.debug("Adding general port %s to VLAN %d" % (port, tag))
+    # Set a general port to be in a specified VLAN (tag)
+    def PortSetGeneralVlan(self, port, tag):
+        logging.debug("Setting general port %s to VLAN %d" % (port, tag))
         if not self._is_port_name_valid(port):
             raise IndexError("Port name %s not recognised" % port)
         if not (self.PortGetMode(port) == "general"):
             raise IndexError("Port %s not in general mode" % port)
 
-        # Special handling for VLAN 1 (default)
-        if tag == 1:
-            return self.PortAllowDefaultVlan(port)
-        else:
-            self._configure()
-            self._cli("interface %s" % port)
-            self._cli("switchport general pvid %d" % tag)
-            self._cli("switchport general allowed vlan add %d untagged" % tag)
-            self._end_configure()        
+        # More complicated than just a "set" on the hardware, so let's
+        # split it up into separate helper calls.
 
-        # Validate it happened
-        read_vlan = self.PortGetGeneralVlan(port)
+        # First, get the current VLAN
+        current_vlan = self.PortGetGeneralVlan(port)
+
+        # Next, drop off that current VLAN
+        # VLAN 1 is handled specially :-(
+        if current_vlan == 1:
+            self._port_general_block_default_vlan(port)
+        else:
+            self._port_remove_general_from_vlan(port, current_vlan)
+
+        # Next, add the desired VLAN
+        # VLAN 1 is handled specially again :-(
+        if tag == 1:
+            self._port_general_allow_default_vlan(port)
+        else:
+            self._port_add_general_to_vlan(port, tag)
+
+        # Finally, validate things worked
+        read_vlan = int(self.PortGetGeneralVlan(port))
+        print "read_vlan is %s, tag is %d" % (read_vlan, tag)
         if read_vlan != tag:
-            raise IOError("Failed to add general port %d to VLAN %d - got VLAN %d"
+            raise IOError("Failed to move general port %d to VLAN %d - got VLAN %d instead"
                           % (port, tag, read_vlan))
 
-    # Remove a general port from a specified VLAN (tag)
-    def PortRemoveGeneralFromVlan(self, port, tag):
-        logging.debug("Removing general port %s from VLAN %d" % (port, tag))
-        if not self._is_port_name_valid(port):
-            raise IndexError("Port name %s not recognised" % port)
-        if not (self.PortGetMode(port) == "general"):
-            raise IndexError("Port %s not in general mode" % port)
-
-        # Special handling for VLAN 1 (default)
-        if tag == 1:
-            return self.PortBlockDefaultVlan(port)
-        else:
-            self._configure()
-            self._cli("interface %s" % port)
-            self._cli("no switchport general pvid")
-            self._cli("switchport general allowed vlan remove %d" % tag)
-            self._end_configure()        
-
-        # Validate it happened
-        read_vlan = self.PortGetGeneralVlan(port)
-        if read_vlan == tag:
-            raise IOError("Failed to remove general port %d from VLAN %d"
-                          % (port, tag))
-
     # Add a trunk port to a specified VLAN (tag)
     def PortAddTrunkToVlan(self, port, tag):
         logging.debug("Adding trunk port %s to VLAN %d" % (port, tag))
@@ -455,6 +422,51 @@
 
         return data
 
+    ######################################
+    # Internal port access helper methods
+    ######################################
+    # N.B. No parameter checking here, for speed reasons - if you're
+    # calling this internal API then you should already have validated
+    # things yourself! Equally, no post-set checks in here - do that
+    # at the higher level.
+    ######################################
+    
+    # Allow the default VLAN on a port
+    def _port_general_allow_default_vlan(self, port):
+        logging.debug("Allowing default VLAN (1) for general port %s" % port)
+        self._configure()
+        self._cli("interface %s" % port)
+        self._cli("no switchport forbidden default-vlan")
+        self._end_configure()
+        # Difficult to validate
+
+    # Block the default VLAN on a port
+    def _port_general_block_default_vlan(self, port):
+        logging.debug("Blocking default VLAN (1) for general port %s" % port)
+        self._configure()
+        self._cli("interface %s" % port)
+        self._cli("switchport forbidden default-vlan")
+        self._end_configure()        
+        # Difficult to validate
+
+    # Add a general port to a specified VLAN (tag)
+    def _port_add_general_to_vlan(self, port, tag):
+        logging.debug("Adding general port %s to VLAN %d" % (port, tag))
+        self._configure()
+        self._cli("interface %s" % port)
+        self._cli("switchport general pvid %d" % tag)
+        self._cli("switchport general allowed vlan add %d untagged" % tag)
+        self._end_configure()
+
+    # Remove a general port from a specified VLAN (tag)
+    def _port_remove_general_from_vlan(self, port, tag):
+        logging.debug("Removing general port %s from VLAN %d" % (port, tag))
+        self._configure()
+        self._cli("interface %s" % port)
+        self._cli("no switchport general pvid")
+        self._cli("switchport general allowed vlan remove %d" % tag)
+        self._end_configure()        
+
     # Wrapper around connection.send - by default, expect() the same
     # text we've sent, to remove it from the output from the
     # switch. For the few cases where we don't need that, override
@@ -509,16 +521,12 @@
     # Test general stuff
     print "Set fa6 to general mode"
     p.PortSetMode("fa6", "general")
-    #print "Remove fa6 from VLAN 1 (default)"
-    #p.PortRemoveGeneralFromVlan("fa6", 1)
-    #print "Move fa6 to VLAN 2"
-    #p.PortAddGeneraltoVlan("fa6", 2)
+    print "Move fa6 to VLAN 2"
+    p.PortSetGeneralVlan("fa6", 2)
     buf = p.PortGetGeneralVlan("fa6")
     print "Read from switch: fa6 is on VLAN %s" % buf
-    print "Remove fa6 from VLAN 2"
-    p.PortRemoveGeneralFromVlan("fa6", 2)
-    print "And re-enable the default VLAN for fa6"
-    p.PortAddGeneraltoVlan("fa6", 1)
+    print "Move fa6 back to default VLAN 1"
+    p.PortSetGeneralVlan("fa6", 1)
     #print "And move fa6 back to a trunk port"
     #p.PortSetMode("fa6", "trunk")
     #buf = p.PortGetMode("fa6")