Introduce OptionString for handling command line 44/18844/21
authorVratko Polak <vrpolak@cisco.com>
Mon, 15 Apr 2019 14:31:54 +0000 (16:31 +0200)
committerVratko Polak <vrpolak@cisco.com>
Wed, 17 Apr 2019 07:58:39 +0000 (07:58 +0000)
+ Convert DpdkUtil to use it.
++ Rename args to kwargs where needed.
++ Fix errors in docstrings.
+ Also convert and QemuUtils to use it.
++ Minor formatting edits to save space.
+ Add disconnect parameter to some ssh.py functions.
++ ssh.SSH.disconnect() tries to work without argument.
+ Exec functions in ssh.py accept OptionString commands.

Change-Id: I82da71c568d120c283544c90242993fc76e9e83a
Signed-off-by: Vratko Polak <vrpolak@cisco.com>
resources/libraries/python/DpdkUtil.py
resources/libraries/python/OptionString.py [new file with mode: 0644]
resources/libraries/python/QemuUtils.py
resources/libraries/python/ssh.py

index 30efbde..887eae1 100644 (file)
 
 """Dpdk Utilities Library."""
 
-from resources.libraries.python.ssh import SSH, exec_cmd_no_error
+from resources.libraries.python.OptionString import OptionString
+from resources.libraries.python.ssh import exec_cmd_no_error
 
 
 class DpdkUtil(object):
     """Utilities for DPDK."""
 
     @staticmethod
-    def get_eal_options(**args):
-        """Create EAL parameters string.
+    def get_eal_options(**kwargs):
+        """Create EAL parameters options (including -v).
 
-        :param args: List of testpmd parameters.
-        :type args: dict
-        :returns: EAL parameters string.
-        :rtype: str
+        :param kwargs: Dict of testpmd parameters.
+        :type kwargs: dict
+        :returns: EAL parameters.
+        :rtype: OptionString
         """
-        eal_version = '-v'
+        options = OptionString(prefix='-')
+        options.add('v')
         # Set the hexadecimal bitmask of the cores to run on.
-        eal_corelist = '-l {}'.format(
-            args.get('eal_corelist', ''))\
-            if args.get('eal_corelist', '') else ''
+        options.add_with_value_from_dict('l', 'eal_corelist', kwargs)
         # Set master core.
-        eal_master_core = '--master-lcore 0'
+        options.add_with_value('-master-lcore', '0')
         # Load an external driver. Multiple -d options are allowed.
-        eal_driver = '-d /usr/lib/librte_pmd_virtio.so'\
-            if args.get('eal_driver', True) else ''
-        # Run in memory.
-        eal_in_memory = '--in-memory'\
-            if args.get('eal_in_memory', False) else ''
-
-        return ' '.join([eal_version,
-                         eal_corelist,
-                         eal_master_core,
-                         eal_driver,
-                         eal_in_memory])
+        options.add_with_value_if_from_dict(
+            'd', '/usr/lib/librte_pmd_virtio.so', 'eal_driver', kwargs, True)
+        options.add_if_from_dict(
+            '-in-memory', 'eal_in_memory', kwargs, False)
+        return options
 
     @staticmethod
-    def get_pmd_options(**args):
-        """Create PMD parameters string.
+    def get_pmd_options(**kwargs):
+        """Create PMD parameters options (without --).
 
-        :param args: List of testpmd parameters.
-        :type args: dict
-        :returns: PMD parameters string.
-        :rtype: str
+        :param kwargs: List of testpmd parameters.
+        :type kwargs: dict
+        :returns: PMD parameters.
+        :rtype: OptionString
         """
+        options = OptionString(prefix='--')
         # Set the forwarding mode: io, mac, mac_retry, mac_swap, flowgen,
         # rxonly, txonly, csum, icmpecho, ieee1588
-        pmd_fwd_mode = '--forward-mode={}'.format(
-            args.get('pmd_fwd_mode', 'io')) \
-            if args.get('pmd_fwd_mode', 'io') else ''
+        options.add_equals_from_dict(
+            'forward-mode', 'pmd_fwd_mode', kwargs, 'io')
         # Set the number of packets per burst to N.
-        pmd_burst = '--burst=64'
+        options.add_equals('burst', 64)
         # Set the number of descriptors in the TX rings to N.
-        pmd_txd = '--txd={}'.format(
-            args.get('pmd_txd', '1024')) \
-            if args.get('pmd_txd', '1024') else ''
+        options.add_equals_from_dict('txd', 'pmd_txd', kwargs, 1024)
         # Set the number of descriptors in the RX rings to N.
-        pmd_rxd = '--rxd={}'.format(
-            args.get('pmd_rxd', '1024')) \
-            if args.get('pmd_rxd', '1024') else ''
+        options.add_equals_from_dict('rxd', 'pmd_rxd', kwargs, 1024)
         # Set the number of queues in the TX to N.
-        pmd_txq = '--txq={}'.format(
-            args.get('pmd_txq', '1')) \
-            if args.get('pmd_txq', '1') else ''
+        options.add_equals_from_dict('txq', 'pmd_txq', kwargs, 1)
         # Set the number of queues in the RX to N.
-        pmd_rxq = '--rxq={}'.format(
-            args.get('pmd_rxq', '1')) \
-            if args.get('pmd_rxq', '1') else ''
-        # Set the hexadecimal bitmask of TX offloads.
-        pmd_tx_offloads = '--txqflags=0xf00'\
-            if args.get('pmd_tx_offloads', True) else ''
+        options.add_equals_from_dict('rxq', 'pmd_rxq', kwargs, 1)
+        # Set the hexadecimal bitmask of offloads.
+        options.add_equals_if_from_dict(
+            'txqflags', '0xf00', 'pmd_tx_offloads', kwargs, True)
         # Set the number of mbufs to be allocated in the mbuf pools.
