From a95ff20a5be8753b073a4d82a373a56b1afa0d68 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Wed, 7 Mar 2012 06:49:44 -0500
Subject: [PATCH] Simply & unify console handling for libvirt drivers

Currently the libvirt.xml.template file contains the following
console definitions

        <!-- The order is significant here.  File must be defined first -->
        <serial type="file">
            <source path='${basepath}/console.log'/>
            <target port='1'/>
        </serial>

        <console type='pty' tty='/dev/pts/2'>
            <source path='/dev/pts/2'/>
            <target port='0'/>
        </console>

        <serial type='pty'>
            <source path='/dev/pts/2'/>
            <target port='0'/>
        </serial>

There are multiple things wrong with this

 - LXC and Xen guests don't honour the <serial> elements
 - The <console> element shouldn't have any <source> element
   of 'tty' attribute set when type=pty, since they are
   dynamically allocated
 - The <console> element will ignored if the <serial>
   element is set and the hypervisor supports this
 - It doesn't say why multiple serial elements are used
   or why the order is important. The reason is that
   the QEMU pty driver throws away data when no client
   is connected. This means we can't use it as a basis
   for the persistent log file. Instead we need two
   separate serial ports, the first of which is used for
   the logfile

In addition in the nova/virt/libvirt/connect.py class the
'get_console_output' method has separate special-case
handling for Xen and LXC despite the fact that both work
in the same way.

All this can be significantly simplified, to unify console
handling across all libvirt drivers. First replacing all
the existing XML with just

  #if $type == 'qemu'
        <serial type='file'>
            <source path='${basepath}/console.log'/>
        </serial>
        <serial type='pty'/>
  #else
        <console type='pty'/>
  #end if

This lets Xen/UML/LXC just use a regular PTY based console,
while special casing QEMU. It is minimal XML, letting
libvirt automatically fill in other attributes

In the code, the get_console_output method can remove the
explicit checks for Xen/LXC and instead be conditionalized
based on what the XML shows.

Finally calling out to 'virsh ttyiname' is pointless since
nova already has a connection to libvirt which can be used
to fetch the XML & extract the TTY path.

Change-Id: I6a966df4ea72e07dbc227683c4225670984fc507
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 nova/tests/fakelibvirt.py  | 13 +++++++++----
 nova/tests/test_libvirt.py | 17 +++++++++++++++--
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/nova/tests/fakelibvirt.py b/nova/tests/fakelibvirt.py
index b8f9bb43..24ff5ae5 100644
--- a/nova/tests/fakelibvirt.py
+++ b/nova/tests/fakelibvirt.py
@@ -357,12 +357,17 @@ class Domain(object):
                function='0x1'/>
     </controller>
     %(nics)s
-    <serial type='pty'>
-      <source pty='/dev/pts/27'/>
+    <serial type='file'>
+      <source path='dummy.log'/>
       <target port='0'/>
     </serial>
-    <console type='pty'>
-      <target type='serial' port='0'/>
+    <serial type='pty'>
+      <source pty='/dev/pts/27'/>
+      <target port='1'/>
+    </serial>
+    <console type='file'>
+      <source path='dummy.log'/>
+      <target port='0'/>
     </console>
     <input type='tablet' bus='usb'/>
     <input type='mouse' bus='ps2'/>
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index d8a44eec..f0e6d498 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -860,6 +860,21 @@ class LibvirtConnTestCase(test.TestCase):
                     check = (lambda t: t.find('./os/initrd'), None)
                 check_list.append(check)
 
+            if hypervisor_type in ['qemu', 'kvm']:
+                check = (lambda t: t.findall('./devices/serial')[0].get(
+                        'type'), 'file')
+                check_list.append(check)
+                check = (lambda t: t.findall('./devices/serial')[1].get(
+                        'type'), 'pty')
+                check_list.append(check)
+                check = (lambda t: t.findall('./devices/serial/source')[0].get(
+                        'path').split('/')[1], 'console.log')
+                check_list.append(check)
+            else:
+                check = (lambda t: t.find('./devices/console').get(
+                        'type'), 'pty')
+                check_list.append(check)
+
         parameter = './devices/interface/filterref/parameter'
         common_checks = [
             (lambda t: t.find('.').tag, 'domain'),
@@ -869,8 +884,6 @@ class LibvirtConnTestCase(test.TestCase):
             (lambda t: t.findall(parameter)[1].get('name'), 'DHCPSERVER'),
             (lambda t: _ipv4_like(t.findall(parameter)[1].get('value'),
                                   '192.168.*.1'), True),
-            (lambda t: t.find('./devices/serial/source').get(
-                'path').split('/')[1], 'console.log'),
             (lambda t: t.find('./memory').text, '2097152')]
         if rescue:
             common_checks += [