[et-mgmt-tools] [PATCH] virt-install: Add --serial and --parallel options

[Cole Robinson <crobinso redhat com>]

The attached patch adds --serial and --parallel options to virt-install, for
attaching the respective devices to the new VM. Isn't much to say here, some
examples explain it all:

Serial PTY:

--serial pty

Parallel to a file:

--serial file,path=/tmp/foo.log

TCP net console in server mode, using telnet format:

--serial tcp,host=0.0.0.0:2222,mode=bind,protocol=telnet

UDP net console, sending output to the specified host:

--serial udp,host=192.168.10.20:4444

The man page has examples for several other types as well. --serial and
--parallel share the same option format, as they do in the libvirt xml.

Any questions or comments appreciated.

- Cole

# HG changeset patch
# User Cole Robinson <crobinso redhat com>
# Date 1247102899 14400
# Node ID 7751290d749d678e6a97a32ea5455af6e2226e78
# Parent  77b5e67be942bd75e5226612ee5fa5ab36c8a56d
virt-install: Add --serial and --parallel options.

Options take the same form of OPT1=VAL1,OPT2=VAL2,... similar to the --disk
option. The docs describe the rest.

diff -r 77b5e67be942 -r 7751290d749d man/en/virt-install.pod.in
--- a/man/en/virt-install.pod.in	Wed Jul 08 21:23:32 2009 -0400
+++ b/man/en/virt-install.pod.in	Wed Jul 08 21:28:19 2009 -0400
@@ -150,6 +150,82 @@
 
 Attach a virtual audio device to the guest.
 
+=item --parallel=CHAROPTS
+
+=item --serial=CHAROPTS
+
+Specifies a serial device to attach to the guest, with various options. The
+general format of a serial string is
+
+    --serial type,opt1=val1,opt2=val2,...
+
+--serial and --parallel devices share all the same options, unless otherwise
+noted. Some of the types of character device redirection are:
+
+=over 4
+
+=item B<--serial pty>
+
+Psuedo TTY. The allocated pty will be listed in the running guests XML
+description.
+
+=item B<--serial dev,path=HOSTPATH>
+
+Host device. For serial devices, this could be /dev/ttyS0. For parallel
+devices, this could be /dev/parport0.
+
+=item B<--serial file,path=FILENAME>
+
+Write output to FILENAME.
+
+=item B<--serial pipe,path=PIPEPATH>
+
+Named pipe (see pipe(7))
+
+=item B<--serial tcp,host=HOST:PORT,mode=MODE,protocol=PROTOCOL>
+
+TCP net console. MODE is either 'bind' (wait for connections on HOST:PORT)
+or 'connect' (send output to HOST:PORT), default is 'connect'. HOST defaults
+to '127.0.0.1', but PORT is required. PROTOCOL can be either 'raw' or 'telnet'
+(default 'raw'). If 'telnet', the port acts like a telnet server or client.
+Some examples:
+
+Connect to localhost, port 1234:
+
+--serial tcp,host=:1234
+
+Wait for connections on any address, port 4567:
+
+--serial tcp,host=0.0.0.0:4567,mode=bind
+
+Wait for telnet connection on localhost, port 2222. The user could then
+connect interactively to this console via 'telnet localhost 2222':
+
+--serial tcp,host=:2222,mode=bind,protocol=telnet
+
+=item B<--serial udp,host=CONNECT_HOST:PORT,bind_port=BIND_HOST:BIND_PORT>
+
+UDP net console. HOST:PORT is the destination to send output to (default
+HOST is '127.0.0.1', PORT is required. BIND_HOST:PORT is the optional local
+address to bind to (default BIND_HOST is 127.0.0.1, but is only set if
+BIND_PORT is specified.) Some examples:
+
+Send output to default syslog port (may need to edit /etc/rsyslog.conf
+accordingly):
+
+--serial udp,host=:514
+
+Send output to remote host 192.168.10.20, port 4444 (this output can be
+read on the remote host using 'nc -u -l 4444':
+
+--serial udp,host=192.168.10.20:4444
+
+=item B<--serial unix,path=UNIXPATH,mode=MODE>
+
+Unix socket (see unix(7). MODE has similar behavior and defaults as 'tcp'.
+
+=back
+
 =item  --noapic
 
 Override the OS type / variant to disables the APIC setting for fully
diff -r 77b5e67be942 -r 7751290d749d tests/clitest.py
--- a/tests/clitest.py	Wed Jul 08 21:23:32 2009 -0400
+++ b/tests/clitest.py	Wed Jul 08 21:28:19 2009 -0400
@@ -211,6 +211,36 @@
 
      }, # category "graphics"
 
+    "char" : {
+     "char_args": "--hvm --nographics --noautoconsole --nodisks --pxe",
+
+     "valid": [
+        # Simple devs
+        "--serial pty --parallel null",
+        # Some with options
+        "--serial file,path=/tmp/foo --parallel unix,path=/tmp/foo --parallel null",
+        # UDP
+        "--parallel udp,host=0.0.0.0:1234,bind_host=127.0.0.1:1234",
+        # TCP
+        "--serial tcp,mode=bind,host=0.0.0.0:1234",
+        # Unix
+        "--parallel unix,path=/tmp/foo-socket",
+        # TCP w/ telnet
+        "--serial tcp,host=:1234,protocol=telnet",
+     ],
+     "invalid" : [
+        # Bogus device type
+        "--parallel foobah",
+        # Unix with no path
+        "--serial unix",
+        # Path where it doesn't belong
+        "--serial null,path=/tmp/foo",
+        # Nonexistent argument
+        "--serial udp,host=:1234,frob=baz",
+     ],
+
+     }, # category 'char'
+
      "misc": {
       "misc_args": "--nographics --noautoconsole",
 
diff -r 77b5e67be942 -r 7751290d749d virt-install
--- a/virt-install	Wed Jul 08 21:23:32 2009 -0400
+++ b/virt-install	Wed Jul 08 21:28:19 2009 -0400
@@ -33,6 +33,8 @@
 import virtinst
 import virtinst.CapabilitiesParser
 import virtinst.cli as cli
+from virtinst.VirtualCharDevice import VirtualCharDevice
+from virtinst.VirtualDevice import VirtualDevice
 from virtinst.cli import fail
 
 import gettext
@@ -45,6 +47,17 @@
 DEFAULT_POOL_PATH = "/var/lib/libvirt/images"
 DEFAULT_POOL_NAME = "default"
 
+def partition(string, sep):
+    if not string:
+        return (None, None, None)
+
+    if string.count(sep):
+        splitres = string.split(sep, 1)
+        ret = (splitres[0], sep, splitres[1])
+    else:
+        ret = (string, None, None)
+    return ret
+
 def build_default_pool(guest):
 
     if not virtinst.util.is_storage_capable(guest.conn):
@@ -71,6 +84,70 @@
         raise RuntimeError(_("Couldn't create default storage pool '%s': %s") %
                              (DEFAULT_POOL_PATH, str(e)))
 
+def parse_char_option(guest, char_type, optstring):
+    """
+    Helper to parse --serial/--parallel options
+    """
+    # Peel the char type off the front
+    dev_type, ignore, optstring = partition(optstring, ",")
+
+    opts = cli.parse_optstr(optstring)
+
+    dev = VirtualCharDevice.get_dev_instance(guest.conn, char_type, dev_type)
+
+    def set_param(paramname, dictname, val=None):
+        if not val:
+            if opts.has_key(dictname):
+                val = opts[dictname]
+                del(opts[dictname])
+            else:
+                return
+
+        if not hasattr(dev, paramname):
+            raise ValueError(_("%(chartype)s type %(devtype)s does not "
+                                "support '%(optname)s' option.") %
+                                {"chartype" : char_type, "devtype": dev_type,
+                                 "optname" : dictname} )
+        setattr(dev, paramname, val)
+
+    def parse_host(key):
+        host, ignore, port = partition(opts.get(key), ":")
+        if opts.has_key(key):
+            del(opts[key])
+
+        return host, port
+
+    host, port = parse_host("host")
+    bind_host, bind_port = parse_host("bind_host")
+
+    set_param("source_path", "path")
+    set_param("source_mode", "mode")
+    set_param("protocol",   "protocol")
+    set_param("source_host", "host", host)
+    set_param("source_port", "host", port)
+    set_param("bind_host", "bind_host", bind_host)
+    set_param("bind_port", "bind_host", bind_port)
+
+    # If extra parameters, then user passed some garbage param
+    if opts:
+        raise ValueError(_("Unknown option(s) %s") % opts.keys())
+
+    # Try to generate dev XML to perform upfront validation
+    dev.get_xml_config()
+
+    return dev
+
+def get_chardevs(char_type, opts, guest):
+
+    for optstr in cli.listify(opts):
+        try:
+            dev = parse_char_option(guest, char_type, optstr)
+            guest.add_device(dev)
+        except Exception, e:
+            fail(_("Error in %(chartype)s device parameters: %(err)s") %
+                 {"chartype": char_type, "err": str(e) })
+
+
 def parse_disk_option(guest, path, size):
     """helper to properly parse --disk options"""
     abspath = None
@@ -81,14 +158,6 @@
     sparse = True
     option_whitelist = ["perms", "cache", "bus", "device", "size", "sparse"]
 
-    def partition(string, sep):
-        if string.count(sep):
-            splitres = string.split(sep, 1)
-            ret = (splitres[0], sep, splitres[1])
-        else:
-            ret = (string, None, None)
-        return ret
-
     # Strip media type
     path, ignore, optstr = partition(path, ",")
     if path.startswith("path="):
@@ -459,6 +528,12 @@
                       action="callback", callback=cli.check_before_store,
                       help=_("The OS variant for fully virtualized guests, "
                              "e.g. 'fedora6', 'rhel5', 'solaris10', 'win2k'"))
+    geng.add_option("", "--serial", type="string", dest="serials",
+                    action="callback", callback=cli.check_before_append,
+                    help=_("Add a serial device to the domain."))
+    geng.add_option("", "--parallel", type="string", dest="parallels",
+                    action="callback", callback=cli.check_before_append,
+                    help=_("Add a parallel device to the domain."))
     geng.add_option("", "--host-device", type="string", dest="hostdevs",
                     action="callback", callback=cli.check_before_append,
                     help=_("Physical host device to attach to the domain."))
@@ -549,7 +624,6 @@
     netg.add_option("-m", "--mac", type="string", dest="mac",
                     action="callback", callback=cli.check_before_append,
                     help=optparse.SUPPRESS_HELP)
-
     parser.add_option_group(netg)
 
     vncg = cli.graphics_option_group(parser)
@@ -661,6 +735,9 @@
     cli.get_cpuset(options.cpuset, guest.memory, guest, conn)
     if ishvm:
         cli.get_sound(options.sound, guest)
+        get_chardevs(VirtualDevice.VIRTUAL_DEV_SERIAL, options.serials, guest)
+        get_chardevs(VirtualDevice.VIRTUAL_DEV_PARALLEL, options.parallels,
+                     guest)
 
     # set up disks
     get_disks(options.file_path, options.diskopts, options.disksize,
diff -r 77b5e67be942 -r 7751290d749d virtinst/cli.py
--- a/virtinst/cli.py	Wed Jul 08 21:23:32 2009 -0400
+++ b/virtinst/cli.py	Wed Jul 08 21:28:19 2009 -0400
@@ -452,6 +452,14 @@
 # Ask for attributes
 #
 
+def listify(l):
+    if l is None:
+        return []
+    elif type(l) != list:
+        return [ l ]
+    else:
+        return l
+
 def get_name(name, guest, image_name=None):
     prompt_txt = _("What is the name of your virtual machine?")
     err_txt = _("A name is required for the virtual machine.")
@@ -614,14 +622,6 @@
              "network" : network_name, "model" : model , "macaddr" : mac }
 
 def digest_networks(conn, macs, bridges, networks, nics = 0):
-    def listify(l):
-        if l is None:
-            return []
-        elif type(l) != list:
-            return [ l ]
-        else:
-            return l
-
     macs     = listify(macs)
     bridges  = listify(bridges)
     networks = listify(networks)