On Mon, May 25, 2009 at 2:15 PM, Michael Goldish <mgoldish@xxxxxxxxxx> wrote:
>
> ----- "sudhir kumar" <smalikphy@xxxxxxxxx> wrote:
>
>> The patch looks sane to me. A very good thing that can be done for
>> remote_login() is to tune the tmeouts. I have seen especialy with
>> windows guests or sometimes when the machine is heavily loaded the
>> timeouts elapse and the test fails. When I increased the timeouts the
>> test did not fail. internal_timeout=0.5 is too less in my views and
>> even timeouts of 10 seconds prove insufficient sometimes. Do you too
>> have any such experience?
>
> Yes, and I have a patch to fix the problem, which depends on another
> patch that isn't ready yet...
>
> Comments:
>
> 1. internal_timeout has nothing to do with this -- it controls the time
> duration read_nonblocking() waits until it decides there's no more output
> to read and returns. 0.5 is high enough in my opinion. Increasing
> internal_timeout leads to more robust prompt recognition. Decreasing it
> makes all related functions return sooner and thus increases overall
> performance (slightly). I think even 0.1 is a reasonable value.
I noticed changing internal_timeout from 0.5 to 1.0 caused my test to
pass for windows guest.
>
> 2. My solution to the prompt timeout problem (which isn't a very common
> problem AFAIK) is not to make the timeouts configurable -- instead I use
> 2 timeouts everywhere: an "initial output" timeout, and a "further output"
> timeout. The first timeout (typically 10 sec) expires if the guest hasn't
> responded to the SSH login request. Then the second timeout (typically 30
> sec) expires if there's no additional output. I think this makes sense
> because it usually doesn't take very long to get a password prompt or an
> "Are you sure" prompt. It can take a while to get the things that follow
> (a shell prompt). If we got some initial output it's likely that the guest
> will provide more, so we can afford to wait 30 seconds. We can make the 2
> timeouts configurable, but even fixing them at 10 and 30 will probably work
> well enough.
That looks ok.
Though if we are going to use the two timeout values everywhere then i
do not think there is any harm in making it configurable. We can keep
two parameters in the config file(provided we are going to use only 2
timeouts with fixed values), give them some default value(in function
or even in config file), and let the user have little bit of control.
Think of the scenario when one wants to stress the system and hence a
test failure because of a timeout is never ever expected. In case
there are any complications in providing the config variables please
let me too know. Please post the patch. i would like to test it and
try to make the timeouts as config variables.
>
>> On Sun, May 24, 2009 at 9:16 PM, Michael Goldish <mgoldish@xxxxxxxxxx>
>> wrote:
>> > 1. Make the 'login:' regular expression stricter so it doesn't
>> match
>> > 'Last login: ...' messages.
>> > 2. Make the 'password:' regular expression stricter.
>> > 3. Handle 'Connection refused' messages.
>> >
>> > Signed-off-by: Michael Goldish <mgoldish@xxxxxxxxxx>
>> > ---
>> > client/tests/kvm_runtest_2/kvm_utils.py | 13 +++++++++----
>> > 1 files changed, 9 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/client/tests/kvm_runtest_2/kvm_utils.py
>> b/client/tests/kvm_runtest_2/kvm_utils.py
>> > index be8ad95..5736cf6 100644
>> > --- a/client/tests/kvm_runtest_2/kvm_utils.py
>> > +++ b/client/tests/kvm_runtest_2/kvm_utils.py
>> > @@ -413,7 +413,8 @@ def remote_login(command, password, prompt,
>> linesep="\n", timeout=10):
>> >
>> > while True:
>> > (match, text) = sub.read_until_last_line_matches(
>> > - ["[Aa]re you sure", "[Pp]assword:", "[Ll]ogin:",
>> "[Cc]onnection.*closed", prompt],
>> > + [r"[Aa]re you sure", r"[Pp]assword:\s*$",
>> r"^\s*[Ll]ogin:\s*$",
>> > + r"[Cc]onnection.*closed",
>> r"[Cc]onnection.*refused", prompt],
>> > timeout=timeout, internal_timeout=0.5)
>> > if match == 0: # "Are you sure you want to continue
>> connecting"
>> > kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
>> > @@ -437,11 +438,15 @@ def remote_login(command, password, prompt,
>> linesep="\n", timeout=10):
>> > kvm_log.debug("Got 'Connection closed'")
>> > sub.close()
>> > return None
>> > - elif match == 4: # prompt
>> > + elif match == 4: # "Connection refused"
>> > + kvm_log.debug("Got 'Connection refused'")
>> > + sub.close()
>> > + return None
>> > + elif match == 5: # prompt
>> > kvm_log.debug("Got shell prompt -- logged in")
>> > return sub
>> > else: # match == None
>> > - kvm_log.debug("Timeout or process terminated")
>> > + kvm_log.debug("Timeout elapsed or process terminated")
>> > sub.close()
>> > return None
>> >
>> > @@ -470,7 +475,7 @@ def remote_scp(command, password, timeout=300,
>> login_timeout=10):
>> >
>> > while True:
>> > (match, text) = sub.read_until_last_line_matches(
>> > - ["[Aa]re you sure", "[Pp]assword:", "lost
>> connection"],
>> > + [r"[Aa]re you sure", r"[Pp]assword:\s*$", r"lost
>> connection"],
>> > timeout=_timeout, internal_timeout=0.5)
>> > if match == 0: # "Are you sure you want to continue
>> connecting"
>> > kvm_log.debug("Got 'Are you sure...'; sending 'yes'")
>> > --
>> > 1.5.4.1
>> >
>> > --
>> > 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
>> >
>>
>>
>>
>> --
>> Sudhir Kumar
>> --
>> 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
>
--
Sudhir Kumar
--
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
