From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH 5/9] KVM test: Log the content from guest serial console Date: Thu, 06 May 2010 11:03:43 +0800 Message-ID: <4BE2318F.2040605@redhat.com> References: <20100426095656.26268.50549.stgit@localhost.localdomain> <20100426100402.26268.22330.stgit@localhost.localdomain> <4BD83780.2040600@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: autotest@test.kernel.org, lmr@redhat.com, kvm@vger.kernel.org To: Michael Goldish Return-path: Received: from mx1.redhat.com ([209.132.183.28]:1026 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751043Ab0EFDDs (ORCPT ); Wed, 5 May 2010 23:03:48 -0400 In-Reply-To: <4BD83780.2040600@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Michael Goldish wrote: > On 04/26/2010 01:04 PM, Jason Wang wrote: > >> This patch tries to get the content of guest serial and log it into >> the debug directoy of the testcase through a dedicated thread which is >> created in the preprocessing and ended in the postprocessing. The >> params of serial_mode must be set to "dump" in order to make use of >> this feature. >> >> Signed-off-by: Jason Wang >> --- >> client/tests/kvm/kvm_preprocessing.py | 59 +++++++++++++++++++++++++++++++- >> client/tests/kvm/tests_base.cfg.sample | 1 + >> 2 files changed, 59 insertions(+), 1 deletions(-) >> >> diff --git a/client/tests/kvm/kvm_preprocessing.py b/client/tests/kvm/kvm_preprocessing.py >> index 4b9290c..50d0e35 100644 >> --- a/client/tests/kvm/kvm_preprocessing.py >> +++ b/client/tests/kvm/kvm_preprocessing.py >> @@ -1,4 +1,5 @@ >> import sys, os, time, commands, re, logging, signal, glob, threading, shutil >> +import socket, select >> from autotest_lib.client.bin import test, utils >> from autotest_lib.client.common_lib import error >> import kvm_vm, kvm_utils, kvm_subprocess, ppm_utils >> @@ -13,7 +14,8 @@ except ImportError: >> >> _screendump_thread = None >> _screendump_thread_termination_event = None >> - >> +_serialdump_thread = None >> +_serialdump_thread_termination_event = None >> >> def preprocess_image(test, params): >> """ >> @@ -267,6 +269,16 @@ def preprocess(test, params, env): >> args=(test, params, env)) >> _screendump_thread.start() >> >> + # Start the serial dump thread >> + if params.get("serial_mode") == "dump": >> + logging.debug("Starting serialdump thread") >> + global _serialdump_thread, _serialdump_thread_termination_event >> + _serialdump_thread_termination_event = threading.Event() >> + _serialdump_thread = threading.Thread(target=_dump_serial_console, >> + args=(test, params, env)) >> + _serialdump_thread.start() >> + >> + >> >> def postprocess(test, params, env): >> """ >> @@ -286,6 +298,13 @@ def postprocess(test, params, env): >> _screendump_thread_termination_event.set() >> _screendump_thread.join(10) >> >> + # Terminate the serialdump thread >> + global _serialdump_thread, _serialdump_thread_termination_event >> + if _serialdump_thread: >> + logging.debug("Terminating serialdump thread...") >> + _serialdump_thread_termination_event.set() >> + _serialdump_thread.join(10) >> + >> # Warn about corrupt PPM files >> for f in glob.glob(os.path.join(test.debugdir, "*.ppm")): >> if not ppm_utils.image_verify_ppm_file(f): >> @@ -450,3 +469,41 @@ def _take_screendumps(test, params, env): >> if _screendump_thread_termination_event.isSet(): >> break >> _screendump_thread_termination_event.wait(delay) >> + >> +def _dump_serial_console(test, params, env): >> + global _serialdump_thread_termination_event >> + rs = [] >> + files = {} >> + >> + while True: >> + for vm in kvm_utils.env_get_all_vms(env): >> + if not files.has_key(vm): >> > > You should probably add "and not vm.is_dead()" to this condition. > Otherwise we'll get lots of "could not connect to serial socket" > messages for dead VMs. > Nice catch, thanks. > Style note: AFAIK in the Autotest project the form "if not vm in files" > is preferred. > > >> + try: >> + serial_socket = socket.socket(socket.AF_UNIX, >> + socket.SOCK_STREAM) >> + serial_socket.setblocking(False) >> + serial_socket.connect(vm.serial_file_name) >> + except: >> + logging.debug("Could not connect to serial socket for %s" % >> + vm.name) >> + continue >> + rs.append(serial_socket) >> + serial_dump_filename = os.path.join(test.debugdir, >> + "serial-%s" % vm.name) >> + files[vm] = [serial_socket, file(serial_dump_filename, "a+")] >> + >> + r, w, x = select.select(rs, [], [], 0.5) >> + for vm in files.keys(): >> + [s ,d] = files[vm] >> > > For consistency, please consider changing this list to a tuple, i.e. > s, d = files[vm] or (s, d) = files[vm]. > > Yes, tuple is better. >> + if s in r: >> + data = s.recv(16384) >> + if len(data) == 0: >> > > Style note: AFAIK the preferred form is "if not data". > > Sorry for the petty comments. Overall the patch looks good. > Thanks for the comments and please do not hesitate to point the defects. > >> + rs.remove(s) >> + files.pop(vm) >> + else: >> + d.write(data) >> + >> + if _serialdump_thread_termination_event.isSet(): >> + break >> + >> + >> diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample >> index 9f82ffb..169a69e 100644 >> --- a/client/tests/kvm/tests_base.cfg.sample >> +++ b/client/tests/kvm/tests_base.cfg.sample >> @@ -13,6 +13,7 @@ start_vm = yes >> kill_vm = no >> kill_vm_gracefully = yes >> kill_unresponsive_vms = yes >> +serial_mode = dump >> >> # Screendump specific stuff >> convert_ppm_files_to_png_on_error = yes >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > >