Merge lp:~jeffmarcom/checkbox/kvm_console_hijack_fix into lp:checkbox

[Jeff Marcom]

So the console terminal was being hijacked by the KVM boot process and not letting go, even after the running KVM instance is terminated in the script. This simply calls a 'reset' command to regain access of terminal. This also includes a fix so that the instance will actual terminate at the end of the script successfully.

[Zygmunt Krynicki (zyga)]

53 + # Reset Console window to regain control from VM Serial I/0
54 + try:
55 + check_call('reset')
56 + except CalledProcessError:
57 + pass

just use call(), no need to check if you discard the check anyway

review: Needs Fixing

[Jeff Lane  (bladernr)]

well, it may be unnecessary anyway... via SSH, this successfully runs without hijacking the session, but when run locally, it still fails.

The difference, other than the obvious ssh bit is that the ssh session is running in xterm while the local is running in a different console.

I'm wondering if running the VM using the libvirt stuff wouldn't be cleaner... not sure though. So need to try it out.

review: Needs Fixing

[Jeff Marcom (jeffmarcom)]

I tried this test again with the latest fix in checkbox-certification-server in a linux terminal (not xterm) and I did not lose the console.

review: Needs Resubmitting

[Jeff Lane  (bladernr)]

I too tried the latest fix (including the change to call() from check_call()) and it does seem to resolve the problem of console hijacking.

I was able to run the KVM test, the reboot manual test, and then the followin Xen test from my own branches successfully .

review: Approve

[lp:~jeffmarcom/checkbox/kvm_console_hijack_fix]

2034. By Jeff Marcom on 2013-04-05

Fixed issue where a terminal would sometimes return non 0 on reset command

Signed-off-by: Jeff Marcom <email address hidden>

2035. By Jeff Marcom on 2013-04-08

Changed subprocess to PIPE stdin and changed subprocess call

Signed-off-by: Jeff Marcom <email address hidden>

2036. By Jeff Marcom on 2013-04-08

Removed try and except block from subprocess call

Signed-off-by: Jeff Marcom <email address hidden>

[Zygmunt Krynicki (zyga)]

Let's land this.

There's a longer story that we talked about on IRC about using PIPE vs DEVNULL for stdin but since testing this is expensive and PIPE was tested to work okay it should land as is. If possible we should see if using DEVNULL works as it's cleaner conceptually

review: Approve

[Jeff Marcom (jeffmarcom)]

> Let's land this.
>
> There's a longer story that we talked about on IRC about using PIPE vs DEVNULL
> for stdin but since testing this is expensive and PIPE was tested to work okay
> it should land as is. If possible we should see if using DEVNULL works as it's
> cleaner conceptually

I tested pushing stdin to dev null by changing it to:
stdin=open(os.devnull, 'wb')

This causes the test to always fail as the KVM I/O messages are never properly forwarded to the log file. Instead doing stdin=PIPE works fine.

[Zygmunt Krynicki (zyga)]

> > Let's land this.
> >
> > There's a longer story that we talked about on IRC about using PIPE vs
> DEVNULL
> > for stdin but since testing this is expensive and PIPE was tested to work
> okay
> > it should land as is. If possible we should see if using DEVNULL works as
> it's
> > cleaner conceptually
>
> I tested pushing stdin to dev null by changing it to:
> stdin=open(os.devnull, 'wb')
>
> This causes the test to always fail as the KVM I/O messages are never properly
> forwarded to the log file. Instead doing stdin=PIPE works fine.

You used devnull incorrectly. I literally meant call(stdin=subprocess.DEVNULL). If you open /dev/null yourself then the correct way would be to open it for _reading_ not writing as the child process tries to read from it, not write to it.

=== modified file 'debian/changelog'
--- debian/changelog	2013-04-08 15:56:09 +0000
+++ debian/changelog	2013-04-08 23:04:21 +0000
@@ -13,7 +13,11 @@
   [ Jeff Lane ]
   * jobs/info.txt.in: fixed bad driver name in audio_codecs job (LP: #1165215)
 
- -- Brendan Donegan <[e-mail]>  Thu, 04 Apr 2013 17:04:44 +0100
+   [ Jeff Marcom ]
+   * scripts/virtualization - Fixes issue where console terminal would 
+     remain hijacked by child kvm process (LP: #1164028)
+
+ -- Jeff Marcom <[e-mail]>  Fri, 05 Apr 2013 17:04:44 +0100
 
 checkbox (0.15.6) raring; urgency=low
 

=== modified file 'scripts/virtualization'
--- scripts/virtualization	2013-03-18 18:46:06 +0000
+++ scripts/virtualization	2013-04-08 23:04:21 +0000
@@ -27,8 +27,15 @@
 import os
 import logging
 import lsb_release
+import shlex
 import signal
-from subprocess import Popen, PIPE, CalledProcessError, check_output
+from subprocess import (
+    Popen, 
+    PIPE, 
+    CalledProcessError, 
+    check_output,
+    call
+)
 import sys
 import tempfile
 import time
@@ -123,8 +130,8 @@
 
         # Start Virtual machine
         self.process = Popen(
-            params, stderr=file, stdout=file,
-            universal_newlines=True, shell=True)
+            shlex.split(params), stdin=PIPE, stderr=file, stdout=file,
+            universal_newlines=True, shell=False)
 
     @classmethod
     def create_cloud_disk(cls):
@@ -184,6 +191,8 @@
 
                 if instance is not False:
                     time.sleep(self.timeout)
+                    # Reset Console window to regain control from VM Serial I/0
+                    call('reset')
                     # Check to be sure VM boot was successful
                     if "END SSH HOST KEY KEYS" \
                     in open(self.debug_file, 'r').read():