-        pmd_total_num_mbufs = '--total-num-mbufs={}'.format(
-            args.get('pmd_num_mbufs', '')) \
-            if args.get('pmd_num_mbufs', '') else ''
-        # Set the max packet length.
-        pmd_max_pkt_len = '--max-pkt-len={}'.format(
-            args.get("pmd_max_pkt_len", "")) \
-            if args.get("pmd_max_pkt_len", "") else ""
+        options.add_equals_from_dict('total-num-mbufs', 'pmd_num_mbufs', kwargs)
         # Disable hardware VLAN.
-        pmd_disable_hw_vlan = '--disable-hw-vlan'\
-            if args.get('pmd_disable_hw_vlan', True) else ''
+        options.add_if_from_dict(
+            'disable-hw-vlan', 'pmd_disable_hw_vlan', kwargs, True)
         # Set the MAC address XX:XX:XX:XX:XX:XX of the peer port N
-        pmd_eth_peer_0 = '--eth-peer={}'.format(
-            args.get('pmd_eth_peer_0', ''))\
-            if args.get('pmd_eth_peer_0', '') else ''
-        pmd_eth_peer_1 = '--eth-peer={}'.format(
-            args.get('pmd_eth_peer_1', ''))\
-            if args.get('pmd_eth_peer_1', '') else ''
+        options.add_equals_from_dict('eth-peer', 'pmd_eth-peer_0', kwargs)
+        options.add_equals_from_dict('eth-peer', 'pmd_eth-peer_1', kwargs)
+        # Set the max packet length.
+        options.add_equals_from_dict('max-pkt-len', 'pmd_max_pkt_len', kwargs)
         # Set the number of forwarding cores based on coremask.
-        pmd_nb_cores = '--nb-cores={}'.format(
-            args.get('pmd_nb_cores', ''))\
-            if args.get('pmd_nb_cores', '') else ''
-
-        return ' '.join([pmd_fwd_mode,
-                         pmd_burst,
-                         pmd_txd,
-                         pmd_rxd,
-                         pmd_txq,
-                         pmd_rxq,
-                         pmd_tx_offloads,
-                         pmd_total_num_mbufs,
-                         pmd_disable_hw_vlan,
-                         pmd_eth_peer_0,
-                         pmd_eth_peer_1,
-                         pmd_max_pkt_len,
-                         pmd_nb_cores])
+        options.add_equals_from_dict('nb-cores', 'pmd_nb_cores', kwargs)
+        return options
 
     @staticmethod
     def get_testpmd_cmdline(**kwargs):
@@ -127,12 +90,14 @@ class DpdkUtil(object):
         :param args: Key-value testpmd parameters.
         :type args: dict
         :returns: Command line string.
-        :rtype: str
+        :rtype: OptionString
         """
-        eal_options = DpdkUtil.get_eal_options(**kwargs)
-        pmd_options = DpdkUtil.get_pmd_options(**kwargs)
-
-        return 'testpmd {0} -- {1}'.format(eal_options, pmd_options)
+        options = OptionString()
+        options.add('testpmd')
+        options.extend(DpdkUtil.get_eal_options(**kwargs))
+        options.add('--')
+        options.extend(DpdkUtil.get_pmd_options(**kwargs))
+        return options
 
     @staticmethod
     def dpdk_testpmd_start(node, **kwargs):
@@ -141,17 +106,14 @@ class DpdkUtil(object):
         :param node: VM Node to start testpmd on.
         :param args: Key-value testpmd parameters.
         :type node: dict
-        :type args: dict
-        :returns: nothing
+        :type kwargs: dict
         """
-        eal_options = DpdkUtil.get_eal_options(**kwargs)
-        pmd_options = DpdkUtil.get_pmd_options(**kwargs)
-
-        ssh = SSH()
-        ssh.connect(node)
-        cmd = "/start-testpmd.sh {0} -- {1}".format(eal_options, pmd_options)
-        exec_cmd_no_error(node, cmd, sudo=True)
-        ssh.disconnect(node)
+        cmd_options = OptionString()
+        cmd_options.add("/start-testpmd.sh")
+        cmd_options.extend(DpdkUtil.get_eal_options(**kwargs))
+        cmd_options.add('--')
+        cmd_options.extend(DpdkUtil.get_pmd_options(**kwargs))
+        exec_cmd_no_error(node, cmd_options, sudo=True, disconnect=True)
 
     @staticmethod
     def dpdk_testpmd_stop(node):
