From mboxrd@z Thu Jan  1 00:00:00 1970
From: Amos Kong <akong@redhat.com>
Subject: Re: [Autotest PATCH] KVM test: No need close session
	when login	timeout
Date: Sat, 26 Dec 2009 11:56:45 +0800
Message-ID: <20091226035645.GD3258@debian>
References: <843369727.2189451261747394285.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
	<1816815221.2189471261747698769.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
Reply-To: Amos Kong <akong@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: autotest@test.kernel.org, kvm@vger.kernel.org
To: Michael Goldish <mgoldish@redhat.com>
Return-path: <autotest-bounces@test.kernel.org>
Content-Disposition: inline
In-Reply-To: <1816815221.2189471261747698769.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
List-Unsubscribe: <http://test.kernel.org/cgi-bin/mailman/listinfo/autotest>,
	<mailto:autotest-request@test.kernel.org?subject=unsubscribe>
List-Archive: <http://test.kernel.org/pipermail/autotest>
List-Post: <mailto:autotest@test.kernel.org>
List-Help: <mailto:autotest-request@test.kernel.org?subject=help>
List-Subscribe: <http://test.kernel.org/cgi-bin/mailman/listinfo/autotest>,
	<mailto:autotest-request@test.kernel.org?subject=subscribe>
Sender: autotest-bounces@test.kernel.org
Errors-To: autotest-bounces@test.kernel.org
List-Id: kvm.vger.kernel.org

On Fri, Dec 25, 2009 at 08:28:18AM -0500, Michael Goldish wrote:
> 
> ----- "Amos Kong" <akong@redhat.com> wrote:
> 
> > If login timeout, wait_for() returned 'None' and assigned to
> > 'session'.
> > When call session.close(), this prlblem was caused:
> > "AttributeError: 'NoneType' object has no attribute 'close'"
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  client/tests/kvm/tests/timedrift_with_migration.py |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/client/tests/kvm/tests/timedrift_with_migration.py
> > b/client/tests/kvm/tests/timedrift_with_migration.py
> > index a012db3..0b93183 100644
> > --- a/client/tests/kvm/tests/timedrift_with_migration.py
> > +++ b/client/tests/kvm/tests/timedrift_with_migration.py
> > @@ -76,7 +76,8 @@ def run_timedrift_with_migration(test, params,
> > env):
> >                                               time_filter_re,
> > time_format)
> >  
> >      finally:
> > -        session.close()
> > +        if session != None:
> > +            session.close()
> 
> Agreed, but we can make this simply:
> 
> if session:
>     session.close()

Yes,

 
> There's no need to explicitly check for None (and if there was,
> the preferred syntax would be 'is not None' rather than '!= None').
> 
> Also, just to be safe, we should make the same modification to
> timedrift_with_reboot.py.


In timedrift_with_reboot.py, 'session' has been assigned before 'try'. 
If re-login timout, kvm_test_utils.reboot() returns nothing, the value of 'session' isn't changed.
So session.close() couldn't cause this problem:
"AttributeError: 'NoneType' object has no attribute 'close'"



In other testcases, if session wasn't assigned before 'try',
when calling kvm_test_utils.wait_for_login()/kvm_test_utils.reboot() timeout,
It returns nothing, if close 'session' in finally part, Another problem will occur:
"NameError: name 'session' is not defined"

In this condition,
""" 
if session:
    session.close()
"""
also causes this error.



> We can also consider removing the try..finally clauses altogether
> because sessions are now closed automatically when they're no longer
> needed.
> 
> >  
> >      # Report results
> >      host_delta = ht1 - ht0
> > -- 
> > 1.5.5.6

-- 
Amos Kong
Quality Engineer
Raycom Office(Beijing), Red Hat Inc.