From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lucas Meneghel Rodrigues Subject: Re: [Autotest] [PATCH 8/9] KVM test: Create the background threads before calling process() Date: Thu, 6 May 2010 12:35:11 -0300 Message-ID: References: <20100426095656.26268.50549.stgit@localhost.localdomain> <20100426100426.26268.93216.stgit@localhost.localdomain> <4BD8222B.90404@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Jason Wang , autotest@test.kernel.org, kvm@vger.kernel.org To: Michael Goldish Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:64432 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758897Ab0EFPjR convert rfc822-to-8bit (ORCPT ); Thu, 6 May 2010 11:39:17 -0400 Received: by wyg36 with SMTP id 36so77143wyg.19 for ; Thu, 06 May 2010 08:39:15 -0700 (PDT) In-Reply-To: <4BD8222B.90404@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Apr 28, 2010 at 8:55 AM, Michael Goldish = wrote: > On 04/26/2010 01:04 PM, Jason Wang wrote: >> If the screendump and scrialdump threads are created after the >> process(), we may lose the progress tracking of guest shutting >> down. So this patch creates them before calling process() in preproc= ess. >> >> Signed-off-by: Jason Wang >> --- >> =A0client/tests/kvm/kvm_preprocessing.py | =A0 =A05 ++--- >> =A01 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/client/tests/kvm/kvm_preprocessing.py b/client/tests/kv= m/kvm_preprocessing.py >> index 50d0e35..73e835a 100644 >> --- a/client/tests/kvm/kvm_preprocessing.py >> +++ b/client/tests/kvm/kvm_preprocessing.py >> @@ -257,9 +257,6 @@ def preprocess(test, params, env): >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int(params.get("p= re_command_timeout", "600")), >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0params.get("pre_c= ommand_noncritical") =3D=3D "yes") >> >> - =A0 =A0# Preprocess all VMs and images >> - =A0 =A0process(test, params, env, preprocess_image, preprocess_vm) >> - >> =A0 =A0 =A0# Start the screendump thread >> =A0 =A0 =A0if params.get("take_regular_screendumps") =3D=3D "yes": >> =A0 =A0 =A0 =A0 =A0logging.debug("Starting screendump thread") >> @@ -278,6 +275,8 @@ def preprocess(test, params, env): >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0args=3D(test, params, env)) >> =A0 =A0 =A0 =A0 =A0_serialdump_thread.start() >> >> + =A0 =A0# Preprocess all VMs and images >> + =A0 =A0process(test, params, env, preprocess_image, preprocess_vm) >> >> >> =A0def postprocess(test, params, env): > > The initial shutdown procedure is carried out automatically by the > preprocessor in order to prepare the VMs for the current test, and is > not part of the test. =A0During the procedure VMs from a previous tes= t are > shut down and/or restarted. =A0I think it'll be confusing (or at leas= t > irrelevant) for the user to see a Fedora guest shutting down at the > beginning of a Windows test. =A0Also, it will be inconsistent with th= e > pre_*.ppm screendumps which are taken after the new VMs are started. Agreed. Besides, if we didn't specify kill_vm for that particular test, it means that we are not terribly interested on what happens with the VM if it has to be shutdown (worst case scenario is, the postprocessor will quit the VM). Of course, it is allways scary to loose information, but I can't think on why we'd need that info anyway (correct me if I am wrong). --=20 Lucas