@@ -161,8 +123,5 @@ class DpdkUtil(object):
         :type node: dict
         :returns: nothing
         """
-        ssh = SSH()
-        ssh.connect(node)
-        cmd = "/stop-testpmd.sh"
-        exec_cmd_no_error(node, cmd, sudo=True)
-        ssh.disconnect(node)
+        cmd = "/stop-testpmd.sh"  # Completed string, simpler than OptionString.
+        exec_cmd_no_error(node, cmd, sudo=True, disconnect=True)
diff --git a/resources/libraries/python/OptionString.py b/resources/libraries/python/OptionString.py
new file mode 100644 (file)
index 0000000..d6cb40f
--- /dev/null
@@ -0,0 +1,362 @@
+# Copyright (c) 2019 Cisco and/or its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Utility function for handling options without doubled or trailing spaces."""
+
+
+class OptionString(object):
+    """Class serving as a builder for option strings.
+
+    Motivation: Both manual contatenation and .join() methods
+    are prone to leaving superfluous spaces if some parts of options
+    are optional (missing, empty).
+
+    The scope of this class is more general than just command line options,
+    it can concatenate any string consisting of words that may be missing.
+    But options were the first usage, so method arguments are frequently
+    named "parameter" and "value".
+    To keep this generality, automated adding of dashes is optional,
+    and disabled by default.
+
+    Parts of the whole option string are kept as list items (string, stipped),
+    with prefix already added.
+    Empty strings are never added to the list (except by constructor).
+
+    The class offers many methods for adding, so that callers can pick
+    the best fitting one, without much logic near the call site.
+    """
+
+    def __init__(self, prefix="", *args):
+        """Create instance with listed strings as parts to use.
+
+        Prefix will be converted to string and stripped.
+        The typical (nonempty) prefix values are "-" and "--".
+
+        :param prefix: Subtring to prepend to every parameter (not value).
+        :param args: List of positional arguments to become parts.
+        :type prefix: object
+        :type args: list of object
+        """
+        self.prefix = str(prefix).strip()  # Not worth to call change_prefix.
+        self.parts = list(args)
+
+    def __repr__(self):
+        """Return string executable as Python constructor call.
+
+        :returns: Executable constructor call as string.
+        :rtype: str
+        """
+        return "".join([
+            "OptionString(prefix=", repr(self.prefix), ",",
+            repr(self.parts)[1:-1], ")"])
+
+    # TODO: Would we ever need a copy() method?
+    # Currently, supersting "master" is mutable but unique,
+    # substring "slave" can be used to extend, but does not need to be mutated.
+
+    def change_prefix(self, prefix):
+        """Change the prefix field from the initialized value.
+
+        Sometimes it is more convenient to change the prefix in the middle
+        of string construction.
+        Typical use is for constructing a command, where the first part
+        (executeble filename) does not have a dash, but the other parameters do.
+        You could put the first part into constructor argument,
+        but using .add and only then enabling prefix is horizontally shorter.
+
+        :param prefix: New prefix value, to be converted and tripped.
+        :type prefix: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        self.prefix = str(prefix).strip()
+
+    def extend(self, other):
+        """Extend self by contents of other option string.
+
+        :param other: Another instance to add to the end of self.
+        :type other: OptionString
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        self.parts.extend(other.parts)
+        return self
+
+    def _check_and_add(self, part, prefixed):
+        """Convert to string, strip, add conditionally prefixed if non-empty.
+
+        Emptiness is tested before adding prefix.
+
+        :param part: Unchecked part to add to list of parts.
+        :param prefixed: Whether to add prefix when adding.
+        :type part: object
+        :type prefixed: object
+        :returns: The converted part without prefix, empty means not added.
+        :rtype: str
+        """
+        part = str(part).strip()
+        if part:
+            prefixed_part = self.prefix + part if prefixed else part
+            self.parts.append(prefixed_part)
+        return part
+
+    def add(self, parameter):
+        """Add parameter if nonempty to the list of parts.
+
+        Parameter object is converted to string and stripped.
+        If parameter converts to empty string, nothing is added.
+        Parameter is prefixed before adding.
+
+        :param parameter: Parameter object, usually a word starting with dash.
+        :type variable: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        self._check_and_add(parameter, prefixed=True)
+        return self
+
+    def add_if(self, parameter, condition):
+        """Add parameter if nonempty and condition is true to the list of parts.
+
+        If condition truth value is false, nothing is added.
+        Parameter object is converted to string and stripped.
+        If parameter converts to empty string, nothing is added.
+        Parameter is prefixed before adding.
+
+        :param parameter: Parameter object, usually a word starting with dash.
+        :param condition: Do not add if truth value of this is false.
+        :type variable: object
+        :type condition: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        if condition:
+            self.add(parameter)
+        return self
+
+    def add_with_value(self, parameter, value):
+        """Add parameter, if followed by a value to the list of parts.
+
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: Parameter object, usually a word starting with dash.
+        :param value: Value object. Prefix is never added.
+        :type variable: object
+        :type value: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        temp = OptionString(prefix=self.prefix)
+        # TODO: Is pylint really that ignorant?
+        # How could it not understand temp is of type of this class?
+        # pylint: disable=protected-access
+        if temp._check_and_add(parameter, prefixed=True):
+            if temp._check_and_add(value, prefixed=False):
+                self.extend(temp)
+        return self
+
+    def add_equals(self, parameter, value):
+        """Add parameter=value to the list of parts.
+
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: Parameter object, usually a word starting with dash.
+        :param value: Value object. Prefix is never added.
+        :type variable: object
+        :type value: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        temp = OptionString(prefix=self.prefix)
+        # pylint: disable=protected-access
+        if temp._check_and_add(parameter, prefixed=True):
+            if temp._check_and_add(value, prefixed=False):
+                self.parts.append("=".join(temp.parts))
+        return self
+
+    def add_with_value_if(self, parameter, value, condition):
+        """Add parameter and value if condition is true and nothing is empty.
+
+        If condition truth value is false, nothing is added.
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: Parameter object, usually a word starting with dash.
+        :param value: Value object. Prefix is never added.
+        :param condition: Do not add if truth value of this is false.
+        :type variable: object
+        :type value: object
+        :type condition: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        if condition:
+            self.add_with_value(parameter, value)
+        return self
+
+    def add_equals_if(self, parameter, value, condition):
+        """Add parameter=value to the list of parts if condition is true.
+
+        If condition truth value is false, nothing is added.
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: Parameter object, usually a word starting with dash.
+        :param value: Value object. Prefix is never added.
+        :param condition: Do not add if truth value of this is false.
+        :type variable: object
+        :type value: object
+        :type condition: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        if condition:
+            self.add_equals(parameter, value)
+        return self
+
+    def add_with_value_from_dict(self, parameter, key, mapping, default=""):
+        """Add parameter with value from dict under key, or default.
+
+        If key is missing, default is used as value.
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: The parameter part to add with prefix.
+        :param key: The key to look the value for.
+        :param mapping: Mapping with keys and values to use.
+        :param default: The value to use if key is missing.
+        :type parameter: object
+        :type key: str
+        :type mapping: dict
+        :type default: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        value = mapping.get(key, default)
+        return self.add_with_value(parameter, value)
+
+    def add_equals_from_dict(self, parameter, key, mapping, default=""):
+        """Add parameter=value to options where value is from dict.
+
+        If key is missing, default is used as value.
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: The parameter part to add with prefix.
+        :param key: The key to look the value for.
+        :param mapping: Mapping with keys and values to use.
+        :param default: The value to use if key is missing.
+        :type parameter: object
+        :type key: str
+        :type mapping: dict
+        :type default: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        value = mapping.get(key, default)
+        return self.add_equals(parameter, value)
+
+    def add_if_from_dict(self, parameter, key, mapping, default="False"):
+        """Add parameter based on if the condition in dict is true.
+
+        If key is missing, default is used as condition.
+        If condition truth value is false, nothing is added.
+        Parameter is converted to string and stripped.
+        If parameter converts to empty string, nothing is added.
+        Parameter is prefixed before adding.
+
+        :param parameter: The parameter part to add with prefix.
+        :param key: The key to look the value for.
+        :param mapping: Mapping with keys and values to use.
+        :param default: The value to use if key is missing.
+        :type parameter: object
+        :type key: str
+        :type mapping: dict
+        :type default: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        condition = mapping.get(key, default)
+        return self.add_if(parameter, condition)
+
+    def add_with_value_if_from_dict(
+            self, parameter, value, key, mapping, default="False"):
+        """Add parameter and value based on condition in dict.
+
+        If key is missing, default is used as condition.
+        If condition truth value is false, nothing is added.
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: The parameter part to add with prefix.
+        :param value: Value object. Prefix is never added.
+        :param key: The key to look the value for.
+        :param mapping: Mapping with keys and values to use.
+        :param default: The value to use if key is missing.
+        :type parameter: object
+        :type value: object
+        :type key: str
+        :type mapping: dict
+        :type default: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        condition = mapping.get(key, default)
+        return self.add_with_value_if(parameter, value, condition)
+
+    def add_equals_if_from_dict(
+            self, parameter, value, key, mapping, default="False"):
+        """Add parameter=value based on condition in dict.
+
+        If key is missing, default is used as condition.
+        If condition truth value is false, nothing is added.
+        Parameter and value are converted to string and stripped.
+        If parameter or value converts to empty string, nothing is added.
+        If added, parameter (but not value) is prefixed.
+
+        :param parameter: The parameter part to add with prefix.
+        :param value: Value object. Prefix is never added.
+        :param key: The key to look the value for.
+        :param mapping: Mapping with keys and values to use.
+        :param default: The value to use if key is missing.
+        :type parameter: object
+        :type value: object
+        :type key: str
+        :type mapping: dict
+        :type default: object
+        :returns: Self, to enable method chaining.
+        :rtype: OptionString
+        """
+        condition = mapping.get(key, default)
+        return self.add_equals_if(parameter, value, condition)
+
+    def __str__(self):
+        """Return space separated string of nonempty parts.
+
+        The format is suitable to be pasted as (part of) command line.
+        Do not call str() prematurely just to get a substring, consider
+        converting the surrounding text manipulation to OptionString as well.
+
+        :returns: Space separated string of options.
+        :rtype: str
+        """
+        return " ".join(self.parts)
index 55da28a..5404b51 100644 (file)
@@ -22,58 +22,16 @@ from re import match
 from distutils.version import StrictVersion
 
 from robot.api import logger
-from resources.libraries.python.ssh import exec_cmd, exec_cmd_no_error
 from resources.libraries.python.Constants import Constants
 from resources.libraries.python.DpdkUtil import DpdkUtil
 from resources.libraries.python.DUTSetup import DUTSetup
-from resources.libraries.python.topology import NodeType, Topology
+from resources.libraries.python.OptionString import OptionString
 from resources.libraries.python.VppConfigGenerator import VppConfigGenerator
 from resources.libraries.python.VPPUtil import VPPUtil
+from resources.libraries.python.ssh import exec_cmd, exec_cmd_no_error
+from resources.libraries.python.topology import NodeType, Topology
 
-__all__ = ["QemuOptions", "QemuUtils"]
-
-
-class QemuOptions(object):
-    """QEMU option class.
-
-    The class can handle input parameters that acts as QEMU command line
-    parameters. The only variable is a list of dictionaries where dictionaries
-    can be added multiple times. This emulates the QEMU behavior where one
-    command line parameter can be used multiple times (1..N). Example can be
-    device or object (so it is not an issue to have one memory
-    block of 2G and and second memory block of 512M but from other numa).
-
-    Class does support get value or string representation that will return
-    space separated, dash prefixed string of key value pairs used for command
-    line.
-    """
-
-    # Use one instance of class per tests.
-    ROBOT_LIBRARY_SCOPE = 'TEST CASE'
-
-    def __init__(self):
-        self.variables = list()
-
-    def add(self, variable, value):
-        """Add parameter to the list.
-
-        :param variable: QEMU parameter name (without dash).
-        :param value: Paired value.
-        :type variable: str
-        :type value: str or int
-        """
-        self.variables.append({str(variable): value})
-
-    def __str__(self):
-        """Return space separated string of key value pairs.
-
-        The format is suitable to be pasted to qemu command line.
-
-        :returns: Space separated string of key value pairs.
-        :rtype: str
-        """
-        return " ".join(["-{k} {v}".format(k=d.keys()[0], v=d.values()[0])
-                         for d in self.variables])
+__all__ = ["QemuUtils"]
 
 
 class QemuUtils(object):
@@ -135,7 +93,7 @@ class QemuUtils(object):
         else:
             raise RuntimeError('QEMU: Unknown VM image option!')
         # Computed parameters for QEMU command line.
-        self._params = QemuOptions()
+        self._params = OptionString(prefix='-')
         self.add_params()
 
     def add_params(self):
@@ -150,36 +108,38 @@ class QemuUtils(object):
 
     def add_default_params(self):
         """Set default QEMU command line parameters."""
-        self._params.add('daemonize', '')
-        self._params.add('nodefaults', '')
-        self._params.add('name', 'vnf{qemu},debug-threads=on'.
-                         format(qemu=self._opt.get('qemu_id')))
-        self._params.add('no-user-config', '')
-        self._params.add('monitor', 'none')
-        self._params.add('display', 'none')
-        self._params.add('vga', 'none')
-        self._params.add('enable-kvm', '')
-        self._params.add('pidfile', '{pidfile}'.
-                         format(pidfile=self._temp.get('pidfile')))
-        self._params.add('cpu', 'host')
-        self._params.add('machine', 'pc,accel=kvm,usb=off,mem-merge=off')
-        self._params.add('smp', '{smp},sockets=1,cores={smp},threads=1'.
-                         format(smp=self._opt.get('smp')))
-        self._params.add('object',
-                         'memory-backend-file,id=mem,size={mem}M,'
-                         'mem-path=/dev/hugepages,share=on'.
-                         format(mem=self._opt.get('mem')))
-        self._params.add('m', '{mem}M'.
-                         format(mem=self._opt.get('mem')))
-        self._params.add('numa', 'node,memdev=mem')
-        self._params.add('balloon', 'none')
+        self._params.add('daemonize')
+        self._params.add('nodefaults')
+        self._params.add_with_value('name', 'vnf{qemu},debug-threads=on'.format(
+            qemu=self._opt.get('qemu_id')))
+        self._params.add('no-user-config')
+        self._params.add_with_value('monitor', 'none')
+        self._params.add_with_value('display', 'none')
+        self._params.add_with_value('vga', 'none')
+        self._params.add('enable-kvm')
+        self._params.add_with_value('pidfile', self._temp.get('pidfile'))
+        self._params.add_with_value('cpu', 'host')
+        self._params.add_with_value(
+            'machine', 'pc,accel=kvm,usb=off,mem-merge=off')
+        self._params.add_with_value(
+            'smp', '{smp},sockets=1,cores={smp},threads=1'.format(
+                smp=self._opt.get('smp')))
+        self._params.add_with_value(
+            'object', 'memory-backend-file,id=mem,size={mem}M,'
+            'mem-path=/dev/hugepages,share=on'.format(mem=self._opt.get('mem')))
+        self._params.add_with_value(
+            'm', '{mem}M'.format(mem=self._opt.get('mem')))
+        self._params.add_with_value('numa', 'node,memdev=mem')
+        self._params.add_with_value('balloon', 'none')
 
     def add_nestedvm_params(self):
         """Set NestedVM QEMU parameters."""
-        self._params.add('net', 'nic,macaddr=52:54:00:00:{qemu:02x}:ff'.
-                         format(qemu=self._opt.get('qemu_id')))
-        self._params.add('net', 'user,hostfwd=tcp::{info[port]}-:22'.
-                         format(info=self._vm_info))
+        self._params.add_with_value(
+            'net', 'nic,macaddr=52:54:00:00:{qemu:02x}:ff'.format(
+                qemu=self._opt.get('qemu_id')))
+        self._params.add_with_value(
+            'net', 'user,hostfwd=tcp::{info[port]}-:22'.format(
+                info=self._vm_info))
         # TODO: Remove try except after fully migrated to Bionic or
         # qemu_set_node is removed.
         try:
@@ -187,33 +147,37 @@ class QemuUtils(object):
                 if self.qemu_version(version='2.10') else ''
         except AttributeError:
             locking = ''
-        self._params.add('drive',
-                         'file={img},format=raw,cache=none,if=virtio{locking}'.
-                         format(img=self._opt.get('img'), locking=locking))
-        self._params.add('qmp', 'unix:{qmp},server,nowait'.
-                         format(qmp=self._temp.get('qmp')))
-        self._params.add('chardev', 'socket,host=127.0.0.1,port={info[serial]},'
-                         'id=gnc0,server,nowait'.format(info=self._vm_info))
-        self._params.add('device', 'isa-serial,chardev=gnc0')
-        self._params.add('chardev',
-                         'socket,path={qga},server,nowait,id=qga0'.
-                         format(qga=self._temp.get('qga')))
-        self._params.add('device', 'isa-serial,chardev=qga0')
+        self._params.add_with_value(
+            'drive', 'file={img},format=raw,cache=none,if=virtio{locking}'.
+            format(img=self._opt.get('img'), locking=locking))
+        self._params.add_with_value(
+            'qmp', 'unix:{qmp},server,nowait'.format(qmp=self._temp.get('qmp')))
+        self._params.add_with_value(
+            'chardev', 'socket,host=127.0.0.1,port={info[serial]},'
+            'id=gnc0,server,nowait'.format(info=self._vm_info))
+        self._params.add_with_value('device', 'isa-serial,chardev=gnc0')
+        self._params.add_with_value(
+            'chardev', 'socket,path={qga},server,nowait,id=qga0'.format(
+                qga=self._temp.get('qga')))
+        self._params.add_with_value('device', 'isa-serial,chardev=qga0')
 
     def add_kernelvm_params(self):
         """Set KernelVM QEMU parameters."""
-        self._params.add('chardev', 'file,id=char0,path={log}'.
-                         format(log=self._temp.get('log')))
-        self._params.add('device', 'isa-serial,chardev=char0')
-        self._params.add('fsdev', 'local,id=root9p,path=/,security_model=none')
-        self._params.add('device',
-                         'virtio-9p-pci,fsdev=root9p,mount_tag=/dev/root')
-        self._params.add('kernel', '$(readlink -m {img}* | tail -1)'.
-                         format(img=self._opt.get('img')))
-        self._params.add('append',
-                         '"ro rootfstype=9p rootflags=trans=virtio '
-                         'console=ttyS0 tsc=reliable hugepages=256 '
-                         'init={init}"'.format(init=self._temp.get('ini')))
+        self._params.add_with_value(
+            'chardev', 'file,id=char0,path={log}'.format(
+                log=self._temp.get('log')))
+        self._params.add_with_value('device', 'isa-serial,chardev=char0')
+        self._params.add_with_value(
+            'fsdev', 'local,id=root9p,path=/,security_model=none')
+        self._params.add_with_value(
+            'device', 'virtio-9p-pci,fsdev=root9p,mount_tag=/dev/root')
+        self._params.add_with_value(
+            'kernel', '$(readlink -m {img}* | tail -1)'.format(
+                img=self._opt.get('img')))
+        self._params.add_with_value(
+            'append', '"ro rootfstype=9p rootflags=trans=virtio console=ttyS0'
+            ' tsc=reliable hugepages=256 init={init}"'.format(
+                init=self._temp.get('ini')))
 
     def create_kernelvm_config_vpp(self, **kwargs):
         """Create QEMU VPP config files.
@@ -258,9 +222,9 @@ class QemuUtils(object):
 
         with open(template, 'r') as src_file:
             src = Template(src_file.read())
-            exec_cmd_no_error(self._node, "echo '{out}' | sudo tee {running}".
-                              format(out=src.safe_substitute(**kwargs),
-                                     running=running))
+            exec_cmd_no_error(
+                self._node, "echo '{out}' | sudo tee {running}".format(
+                    out=src.safe_substitute(**kwargs), running=running))
 
     def create_kernelvm_config_testpmd_io(self, **kwargs):
         """Create QEMU testpmd-io command line.
@@ -323,16 +287,16 @@ class QemuUtils(object):
         """
         template = '{res}/init.sh'.format(res=Constants.RESOURCES_TPL_VM)
         init = self._temp.get('ini')
-        exec_cmd_no_error(self._node, 'rm -f {init}'.format(init=init),
-                          sudo=True)
+        exec_cmd_no_error(
+            self._node, 'rm -f {init}'.format(init=init), sudo=True)
 
         with open(template, 'r') as src_file:
             src = Template(src_file.read())
-            exec_cmd_no_error(self._node, "echo '{out}' | sudo tee {init}".
-                              format(out=src.safe_substitute(**kwargs),
-                                     init=init))
-            exec_cmd_no_error(self._node, "chmod +x {init}".
-                              format(init=init), sudo=True)
+            exec_cmd_no_error(
+                self._node, "echo '{out}' | sudo tee {init}".format(
+                    out=src.safe_substitute(**kwargs), init=init))
+            exec_cmd_no_error(
+                self._node, "chmod +x {init}".format(init=init), sudo=True)
 
     def configure_kernelvm_vnf(self, **kwargs):
         """Create KernelVM VNF configurations.
@@ -348,7 +312,7 @@ class QemuUtils(object):
             self.create_kernelvm_config_testpmd_mac(**kwargs)
         else:
             raise RuntimeError('QEMU: Unsupported VNF!')
-        self.create_kernelvm_init(vnf_bin=self._opt.get('vnf_bin'))
+        self.create_kernelvm_init(vnf_bin=self._opt['vnf_bin'])
 
     def get_qemu_pids(self):
         """Get QEMU CPU pids.
@@ -429,30 +393,26 @@ class QemuUtils(object):
         :type queues: int
         """
         self._vhost_id += 1
-        self._params.add('chardev',
-                         'socket,id=char{vhost},path={socket}{server}'.
-                         format(vhost=self._vhost_id, socket=socket,
-                                server=',server' if server is True else ''))
-        self._params.add('netdev',
-                         'vhost-user,id=vhost{vhost},'
-                         'chardev=char{vhost},queues={queues}'.
-                         format(vhost=self._vhost_id, queues=queues))
+        self._params.add_with_value(
+            'chardev', 'socket,id=char{vhost},path={socket}{server}'.format(
+                vhost=self._vhost_id, socket=socket,
+                server=',server' if server is True else ''))
+        self._params.add_with_value(
+            'netdev', 'vhost-user,id=vhost{vhost},chardev=char{vhost},'
+            'queues={queues}'.format(vhost=self._vhost_id, queues=queues))
         mac = ('52:54:00:00:{qemu:02x}:{vhost:02x}'.
                format(qemu=self._opt.get('qemu_id'), vhost=self._vhost_id))
         queue_size = ('rx_queue_size={queue_size},tx_queue_size={queue_size}'.
                       format(queue_size=queue_size)) if queue_size else ''
         mbuf = 'on,host_mtu=9200'
