From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lucas Meneghel Rodrigues <lmr@redhat.com>
Subject: Re: [PATCH][KVM-AUTOTEST] Make code to remove kvm modules more
 robust
Date: Tue, 26 May 2009 01:43:15 -0300
Message-ID: <1243312995.2683.7.camel@localhost.localdomain>
References: <1242849851-8029-1-git-send-email-mburns@redhat.com>
	 <1243258893.2838.18.camel@localhost.localdomain>
	 <4A1AC9F2.9030000@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Cc: Mike Burns <mburns@redhat.com>, kvm@vger.kernel.org,
	ulublin@redhat.com, dhuff@redhat.com
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:46392 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750965AbZEZEnR (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 26 May 2009 00:43:17 -0400
Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
	by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n4Q4hJZs006239
	for <kvm@vger.kernel.org>; Tue, 26 May 2009 00:43:19 -0400
In-Reply-To: <4A1AC9F2.9030000@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, 2009-05-25 at 19:40 +0300, Avi Kivity wrote:
> Lucas Meneghel Rodrigues wrote:
> > Hi Mike,
> >
> > On Wed, 2009-05-20 at 16:04 -0400, Mike Burns wrote:
> >   
> >> +
> >> +def __unload_modules(module):
> >> +  lsmod = os.popen("lsmod | grep \"^%s \"" % module)
> >> +  line_parts = lsmod.readline().split()
> >> +  if len(line_parts) == 0:
> >> +    kvm_log.info("%s not loaded." % module)
> >> +  else:
> >> +    kvm_log.info("Found module %s, checking for dependecies..." %module)
> >> +    if len(line_parts) == 4:
> >> +      submodules = line_parts[3].split(",")
> >> +      for submodule in submodules:
> >> +        __unload_modules(submodule)
> >> +    else:
> >> +      kvm_log.info("No modules dependent on %s" % module )
> >> +  
> >> +    kvm_log.info("Removing module: %s" % module)
> >> +    utils.system("/sbin/modprobe -r %s" % module, ignore_status=False)
> >>     
> >
> > ignore_status is False by default, so no need to do explicitly pass it
> > to the function. Also, as we are putting something that can throw an
> > exception, you can wrap it in a try/except module that throws a
> > error.TestError exception in case our module removal still fails.
> >   
> 
> Wrapping every function with try/except defeats the purpose of 
> exceptions.  Can't autotest handle arbitrary exceptions?

Yes it can, the point is that a failure trying to unload the module
leads to a test failure, so the only idea here is try to mark the test
as failed (with error.TestFail exception).

The CmdError will mark the test as failed anyway, so my suggestion of
wrapping the utils.system call with a try/except block is not necessary,
Mike can just remove the ignore_status assignment and it's done.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies