* [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py
2009-12-30 9:47 [AUTOTEST PATCH 1/2] KVM-test: Fix a bug that about list slice in scan_results.py Yolkfull Chow
@ 2009-12-30 9:47 ` Yolkfull Chow
0 siblings, 0 replies; 5+ messages in thread
From: Yolkfull Chow @ 2009-12-30 9:47 UTC (permalink / raw)
To: autotest; +Cc: kvm
If vm.remote_login failed 'session.close()' can result in exception
in finally clause. This patch fix the problem.
Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
---
client/tests/kvm/kvm_vm.py | 51 +++++++++++++++++++++----------------------
1 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 7229b79..f746331 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -827,40 +827,39 @@ class VM:
"""
Get the cpu count of the VM.
"""
+ session = self.remote_login()
+ cmd = self.params.get("cpu_chk_cmd")
try:
- session = self.remote_login()
- if session:
- cmd = self.params.get("cpu_chk_cmd")
- s, count = session.get_command_status_output(cmd)
- if s == 0:
- return int(count)
+ s, count = session.get_command_status_output(cmd)
+ if s == 0:
+ return int(count)
return None
finally:
- session.close()
+ if session:
+ session.close()
def get_memory_size(self):
"""
Get memory size of the VM.
"""
+ session = self.remote_login()
+ cmd = self.params.get("mem_chk_cmd")
try:
- session = self.remote_login()
- if session:
- cmd = self.params.get("mem_chk_cmd")
- s, mem_str = session.get_command_status_output(cmd)
- if s != 0:
- return None
- mem = re.findall("([0-9][0-9][0-9]+)", mem_str)
- mem_size = 0
- for m in mem:
- mem_size += int(m)
- if "GB" in mem_str:
- mem_size *= 1024
- elif "MB" in mem_str:
- pass
- else:
- mem_size /= 1024
- return int(mem_size)
- return None
+ s, mem_str = session.get_command_status_output(cmd)
+ if s != 0:
+ return None
+ mem = re.findall("([0-9][0-9][0-9]+)", mem_str)
+ mem_size = 0
+ for m in mem:
+ mem_size += int(m)
+ if "GB" in mem_str:
+ mem_size *= 1024
+ elif "MB" in mem_str:
+ pass
+ else:
+ mem_size /= 1024
+ return int(mem_size)
finally:
- session.close()
+ if session:
+ session.close()
--
1.6.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py
[not found] <278734081.2246011262253625627.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
@ 2009-12-31 10:03 ` Michael Goldish
2010-01-12 22:37 ` [Autotest] " Lucas Meneghel Rodrigues
2010-01-13 2:22 ` Yolkfull Chow
0 siblings, 2 replies; 5+ messages in thread
From: Michael Goldish @ 2009-12-31 10:03 UTC (permalink / raw)
To: Yolkfull Chow; +Cc: kvm, autotest
----- "Yolkfull Chow" <yzhou@redhat.com> wrote:
> If vm.remote_login failed 'session.close()' can result in exception
> in finally clause. This patch fix the problem.
>
> Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> ---
> client/tests/kvm/kvm_vm.py | 51
> +++++++++++++++++++++----------------------
> 1 files changed, 25 insertions(+), 26 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 7229b79..f746331 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -827,40 +827,39 @@ class VM:
> """
> Get the cpu count of the VM.
> """
> + session = self.remote_login()
> + cmd = self.params.get("cpu_chk_cmd")
> try:
> - session = self.remote_login()
> - if session:
> - cmd = self.params.get("cpu_chk_cmd")
> - s, count = session.get_command_status_output(cmd)
> - if s == 0:
> - return int(count)
> + s, count = session.get_command_status_output(cmd)
> + if s == 0:
> + return int(count)
> return None
> finally:
> - session.close()
> + if session:
> + session.close()
If self.remote_login() fails, session will be None, and then the attempted
call to session.get_command_status_output() will raise an exception which
will fail the test with a rather ugly "reason" string.
I think this form is preferable:
session = self.remote_login()
if not session:
return None
try:
cmd = ...
s, count = ...
if s == 0:
return int(count)
return None
finally:
session.close()
There's no need for a second 'if session' in the finally block.
> def get_memory_size(self):
> """
> Get memory size of the VM.
> """
> + session = self.remote_login()
> + cmd = self.params.get("mem_chk_cmd")
> try:
> - session = self.remote_login()
> - if session:
> - cmd = self.params.get("mem_chk_cmd")
> - s, mem_str = session.get_command_status_output(cmd)
> - if s != 0:
> - return None
> - mem = re.findall("([0-9][0-9][0-9]+)", mem_str)
> - mem_size = 0
> - for m in mem:
> - mem_size += int(m)
> - if "GB" in mem_str:
> - mem_size *= 1024
> - elif "MB" in mem_str:
> - pass
> - else:
> - mem_size /= 1024
> - return int(mem_size)
> - return None
> + s, mem_str = session.get_command_status_output(cmd)
> + if s != 0:
> + return None
> + mem = re.findall("([0-9][0-9][0-9]+)", mem_str)
> + mem_size = 0
> + for m in mem:
> + mem_size += int(m)
> + if "GB" in mem_str:
> + mem_size *= 1024
> + elif "MB" in mem_str:
> + pass
> + else:
> + mem_size /= 1024
> + return int(mem_size)
> finally:
> - session.close()
> + if session:
> + session.close()
The same applies to this function.
> --
> 1.6.6
>
> --
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Autotest] [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py
2009-12-31 10:03 ` [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py Michael Goldish
@ 2010-01-12 22:37 ` Lucas Meneghel Rodrigues
2010-01-13 2:24 ` Yolkfull Chow
2010-01-13 2:22 ` Yolkfull Chow
1 sibling, 1 reply; 5+ messages in thread
From: Lucas Meneghel Rodrigues @ 2010-01-12 22:37 UTC (permalink / raw)
To: Michael Goldish; +Cc: Yolkfull Chow, autotest, kvm
On Thu, Dec 31, 2009 at 8:03 AM, Michael Goldish <mgoldish@redhat.com> wrote:
>
> ----- "Yolkfull Chow" <yzhou@redhat.com> wrote:
>
>> If vm.remote_login failed 'session.close()' can result in exception
>> in finally clause. This patch fix the problem.
Hi Yolkfull, please implement Michael's comments and re-submit.
Cheers,
--
Lucas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py
2009-12-31 10:03 ` [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py Michael Goldish
2010-01-12 22:37 ` [Autotest] " Lucas Meneghel Rodrigues
@ 2010-01-13 2:22 ` Yolkfull Chow
1 sibling, 0 replies; 5+ messages in thread
From: Yolkfull Chow @ 2010-01-13 2:22 UTC (permalink / raw)
To: Michael Goldish; +Cc: autotest, kvm
On Thu, Dec 31, 2009 at 05:03:21AM -0500, Michael Goldish wrote:
>
> ----- "Yolkfull Chow" <yzhou@redhat.com> wrote:
>
> > If vm.remote_login failed 'session.close()' can result in exception
> > in finally clause. This patch fix the problem.
> >
> > Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> > ---
> > client/tests/kvm/kvm_vm.py | 51
> > +++++++++++++++++++++----------------------
> > 1 files changed, 25 insertions(+), 26 deletions(-)
> >
> > diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> > index 7229b79..f746331 100755
> > --- a/client/tests/kvm/kvm_vm.py
> > +++ b/client/tests/kvm/kvm_vm.py
> > @@ -827,40 +827,39 @@ class VM:
> > """
> > Get the cpu count of the VM.
> > """
> > + session = self.remote_login()
> > + cmd = self.params.get("cpu_chk_cmd")
> > try:
> > - session = self.remote_login()
> > - if session:
> > - cmd = self.params.get("cpu_chk_cmd")
> > - s, count = session.get_command_status_output(cmd)
> > - if s == 0:
> > - return int(count)
> > + s, count = session.get_command_status_output(cmd)
> > + if s == 0:
> > + return int(count)
> > return None
> > finally:
> > - session.close()
> > + if session:
> > + session.close()
>
> If self.remote_login() fails, session will be None, and then the attempted
> call to session.get_command_status_output() will raise an exception which
> will fail the test with a rather ugly "reason" string.
>
> I think this form is preferable:
>
> session = self.remote_login()
> if not session:
> return None
Reasonable, thanks Michael. :)
Will post the updated patch soon.
> try:
> cmd = ...
> s, count = ...
> if s == 0:
> return int(count)
> return None
> finally:
> session.close()
>
> There's no need for a second 'if session' in the finally block.
>
> > def get_memory_size(self):
> > """
> > Get memory size of the VM.
> > """
> > + session = self.remote_login()
> > + cmd = self.params.get("mem_chk_cmd")
> > try:
> > - session = self.remote_login()
> > - if session:
> > - cmd = self.params.get("mem_chk_cmd")
> > - s, mem_str = session.get_command_status_output(cmd)
> > - if s != 0:
> > - return None
> > - mem = re.findall("([0-9][0-9][0-9]+)", mem_str)
> > - mem_size = 0
> > - for m in mem:
> > - mem_size += int(m)
> > - if "GB" in mem_str:
> > - mem_size *= 1024
> > - elif "MB" in mem_str:
> > - pass
> > - else:
> > - mem_size /= 1024
> > - return int(mem_size)
> > - return None
> > + s, mem_str = session.get_command_status_output(cmd)
> > + if s != 0:
> > + return None
> > + mem = re.findall("([0-9][0-9][0-9]+)", mem_str)
> > + mem_size = 0
> > + for m in mem:
> > + mem_size += int(m)
> > + if "GB" in mem_str:
> > + mem_size *= 1024
> > + elif "MB" in mem_str:
> > + pass
> > + else:
> > + mem_size /= 1024
> > + return int(mem_size)
> > finally:
> > - session.close()
> > + if session:
> > + session.close()
>
> The same applies to this function.
>
> > --
> > 1.6.6
> >
> > --
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py
2010-01-12 22:37 ` [Autotest] " Lucas Meneghel Rodrigues
@ 2010-01-13 2:24 ` Yolkfull Chow
0 siblings, 0 replies; 5+ messages in thread
From: Yolkfull Chow @ 2010-01-13 2:24 UTC (permalink / raw)
To: Lucas Meneghel Rodrigues; +Cc: autotest, kvm
On Tue, Jan 12, 2010 at 08:37:25PM -0200, Lucas Meneghel Rodrigues wrote:
> On Thu, Dec 31, 2009 at 8:03 AM, Michael Goldish <mgoldish@redhat.com> wrote:
> >
> > ----- "Yolkfull Chow" <yzhou@redhat.com> wrote:
> >
> >> If vm.remote_login failed 'session.close()' can result in exception
> >> in finally clause. This patch fix the problem.
>
> Hi Yolkfull, please implement Michael's comments and re-submit.
Sorry, hadn't seen the reply from Michael. I will post the updated patch soon.
Thanks, Lucas. :)
>
> Cheers,
>
> --
> Lucas
> --
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-01-13 2:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <278734081.2246011262253625627.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com>
2009-12-31 10:03 ` [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py Michael Goldish
2010-01-12 22:37 ` [Autotest] " Lucas Meneghel Rodrigues
2010-01-13 2:24 ` Yolkfull Chow
2010-01-13 2:22 ` Yolkfull Chow
2009-12-30 9:47 [AUTOTEST PATCH 1/2] KVM-test: Fix a bug that about list slice in scan_results.py Yolkfull Chow
2009-12-30 9:47 ` [AUTOTEST PATCH 2/2] KVM-test: Move two 'remote_login' out of try block in kvm_vm.py Yolkfull Chow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox