[PATCH] board.linux: make linux login more robust

[Harald Seiler]

Hello Heiko,

thanks a lot for the patch!

On Mon, 2020-03-30 at 07:56 +0200, Heiko Schocher wrote:
> There is already the login_delay parameter, which
> helps to make login more robust if console is
> flooded on boot up time.
> 
> tbot waits for the string "login: " and expects that
> there are no more characters after it. But in case the
> console get flooded with characters on boot up time,
> it can happen that there are characters in the same line
> after "login: ".
> 
> Currently, read_until_prompt waits endless, when trying
> to get "login: " and so tbot never login into linux.
> 
> rework this part, so read_until_prompt waits "login_delay"
> seconds, and after that, trigger a new login prompt.
> 
> Signed-off-by: Heiko Schocher <hs at denx.de>
> ---

This change makes a lot of sense; the current implementation only catches the
prompt robustly if `login_prompt` is a regex pattern ending in `{0,256}` (that
is, accounts for trailing characters).

I am, however, a bit hesitant about applying this patch directly because it
changes the meaning of `login_delay` quite a bit.  The original intention when
this parameter was added in commit 052038a9c30f ("board.linux: Provide
login_delay parameter") was a total time to wait after which no more flooding
would happen from any source.

With this patch, `login_delay` is now the time after which a login attempt is
retried.  This could mean that we login in way before the delay is over and
catch more flooding later on.  

Maybe Lukasz, who originally added the parameter, can comment on this?

Additionally, with this change tbot periodically sends a "\r" during boot,
every `login_delay` seconds.  This shouldn't be a problem though as far as
I can tell, but of course it would be nicer to avoid it.

---

Unless this is a critical issue for someone, I'd apply the patch for now and
rethink this later.  I've got a few channel helpers in the pipeline which will
make these kind of things a lot easier and I'd revisit the topic once those are
in ...

>  tbot/machine/board/linux.py | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/tbot/machine/board/linux.py b/tbot/machine/board/linux.py
> index 1e76b28..942cee5 100644
> --- a/tbot/machine/board/linux.py
> +++ b/tbot/machine/board/linux.py
> @@ -105,17 +105,18 @@ class LinuxBootLogin(machine.Initializer, LinuxBoot):
>              ev = cx.enter_context(self._linux_boot_event())
>              cx.enter_context(self.ch.with_stream(ev))
>  
> -            self.ch.read_until_prompt(prompt=self.login_prompt)
> -
>              # On purpose do not login immediately as we may get some
>              # console flooding from upper SW layers (and tbot's console
>              # setup may get broken)
>              if self.login_delay != 0:
> -                try:
> -                    self.ch.read(timeout=self.login_delay)
> -                except TimeoutError:
> -                    pass
> -                self.ch.sendline("")
> +                while 1:

Please use `while True:`

> +                    try:
> +                        self.ch.read_until_prompt(prompt=self.login_prompt, timeout=self.login_delay)
> +                        break
> +                    except TimeoutError:
> +                        self.ch.sendline("")
> +                        pass

You don't need this `pass` statement here.

> +            else:
>                  self.ch.read_until_prompt(prompt=self.login_prompt)
>  
>              self.ch.sendline(self.username)

-- 
Harald