On Wed, Aug 19, 2015 at 09:50:13AM +0200, Jiri Prochazka wrote:
Comments inline
2015-08-17 10:31 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com>:
> I found 2 more problems, see inline.
>
> On Tue, Aug 11, 2015 at 12:07:43PM +0200, Jan Tluka wrote:
> > Just one issue found, see inline, but otherwise looks fine.
> > Much more readable and predictable ;-) Nice work!
> >
> > Mon, Aug 10, 2015 at 05:39:32PM CEST, jprochaz(a)redhat.com wrote:
> > >Removed MessageDispatcherLite.py
> > >
> > >lnst-pool-wizard
> > >+ Make pep8 compliant
> > >+ Print error messages on stderr
> > >+ Changes correspoding with Wizard.py refactorization
> > >+ Remove blank line in help
> > >
> > >Wizard.py
> > >+ Make pep8 compliant
> > >+ Most of the methods reworked to be more clean and readable
> > >+ Setup for implementation of support for virtual machines
> > >
> > >Closes #129
> > >
> > >Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com>
> > >---
> > > lnst-pool-wizard | 55 ++--
> > > lnst/Controller/MessageDispatcherLite.py | 66 -----
> > > lnst/Controller/Wizard.py | 422
> +++++++++++++++++++++----------
> > > 3 files changed, 332 insertions(+), 211 deletions(-)
> > > delete mode 100644 lnst/Controller/MessageDispatcherLite.py
> > >
> > >diff --git a/lnst-pool-wizard b/lnst-pool-wizard
> > >index c788688..d7ad506 100755
> > >--- a/lnst-pool-wizard
> > >+++ b/lnst-pool-wizard
> > >@@ -17,48 +17,67 @@ import getopt
> > > RETVAL_PASS = 0
> > > RETVAL_ERR = 1
> > >
> > >-def help(retval = 0):
> > >+
> > >+def help(retval=0):
> > > print "Usage:\n"\
> > > " lnst-pool-wizard [mode] [hostname[:port]]\n"\
> > > "\n"\
> > > "Modes:\n"\
> > >- " -h, --help display this help text and
> exit\n"\
> > >- " -i, --interactive start wizard in interactive
mode
> (this "\
> > >- "is default mode)\n"\
> > >- " -n, --noninteractive start wizard in noninteractive
> mode\n"\
> > >+ " -h, --help display this help text and
> exit\n"\
> > >+ " -p, --pool_dir <directory> set the pool dir (works
both
> in) "\
> > >+ "interactive and noninteractive mode\n"\
> > >+ " -i, --interactive start wizard in interactive
> mode (this "\
> > >+ "is default mode)\n"\
> > >+ " -n, --noninteractive start wizard in
noninteractive
> mode\n"\
> > > "Examples:\n"\
> > > " lnst-pool-wizard --interactive\n"\
> > > " lnst-pool-wizard --noninteractive 192.168.122.2\n"\
> > >- " lnst-pool-wizard -n 192.168.122.2:8888
192.168.122.3:9999
> 192.168.122.4\n"
> > >+ " lnst-pool-wizard -n 192.168.122.2:8888
192.168.122.4\n"\
> > >+ " lnst-pool-wizard -p \".pool/\" -n
192.168.1.1:8877
> 192.168.122.4"
> > > sys.exit(retval)
> > >
> > >
> > > def main():
> > > try:
> > > opts, args = getopt.getopt(
> > >- sys.argv[1:],
> > >- "hin",
> > >- ["help", "interactive",
"noninteractive"]
> > >+ sys.argv[1:],
> > >+ "hinp:",
> > >+ ["help", "interactive",
"noninteractive", "pool_dir="]
> > > )
> > > except getopt.GetoptError as err:
> > >- print str(err)
> > >+ sys.stderr.write(str(err))
> > > help(RETVAL_ERR)
> > >- wizard = Wizard()
> > >- for opt, arg in opts:
> > >+
> > >+ pool_dir= None
> > >+ mode = "interactive"
> > >+
> > >+ for opt, arg in opts:
> > > if opt in ("-h", "--help"):
> > > help()
> > > elif opt in ("-i", "--interactive"):
> > >- wizard.interactive()
> > >- sys.exit(RETVAL_PASS)
> > >+ mode = "interactive"
> > > elif opt in ("-n", "--noninteractive"):
> > > if not args:
> > >- print "No hostname entered!"
> > >+ sys.stderr.write("No hostnames entered\n")
> > > return RETVAL_ERR
> > >- wizard.noninteractive(args)
> > >- sys.exit(RETVAL_PASS)
> > >+ else:
> > >+ hostlist = args
> > >+ mode = "noninteractive"
> > >+ elif opt in ("-p", "--pool_dir"):
> > >+ if not arg:
> > >+ sys.stderr.write("No pool directory
specified\n")
> > >+ else:
> > >+ pool_dir = arg
> > > else:
> > > help(RETVAL_ERR)
> > >- wizard.interactive()
> > >+
> > >+ w = Wizard()
> > >+
> > >+ if mode == "noninteractive":
> > >+ w.noninteractive(hostlist, pool_dir)
> > >+ else:
> > >+ w.interactive(pool_dir)
> > >+
> > > sys.exit(RETVAL_PASS)
> > >
> > >
> > >diff --git a/lnst/Controller/MessageDispatcherLite.py
> b/lnst/Controller/MessageDispatcherLite.py
> > >deleted file mode 100644
> > >index 2973763..0000000
> > >--- a/lnst/Controller/MessageDispatcherLite.py
> > >+++ /dev/null
> > >@@ -1,66 +0,0 @@
> > >-"""
> > >-This module defines MessageDispatcherLite class which is derived from
> > >-lnst.NetTestController.MessageDispatcher.
> > >-
> > >-Copyright 2011 Red Hat, Inc.
> > >-Licensed under the GNU General Public License, version 2 as
> > >-published by the Free Software Foundation; see COPYING for details.
> > >-"""
> > >-
> > >-__author__ = """
> > >-jpirko(a)redhat.com (Jiri Pirko)
> > >-"""
> > >-
> > >-from lnst.Common.ConnectionHandler import send_data
> > >-from lnst.Common.ConnectionHandler import ConnectionHandler
> > >-from lnst.Controller.NetTestController import NetTestError
> > >-
> > >-class MessageDispatcherLite(ConnectionHandler):
> > >- def __init__(self):
> > >- super(MessageDispatcherLite, self).__init__()
> > >-
> > >- def add_slave(self, machine, connection):
> > >- self.add_connection(1, connection)
> > >-
> > >- def send_message(self, machine_id, data):
> > >- soc = self.get_connection(1)
> > >-
> > >- if send_data(soc, data) == False:
> > >- msg = "Connection error from slave %s" % str(1)
> > >- raise NetTestError(msg)
> > >-
> > >- def wait_for_result(self, machine_id):
> > >- wait = True
> > >- while wait:
> > >- connected_slaves = self._connection_mapping.keys()
> > >-
> > >- messages = self.check_connections()
> > >-
> > >- remaining_slaves = self._connection_mapping.keys()
> > >-
> > >- for msg in messages:
> > >- if msg[1]["type"] == "result" and
msg[0] == 1:
> > >- wait = False
> > >- result = msg[1]["result"]
> > >- else:
> > >- self._process_message(msg)
> > >-
> > >- if connected_slaves != remaining_slaves:
> > >- disconnected_slaves = set(connected_slaves) -\
> > >- set(remaining_slaves)
> > >- msg = "Slaves " + str(list(disconnected_slaves))
+ \
> > >- " disconnected from the controller."
> > >- raise NetTestError(msg)
> > >-
> > >- return result
> > >-
> > >- def _process_message(self, message):
> > >- if message[1]["type"] == "log":
> > >- pass
> > >- else:
> > >- msg = "Unknown message type: %s" %
message[1]["type"]
> > >- raise NetTestError(msg)
> > >-
> > >- def disconnect_slave(self, machine_id):
> > >- soc = self.get_connection(machine_id)
> > >- self.remove_connection(soc)
> > >diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py
> > >index c38aae5..e4ddc54 100644
> > >--- a/lnst/Controller/Wizard.py
> > >+++ b/lnst/Controller/Wizard.py
> > >@@ -1,7 +1,7 @@
> > > """
> > >-Wizard class for creating machine pool .xml files
> > >+Wizard class for creating slave machine config files
> > >
> > >-Copyright 2014 Red Hat Inc.
> > >+Copyright 2015 Red Hat Inc.
> > > Licensed under the GNU General Public Licence, version 2 as
> > > published by the Free Software Foundation; see COPYING for details.
> > > """
> > >@@ -9,150 +9,196 @@ published by the Free Software Foundation; see
> COPYING for details.
> > > __author__ = """
> > > jprochaz(a)redhat.com (Jiri Prochazka)
> > > """
> > >+
> > > import socket
> > > import sys
> > >+import time
> > > import os
> > >-from lnst.Controller.Machine import Machine
> > >-from lnst.Controller.MessageDispatcherLite import MessageDispatcherLite
> > >-from lnst.Common.NetUtils import test_tcp_connection
> > > from lnst.Common.Utils import mkdir_p
> > > from lnst.Common.Config import DefaultRPCPort
> > >+from lnst.Common.ConnectionHandler import send_data, recv_data
> > > from xml.dom.minidom import getDOMImplementation
> > >
> > >+DefaultPoolDir = os.path.expanduser("~/.lnst/pool/")
> > >+PATH_IS_DIR_ACCESSIBLE = 0
> > >+PATH_IS_DIR_NOT_ACCESSIBLE = 1
> > >+PATH_DOES_NOT_EXIST = 2
> > >+PATH_NOT_DIR = 3
> > >+
> > >+
> > > class Wizard:
> > >- def __init__(self):
> > >- self._msg_dispatcher = MessageDispatcherLite()
> > >- self._pool_dir = os.path.expanduser("~/.lnst/pool")
> > >
> > >- def interactive(self):
> > >+ def interactive(self, pool_dir=None):
> > >+ """ Starts Wizard in an interactive mode
> > >+ @param pool_dir Path to pool directory (optional)
> > > """
> > >- Starts Wizard in interactive mode. Wizard requests hostname
> and port
> > >- from user, tests connectivity to entered host, then he tries
> to connect
> > >- and configure slave on host machine, and finally he requests
> list of
> > >- ethernet interfaces in state DOWN. Then he writes them into
> .xml file
> > >- named by user. User can choose which interfaces should be
> added to the
> > >- .xml file.
> > >- """
> > >-
> > >- pool_dir = raw_input("Enter path to pool directory "\
> > >- "(default: ~/.lnst/pool): ")
> > >- if pool_dir != "":
> > >- self._pool_dir = os.path.expanduser(pool_dir)
> > >- print "Pool directory set to %s" % self._pool_dir
> > >+ if pool_dir is None:
> > >+ pool_dir = self._query_pool_dir()
> > >+ else:
> > >+ rv = self._check_path(pool_dir)
> > >+ if rv == PATH_IS_DIR_ACCESSIBLE:
> > >+ print("Pool directory set to '%s'" %
pool_dir)
> > >+ elif rv == PATH_DOES_NOT_EXIST:
> > >+ sys.stderr.write("Path '%s' does not
exist\n" %
> pool_dir)
> > >+ pool_dir = self._query_pool_dir()
> > >+ elif rv == PATH_NOT_DIR:
> > >+ sys.stderr.write("Path '%s' exists but is not
a
> directory\n"
> > >+ % pool_dir)
> > >+ pool_dir = self._query_pool_dir()
> > >+ elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
> > >+ sys.stderr.write("Directory '%s' is not
writable\n" %
> pool_dir)
> > >+ pool_dir = self._query_pool_dir()
> > >
> > > while True:
> > >- while True:
> > >- hostname = raw_input("Enter hostname: ")
> > >- try:
> > >- # Tests if hostname is translatable into IP address
> > >- socket.gethostbyname(hostname)
> > >- break
> > >- except:
> > >- sys.stderr.write("Hostname is not translatable
> into valid"\
> > >- " IP address.\n")
> > >+ hostname = self._query_hostname()
> > >+ port = self._query_port()
> > >+
> > >+ sock = self._get_connection(hostname, port)
> > >+ if sock is None:
> > >+ if self._query_continuation():
> > > continue
> > >- while True:
> > >- port = raw_input("Enter port(default: 9999): ")
> > >- if port == "":
> > >- port = DefaultRPCPort
> > >- try:
> > >- port = int(port)
> > >+ else:
> > > break
> > >- except:
> > >- sys.stderr.write("Invalid port.")
> > >- continue
> > >- msg =
> self._get_suitable_interfaces(socket.gethostbyname(hostname),\
> > >- port)
> > >- if msg:
> > >- self._write_to_xml(msg, hostname, port,
"interactive")
> > >- next_machine = raw_input("Do you want to add another
> machine? "\
> > >- "[Y/n]: ")
> > >- if next_machine.lower() == 'y' or next_machine ==
"":
> > >+
> > >+ machine_interfaces = self._get_machine_interfaces(sock)
> > >+ sock.close()
> > >+
> > >+ if machine_interfaces == {}:
> > >+ sys.stderr.write("No suitable interfaces found on
the
> host "
> > >+ "'%s:%s'\n" %
(hostname, port))
> > >+ elif machine_interfaces is not None:
> > >+ filename = self._query_filename(hostname)
> > >+ self._create_xml(machine_interfaces, hostname,
> > >+ port, pool_dir, filename,
> "interactive")
> > >+ if self._query_continuation():
> > > continue
> > > else:
> > > break
> > >- return
> > >
> > >- def noninteractive(self, hostlist):
> > >- """
> > >- Starts Wizard in noninteractive mode. Wizard gets list of
> hosts and
> > >- ports as arguments. He tries to connect and get info about
> suitable
> > >- interfaces for each of the hosts. Noninteractive mode does not
> prompt
> > >- user about anything, it automatically adds all suitable
> interfaces into
> > >- .xml file named the same as the hostname of the selected
> machine.
> > >+ def noninteractive(self, hostlist, pool_dir):
> > >+ """ Starts Wizard in noninteractive mode
> > >+ @param hostlist List of hosts (mandatory)
> > >+ @param pool_dir Path to pool_directory (optional)
> > > """
> > >- self._mode = "noninteractive"
> > >+ if pool_dir is None:
> > >+ pool_dir = DefaultPoolDir
> > >+
> > >+ rv = self._check_path(pool_dir)
> > >+ if rv == PATH_IS_DIR_ACCESSIBLE:
> > >+ print("Pool directory set to '%s'" %
pool_dir)
> > >+ elif rv == PATH_DOES_NOT_EXIST:
> > >+ sys.stderr.write("Path '%s' does not
exist\n" % pool_dir)
> > >+ pool_dir = self._create_dir(pool_dir)
> > >+ if pool_dir is None:
> > >+ sys.stderr.write("Pool wizard aborted\n")
> > >+ return
> > >+ elif rv == PATH_NOT_DIR:
> > >+ sys.stderr.write("Path '%s' exists but is not a
> directory\n"
> > >+ % pool_dir)
> > >+ sys.stderr.write("Pool wizard aborted\n")
> > >+ return
> > >+ elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
> > >+ sys.stderr.write("Directory '%s' is not
writable\n" %
> pool_dir)
> > >+ sys.stderr.write("Pool wizard aborted\n")
> > >+ return
> > >+
> > >
> > > for host in hostlist:
> > >- # Checks if port was entered along with hostname
> > >+ print("Processing host '%s'" % host)
> > >+ # Check if port was entered along with hostname
> > > if host.find(":") != -1:
> > >- hostname = host.split(':')[0]
> > >- try:
> > >- port = int(host.split(':')[1])
> > >- except:
> > >- port = DefaultRPCPort
> > >+ hostname = host.split(":")[0]
> > >+ if hostname == "":
> > >+ msg = "'%s' does not contain valid
hostname\n" %
> host
> > >+ sys.stderr.write(msg)
> > >+ sys.stderr.write("Skipping host
'%s'\n" % host)
> > >+ continue
> > >+ try:
> > >+ port = int(host.split(":")[1])
> > >+ except:
> > >+ port = DefaultRPCPort
> > >+ msg = "Invalid port entered, "\
> > >+ "using '%s' instead\n" % port
> > >+ sys.stderr.write(msg)
> > > else:
> > > hostname = host
> > > port = DefaultRPCPort
> > >- msg = self._get_suitable_interfaces(hostname, port)
> > >- if not msg:
> > >+
> > >+ if not self._check_hostname(hostname):
> > >+ sys.stderr.write("Hostname '%s' is not
translatable
> into a "
> > >+ "valid IP address\n" %
hostname)
> > >+ sys.stderr.write("Skipping host '%s'\n"
% host)
> > > continue
> > >- self._write_to_xml(msg, hostname, port,
"noninteractive")
> > >
> > >- def _get_suitable_interfaces(self, hostname, port):
> > >- """
> > >- Calls all functions, which are used by both interactive and
> > >- noninteractive mode to get list of suitable interfaces. The
> list is
> > >- saved to variable msg.
> > >- """
> > >- if not test_tcp_connection(hostname, port):
> > >- sys.stderr.write("Host destination '%s:%s'
unreachable.\n"
> > >- % (hostname, port))
> > >- return False
> > >- if not self._connect_and_configure_machine(hostname, port):
> > >- return False
> > >- msg = self._get_device_ifcs(hostname, port)
> > >- self._msg_dispatcher.disconnect_slave(1)
> > >- return msg
> > >+ sock = self._get_connection(hostname, port)
> > >+ if sock is None:
> > >+ sys.stderr.write("Skipping host '%s'\n"
% host)
> > >+ continue
> > >
> > >- def _get_device_ifcs(self, hostname, port):
> > >- """
> > >- Sends RPC call request to Slave to call function get_devices,
> which
> > >- returns list of ethernet devices which are in state DOWN.
> > >- """
> > >- msg = self._machine._rpc_call("get_devices")
> > >- if msg == {}:
> > >- sys.stderr.write("No suitable interfaces found on the
> slave "\
> > >- "'%s:%s'.\n" % (hostname,
port))
> > >- return False
> > >- return msg
> > >+ machine_interfaces = self._get_machine_interfaces(sock)
> > >+ sock.close()
> > >
> > >- def _connect_and_configure_machine(self, hostname, port):
> > >- """
> > >- Connects to Slave and configures it
> > >+ if machine_interfaces is {}:
> > >+ sys.stderr.write("No suitable interfaces found on
the
> host "
> > >+ "'%s:%s'\n" %
(hostname, port))
> > >+ continue
> > >+ elif machine_interfaces is None:
> > >+ continue
> > >+ else:
> > >+ filename = hostname + ".xml"
> > >+ self._create_xml(machine_interfaces, hostname,
> > >+ port, pool_dir, filename,
> "noninteractive")
> > >+
> > >+ def _check_hostname(self, hostname):
> > >+ """ Checks hostnames translatibility
> > >+ @param hostname Hostname which is checked whether it's valid
> > >+ @return True if valid hostname was entered, False otherwise
> > > """
> > > try:
> > >- self._machine = Machine(1, hostname, None, port)
> > >- self._machine.set_rpc(self._msg_dispatcher)
> > >- self._machine.configure("MachinePoolWizard")
> > >+ # Test if hostname is translatable into IP address
> > >+ socket.gethostbyname(hostname)
> > > return True
> > > except:
> > >- sys.stderr.write("Remote machine '%s:%s'
configuration
> failed!\n"
> > >- % (hostname, port))
> > >- self._msg_dispatcher.disconnect_slave(1)
> > > return False
> > >
> > >- def _write_to_xml(self, msg, hostname, port, mode):
> > >+ def _check_path(self, pool_dir):
> > >+ """ Checks if path exists, is dir and is accessible
by user
> > >+ @param pool_dir Path to checked directory
> > >+ @return True if user can write in entered directory, False
> otherwise
> > > """
> > >- Used for writing desired output into .xml file. In interactive
> mode,
> > >- user is prompted for every interface, in noninteractive mode
> all
> > >- interfaces are automatically added to the .xml file.
> > >+ if not os.path.exists(pool_dir):
> > >+ return PATH_DOES_NOT_EXIST
> > >+ if os.path.isdir(pool_dir):
> > >+ if os.access(pool_dir, os.W_OK):
> > >+ return PATH_IS_DIR_ACCESSIBLE
> > >+ else:
> > >+ return PATH_IS_DIR_NOT_ACCESSIBLE
> > >+ else:
> > >+ return PATH_NOT_DIR
> > >+
> > >+ def _create_dir(self, pool_dir):
> > >+ """ Creates specified directory
> > >+ @param pool_dir Directory to be created
> > >+ @return Path to dir which was created, None if no directory
> was created
> > >+ """
> > >+ try:
> > >+ mkdir_p(pool_dir)
> > >+ print("Dir '%s' has been created" %
pool_dir)
> > >+ return pool_dir
> > >+ except:
> > >+ sys.stderr.write("Failed creating dir")
> > >+ return None
> > >+
> > >+ def _create_xml(self, machine_interfaces, hostname,
> > >+ port, pool_dir, filename, mode):
> > >+ """ Creates slave machine XML file
> > >+ @param machine_interfaces Dictionary with machine's
interfaces
> > >+ @param hostname Hostname of the machine
> > >+ @param port Port on which LNST listens on the machine
> > >+ @param pool_dir Path to directory where XML file will be
> created
> > >+ @param filename Name of the XML file
> > >+ @param mode Mode in which wizard was started
> > > """
> > >- if mode == "interactive":
> > >- output_file = raw_input("Enter the name of the output
.xml
> file "\
> > >- "(without .xml, default is
> hostname.xml): ")
> > >- if mode == "noninteractive" or output_file ==
"":
> > >- output_file = hostname
> > >
> > > impl = getDOMImplementation()
> > > doc = impl.createDocument(None, "slavemachine", None)
> > >@@ -166,42 +212,164 @@ class Wizard:
> > > interfaces_el = doc.createElement("interfaces")
> > > top_el.appendChild(interfaces_el)
> > >
> > >- devices_added = 0
> > >- for interface in msg.itervalues():
> > >- if mode == 'interactive':
> > >+ interfaces_added = 0
> > >+ for iface in machine_interfaces.itervalues():
> > >+ if mode == "interactive":
> > > answer = raw_input("Do you want to add interface
'%s'
> (%s) "
> > >- "to the recipe? [Y/n]" %
> (interface['name'],
> > >-
> interface['hwaddr']))
> > >- if mode == "noninteractive" or answer.lower() ==
'y'\
> > >+ "to the recipe? [Y/n]: " %
> (iface["name"],
> > >+
> iface["hwaddr"]))
> > >+ if mode == "noninteractive" or answer.lower() ==
"y"\
> > > or answer == "":
> > >- devices_added += 1
> > >+ interfaces_added += 1
> > > eth_el = doc.createElement("eth")
> > >- eth_el.setAttribute("id",
interface['name'])
> > >+ eth_el.setAttribute("id",
iface["name"])
> > > eth_el.setAttribute("label",
"default_network")
> > > interfaces_el.appendChild(eth_el)
> > > params_el = doc.createElement("params")
> > > eth_el.appendChild(params_el)
> > > param_el = doc.createElement("param")
> > > param_el.setAttribute("name",
"hwaddr")
> > >- param_el.setAttribute("value",
interface['hwaddr'])
> > >+ param_el.setAttribute("value",
iface["hwaddr"])
> > > params_el.appendChild(param_el)
> > >- if devices_added == 0:
> > >- sys.stderr.write("You didn't add any interface, no
file
> '%s.xml' "\
> > >- "will be created!\n" % output_file)
> > >+ if interfaces_added == 0:
> > >+ sys.stderr.write("You didn't add any interface, no
file "
> > >+ "'%s' will be created\n" %
filename)
> > > return
> > >
> > >- mkdir_p(self._pool_dir)
> > >
> > > try:
> > >- f = open(self._pool_dir + "/" + output_file +
".xml", 'w')
> > >+ f = open(pool_dir + "/" + filename, "w")
> > > f.write(doc.toprettyxml())
> > > f.close()
> > > except:
> > >- sys.stderr.write("File '%s.xml' could not be
opened "\
> > >- "or data written." %
output_file+"\n")
> > >- raise WizardException()
> > >+ msg = "File '%s/%s' could not be opened or data
written\n"\
> > >+ % (pool_dir, filename)
> > >+ raise WizardException(msg)
> >
> > You raise an exception but this is not handled here nor in
> > lnst-pool-wizard. Either remove it completely or add try-except block
> > somewhere in the code.
> >
> > >+
> > >+ print("File '%s' successfuly created" %
filename)
> > >+
> > >+ def _get_connection(self, hostname, port):
> > >+ """ Connects to machine
> > >+ @param hostname Hostname of the machine
> > >+ @param port Port of the machine
> > >+ @return Connected socket if connection was successful, None
> otherwise
> > >+ """
> > >+ try:
> > >+ sock = socket.create_connection((hostname, port))
> > >+ return sock
> > >+ except socket.error:
> > >+ sys.stderr.write("Connection to remote host
'%s:%s'
> failed\n"
> > >+ % (hostname, port))
> > >+ return None
> > >+
> > >+ def _get_machine_interfaces(self, sock):
> > >+ """ Gets machine interfaces via RPC call
> > >+ @param sock Socket used for connecting to machine
> > >+ @return Dictionary with machine interfaces or None if RPC call
> fails
> > >+ """
> > >+ msg = {"type": "command",
"method_name": "get_devices",
> "args": {}}
> > >+ if not send_data(sock, msg):
> > >+ sys.stderr.write("Could not send request to slave
> machine\n")
> > >+ return None
> > >+
> > >+ while True:
> > >+ data = recv_data(sock)
> > >+ if data["type"] == "result":
> > >+ return data["result"]
> > >+
> > >+ def _query_continuation(self):
> > >+ """ Queries user for adding next machine
> > >+ @return True if user wants to add another machine, False
> otherwise
> > >+ """
> > >+ answer = raw_input("Do you want to add another machine?
[Y/n]:
> ")
> > >+ if answer.lower() == "y" or answer == "":
> > >+ return True
> > >+ else:
> > >+ return False
> > >+
> > >+ def _query_dir_creation(self, pool_dir):
> > >+ """ Queries user for creating specified directory
> > >+ @return True if user wants to create the directory, False
> otherwise
> > >+ """
> > >+ answer = raw_input("Create dir '%s'? [Y/n]: " %
pool_dir)
> > >+ if answer.lower() == 'y' or answer == "":
> > >+ return True
> > >+ else:
> > >+ return False
> > >+
> > >+ def _query_filename(self, hostname):
> > >+ """ Queries user for name of the file
> > >+ @hostname Hostname of the machine which is used as default
> filename
> > >+ @return Name of the file with .xml extension
> > >+ """
> > >+ output_file = raw_input("Enter the name of the output .xml
> file "
> > >+ "(without .xml, default is
'%s.xml'): "
> > >+ % hostname)
> > >+ if output_file == "":
> > >+ return hostname + ".xml"
> > >+ else:
> > >+ return output_file + ".xml"
> > >+
> > >+ def _query_hostname(self):
> > >+ """ Queries user for hostname
> > >+ @return Valid (is translatable to an IP address) hostname
> > >+ """
> > >+ while True:
> > >+ hostname = raw_input("Enter hostname: ")
> > >+ if hostname == "":
> > >+ sys.stderr.write("No hostname entered\n")
> > >+ continue
> > >+ if self._check_hostname(hostname):
> > >+ return hostname
> > >+ else:
> > >+ sys.stderr.write("Hostname '%s' is not
translatable
> into a "
> > >+ "valid IP address\n" %
hostname)
> > >+
>
> Trailing whitespace here...
>
Will be fixed in next patch
>
> > >+ def _query_pool_dir(self):
> > >+ """ Queries user for pool directory
> > >+ @return Valid (is writable by user) path to directory
> > >+ """
> > >+ while True:
> > >+ pool_dir = raw_input("Enter path to a pool directory
"
> > >+ "(default: '%s'): " %
DefaultPoolDir)
> > >+ if pool_dir == "":
> > >+ pool_dir = DefaultPoolDir
> > >+
> > >+ pool_dir = os.path.expanduser(pool_dir)
> > >+ rv = self._check_path(pool_dir)
> > >+ if rv == PATH_IS_DIR_ACCESSIBLE:
> > >+ print("Pool directory set to '%s'" %
pool_dir)
> > >+ return pool_dir
> > >+ elif rv == PATH_DOES_NOT_EXIST:
> > >+ sys.stderr.write("Path '%s' does not
exist\n"
> > >+ % pool_dir)
> > >+ if self._query_dir_creation(pool_dir):
> > >+ created_dir = self._create_dir(pool_dir)
> > >+ if created_dir is not None:
> > >+ return created_dir
> > >+
> > >+ elif rv == PATH_NOT_DIR:
> > >+ sys.stderr.write("Path '%s' exists but is not
a
> directory\n"
> > >+ % pool_dir)
> > >+ elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
> > >+ sys.stderr.write("Directory '%s' is not
writable\n"
> > >+ % pool_dir)
> > >+
> > >+ def _query_port(self):
> > >+ """ Queries user for port
> > >+ @return Integer representing port
> > >+ """
> > >+ while True:
> > >+ port = raw_input("Enter port (default: %d): " %
> DefaultRPCPort)
> > >+ if port == "":
> > >+ return DefaultRPCPort
> > >+ else:
> > >+ try:
> > >+ port = int(port)
> > >+ return port
> > >+ except:
> > >+ sys.stderr.write("Invalid port entered\n")
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> What happens in this case? The _query_port method will return None to
> the calling function which is "interactive()" where the result is not
> checked and passed directly to _get_connection where you'll try to
> connect to a None port.
>
> In case invalid port is entered, the except part prints error message and
then continues in while loop, meaning that it queries user for port again,
until he enters a valid one. If I'm not mistaken, the method does not
return None in any scenario.
ah, in that case it's fine, I didn't notice the while True loop...