On 05/07/2010 01:26 PM, Feng Yang wrote:
> 1. In remote_scp(), if SCP connetion stalled for some reason, following
> code will be ran.
> else: # match == None
>
> logging.debug("Timeout elapsed or process terminated")
> status = sub.get_status()
> sub.close()
> return status == 0
> At this moment, kvm_subprocess server is still running which means
> lock_server_running_filename is still locked. But sub.get_status()
> tries to lock it again. If kvm_subprocess server keeps running,
> a deadlock will happen. This patch will fix this issue by enable
Agreed. It's a mistake (my mistake) to call get_status() on a process
that's still running and isn't expected to terminate soon. I think even
the docstring of get_status() says that it blocks, so that's expected
behavior.
However, there's a simple solution to that, and I don't see why an
additional timeout is necessary.
> timeout parameter. Update default value for timeout to 600, it should
> be enough.
>
> 2. Add "-v" in scp command to catch more infomation. Also add "Exit status"
> and "stalled" match prompt in remote_scp().
> Signed-off-by: Feng Yang <fyang@xxxxxxxxxx>
> ---
> client/tests/kvm/kvm_utils.py | 36 ++++++++++++++++++++++++++++--------
> client/tests/kvm/kvm_vm.py | 4 ++--
> 2 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
> index 25f3c8c..3db4dec 100644
> --- a/client/tests/kvm/kvm_utils.py
> +++ b/client/tests/kvm/kvm_utils.py
> @@ -524,7 +524,7 @@ def remote_login(command, password, prompt, linesep="\n", timeout=10):
> return None
>
>
> -def remote_scp(command, password, timeout=300, login_timeout=10):
> +def remote_scp(command, password, timeout=600, login_timeout=10):
> """
> Run the given command using kvm_spawn and provide answers to the questions
> asked. If timeout expires while waiting for the transfer to complete ,
> @@ -548,12 +548,18 @@ def remote_scp(command, password, timeout=300, login_timeout=10):
>
> password_prompt_count = 0
> _timeout = login_timeout
> + end_time = time.time() + timeout
> + logging.debug("Trying to SCP...")
>
> - logging.debug("Trying to login...")
>
> while True:
> + if end_time <= time.time():
> + logging.debug("transfer timeout!")
> + sub.close()
> + return False
> (match, text) = sub.read_until_last_line_matches(
> - [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"lost connection"],
> + [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"lost connection",
> + r"Exit status", r"stalled"],
> timeout=_timeout, internal_timeout=0.5)
> if match == 0: # "Are you sure you want to continue connecting"
> logging.debug("Got 'Are you sure...'; sending 'yes'")
> @@ -574,15 +580,29 @@ def remote_scp(command, password, timeout=300, login_timeout=10):
> logging.debug("Got 'lost connection'")
> sub.close()
> return False
> + elif match == 3: # "Exit status"
This check for "Exit status" is redundant. When the process terminates,
read_until_last_line_matches() will return None and get_status() will
return the exit status.
> + sub.close()
> + if "Exit status 0" in text:
> + logging.debug("SCP command completed successfully")
> + return True
> + else:
> + logging.debug("SCP command fail with exit status %s" % text)
> + return False
> + elif match == 4: # "stalled"
> + logging.debug("SCP connection stalled for some reason")
> + continue
> +
> else: # match == None
> - logging.debug("Timeout elapsed or process terminated")
> + if sub.is_alive():
> + continue
> + logging.debug("Process terminated for some reason")
> status = sub.get_status()
> sub.close()
> return status == 0
To avoid a deadlock, we can simply check if the process is still alive
before calling get_status(), i.e.
else: # match == None
if sub.is_alive():
logging.debug("Timeout elapsed")
sub.close()
return False
else:
status = sub.get_status()
sub.close()
logging.debug("Process exited with status %d", status)
return status == 0
This looks like the simplest solution to me.
>
>
> def scp_to_remote(host, port, username, password, local_path, remote_path,
> - timeout=300):
> + timeout=600):
> """
> Copy files to a remote host (guest).
>
> @@ -596,14 +616,14 @@ def scp_to_remote(host, port, username, password, local_path, remote_path,
>
> @return: True on success and False on failure.
> """
> - command = ("scp -o UserKnownHostsFile=/dev/null "
> + command = ("scp -v -o UserKnownHostsFile=/dev/null "
> "-o PreferredAuthentications=password -r -P %s %s %s@%s:%s" %
> (port, local_path, username, host, remote_path))
> return remote_scp(command, password, timeout)
>
>
> def scp_from_remote(host, port, username, password, remote_path, local_path,
> - timeout=300):
> + timeout=600):
> """
> Copy files from a remote host (guest).
>
> @@ -617,7 +637,7 @@ def scp_from_remote(host, port, username, password, remote_path, local_path,
>
> @return: True on success and False on failure.
> """
> - command = ("scp -o UserKnownHostsFile=/dev/null "
> + command = ("scp -v -o UserKnownHostsFile=/dev/null "
> "-o PreferredAuthentications=password -r -P %s %s@%s:%s %s" %
> (port, username, host, remote_path, local_path))
> return remote_scp(command, password, timeout)
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 6bc7987..d1e0246 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -808,7 +808,7 @@ class VM:
> return session
>
>
> - def copy_files_to(self, local_path, remote_path, nic_index=0, timeout=300):
> + def copy_files_to(self, local_path, remote_path, nic_index=0, timeout=600):
> """
> Transfer files to the guest.
>
> @@ -833,7 +833,7 @@ class VM:
> local_path, remote_path, timeout)
>
>
> - def copy_files_from(self, remote_path, local_path, nic_index=0, timeout=300):
> + def copy_files_from(self, remote_path, local_path, nic_index=0, timeout=600):
> """
> Transfer files from the guest.
>
--
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
