On Thu, Dec 31, 2009 at 05:03:21AM -0500, Michael Goldish wrote:
>
> ----- "Yolkfull Chow" <yzhou@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
