----- "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
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