-        self._params.add('device',
-                         'virtio-net-pci,netdev=vhost{vhost},'
-                         'mac={mac},bus=pci.0,addr={addr}.0,mq=on,'
-                         'vectors={vectors},csum=off,gso=off,'
-                         'guest_tso4=off,guest_tso6=off,guest_ecn=off,'
-                         'mrg_rxbuf={mbuf},{queue_size}'.
-                         format(addr=self._vhost_id+5,
-                                vhost=self._vhost_id, mac=mac,
-                                mbuf=mbuf if jumbo_frames else 'off',
-                                queue_size=queue_size,
-                                vectors=(2 * queues + 2)))
+        self._params.add_with_value(
+            'device', 'virtio-net-pci,netdev=vhost{vhost},mac={mac},bus=pci.0,'
+            'addr={addr}.0,mq=on,vectors={vectors},csum=off,gso=off,'
+            'guest_tso4=off,guest_tso6=off,guest_ecn=off,mrg_rxbuf={mbuf},'
+            '{queue_size}'.format(
+                addr=self._vhost_id+5, vhost=self._vhost_id, mac=mac,
+                mbuf=mbuf if jumbo_frames else 'off', queue_size=queue_size,
+                vectors=(2 * queues + 2)))
 
         # Add interface MAC and socket to the node dict.
         if_data = {'mac_address': mac, 'socket': socket}
@@ -479,14 +439,14 @@ class QemuUtils(object):
                    format(cmd=cmd, qmp=self._temp.get('qmp')))
         message = ('QMP execute "{cmd}" failed on {host}'.
                    format(cmd=cmd, host=self._node['host']))
-        stdout, _ = exec_cmd_no_error(self._node, command, sudo=False,
-                                      message=message)
+        stdout, _ = exec_cmd_no_error(
+            self._node, command, sudo=False, message=message)
 
         # Skip capabilities negotiation messages.
         out_list = stdout.splitlines()
         if len(out_list) < 3:
-            raise RuntimeError('Invalid QMP output on {host}'.
-                               format(host=self._node['host']))
+            raise RuntimeError(
+                'Invalid QMP output on {host}'.format(host=self._node['host']))
         return json.loads(out_list[2])
 
     def _qemu_qga_flush(self):
@@ -495,8 +455,8 @@ class QemuUtils(object):
                    'sudo -S socat - UNIX-CONNECT:{qga}'.
                    format(qga=self._temp.get('qga')))
         message = ('QGA flush failed on {host}'.format(host=self._node['host']))
-        stdout, _ = exec_cmd_no_error(self._node, command, sudo=False,
-                                      message=message)
+        stdout, _ = exec_cmd_no_error(
+            self._node, command, sudo=False, message=message)
 
         return json.loads(stdout.split('\n', 1)[0]) if stdout else dict()
 
@@ -513,8 +473,8 @@ class QemuUtils(object):
                    format(cmd=cmd, qga=self._temp.get('qga')))
         message = ('QGA execute "{cmd}" failed on {host}'.
                    format(cmd=cmd, host=self._node['host']))
-        stdout, _ = exec_cmd_no_error(self._node, command, sudo=False,
-                                      message=message)
+        stdout, _ = exec_cmd_no_error(
+            self._node, command, sudo=False, message=message)
 
         return json.loads(stdout.split('\n', 1)[0]) if stdout else dict()
 
@@ -623,8 +583,8 @@ class QemuUtils(object):
             mac = interface.get('mac_address')
             if_name = mac_name.get(mac)
             if if_name is None:
-                logger.trace('Interface name for MAC {mac} not found'.
-                             format(mac=mac))
+                logger.trace(
+                    'Interface name for MAC {mac} not found'.format(mac=mac))
             else:
                 interface['name'] = if_name
 
@@ -634,18 +594,19 @@ class QemuUtils(object):
         :returns: VM node info.
         :rtype: dict
         """
-        command = ('{bin_path}/qemu-system-{arch} {params}'.
-                   format(bin_path=Constants.QEMU_BIN_PATH,
-                          arch=Topology.get_node_arch(self._node),
-                          params=self._params))
+        cmd_opts = OptionString()
+        cmd_opts.add('{bin_path}/qemu-system-{arch}'.format(
+            bin_path=Constants.QEMU_BIN_PATH,
+            arch=Topology.get_node_arch(self._node)))
+        cmd_opts.extend(self._params)
         message = ('QEMU: Start failed on {host}!'.
                    format(host=self._node['host']))
         try:
-            DUTSetup.check_huge_page(self._node, '/dev/hugepages',
-                                     self._opt.get('mem'))
+            DUTSetup.check_huge_page(
+                self._node, '/dev/hugepages', self._opt.get('mem'))
 
-            exec_cmd_no_error(self._node, command, timeout=300, sudo=True,
-                              message=message)
+            exec_cmd_no_error(
+                self._node, cmd_opts, timeout=300, sudo=True, message=message)
             self._wait_until_vm_boot()
         except RuntimeError:
             self.qemu_kill_all()
@@ -679,9 +640,9 @@ class QemuUtils(object):
         :returns: Qemu version or Boolean if version is higher than parameter.
         :rtype: str or bool
         """
-        command = ('{bin_path}/qemu-system-{arch} --version'.
-                   format(bin_path=Constants.QEMU_BIN_PATH,
-                          arch=Topology.get_node_arch(self._node)))
+        command = ('{bin_path}/qemu-system-{arch} --version'.format(
+            bin_path=Constants.QEMU_BIN_PATH,
+            arch=Topology.get_node_arch(self._node)))
         try:
             stdout, _ = exec_cmd_no_error(self._node, command, sudo=True)
             ver = match(r'QEMU emulator version ([\d.]*)', stdout).group(1)
index 60f6256..966d1b0 100644 (file)
@@ -111,12 +111,16 @@ class SSH(object):
                 raise IOError('Unable to connect to port {port} on {host}'.
                               format(port=node['port'], host=node['host']))
 
-    def disconnect(self, node):
+    def disconnect(self, node=None):
         """Close SSH connection to the node.
 
-        :param node: The node to disconnect from.
-        :type node: dict
+        :param node: The node to disconnect from. None means last connected.
+        :type node: dict or None
         """
+        if node is None:
+            node = self._node
+        if node is None:
+            return
         node_hash = self._node_hash(node)
         if node_hash in SSH.__existing_connections:
             logger.debug('Disconnecting peer: {host}, {port}'.
@@ -142,12 +146,13 @@ class SSH(object):
         :param cmd: Command to run on the Node.
         :param timeout: Maximal time in seconds to wait until the command is
         done. If set to None then wait forever.
-        :type cmd: str
+        :type cmd: str or OptionString
         :type timeout: int
         :return return_code, stdout, stderr
         :rtype: tuple(int, str, str)
         :raise SSHTimeout: If command is not finished in timeout time.
         """
+        cmd = str(cmd)
         stdout = StringIO.StringIO()
         stderr = StringIO.StringIO()
         try:
@@ -373,7 +378,7 @@ class SSH(object):
         logger.trace('SCP took {0} seconds'.format(end-start))
 
 
-def exec_cmd(node, cmd, timeout=600, sudo=False):
+def exec_cmd(node, cmd, timeout=600, sudo=False, disconnect=False):
     """Convenience function to ssh/exec/return rc, out & err.
 
     Returns (rc, stdout, stderr).
@@ -382,10 +387,12 @@ def exec_cmd(node, cmd, timeout=600, sudo=False):
     :param cmd: Command to execute.
     :param timeout: Timeout value in seconds. Default: 600.
     :param sudo: Sudo privilege execution flag. Default: False.
+    :param disconnect: Close the opened SSH connection if True.
     :type node: dict
-    :type cmd: str
+    :type cmd: str or OptionString
     :type timeout: int
     :type sudo: bool
+    :type disconnect: bool
     :returns: RC, Stdout, Stderr.
     :rtype: tuple(int, str, str)
     """
@@ -437,11 +444,15 @@ def exec_cmd(node, cmd, timeout=600, sudo=False):
     except SSHException as err:
         logger.error(repr(err))
         return None, None, None
+    finally:
+        if disconnect:
+            ssh.disconnect()
 
     return ret_code, stdout, stderr
 
 
-def exec_cmd_no_error(node, cmd, timeout=600, sudo=False, message=None):
+def exec_cmd_no_error(
+        node, cmd, timeout=600, sudo=False, message=None, disconnect=False):
     """Convenience function to ssh/exec/return out & err.
 
     Verifies that return code is zero.
@@ -451,16 +462,19 @@ def exec_cmd_no_error(node, cmd, timeout=600, sudo=False, message=None):
     :param timeout: Timeout value in seconds. Default: 600.
     :param sudo: Sudo privilege execution flag. Default: False.
     :param message: Error message in case of failure. Default: None.
+    :param disconnect: Close the opened SSH connection if True.
     :type node: dict
-    :type cmd: str
+    :type cmd: str or OptionString
     :type timeout: int
     :type sudo: bool
     :type message: str
+    :type disconnect: bool
     :returns: Stdout, Stderr.
     :rtype: tuple(str, str)
     :raises RuntimeError: If bash return code is not 0.
     """
-    ret_code, stdout, stderr = exec_cmd(node, cmd, timeout=timeout, sudo=sudo)
+    ret_code, stdout, stderr = exec_cmd(
+        node, cmd, timeout=timeout, sudo=sudo, disconnect=disconnect)
     msg = ('Command execution failed: "{cmd}"\n{stderr}'.
            format(cmd=cmd, stderr=stderr) if message is None else message)
     if ret_code != 0:
@@ -468,7 +482,8 @@ def exec_cmd_no_error(node, cmd, timeout=600, sudo=False, message=None):
 
     return stdout, stderr
 
-def scp_node(node, local_path, remote_path, get=False, timeout=30):
+def scp_node(
+        node, local_path, remote_path, get=False, timeout=30, disconnect=False):
     """Copy files from local_path to remote_path or vice versa.
 
     :param node: SUT node.
@@ -478,11 +493,13 @@ def scp_node(node, local_path, remote_path, get=False, timeout=30):
         path to remote file which should be downloaded.
     :param get: scp operation to perform. Default is put.
     :param timeout: Timeout value in seconds.
+    :param disconnect: Close the opened SSH connection if True.
     :type node: dict
     :type local_path: str
     :type remote_path: str
     :type get: bool
     :type timeout: int
+    :type disconnect: bool
     :raises RuntimeError: If SSH connection failed or SCP transfer failed.
     """
     ssh = SSH()
@@ -497,3 +514,6 @@ def scp_node(node, local_path, remote_path, get=False, timeout=30):
     except SCPException:
         raise RuntimeError('SCP execution failed on {host}!'
                            .format(host=node['host']))
+    finally:
+        if disconnect:
+            ssh.disconnect()