Merge lp:~mwhudson/lava-dispatcher/wait-for-console-for-lava_test_shell-bug-1073899 into lp:lava-dispatcher

[Michael Hudson-Doyle]

The name of the branch is a fair description. Not yet tested, hopefully soon.

[lp:~mwhudson/lava-dispatcher/wait-for-console-for-lava_test_shell-bug-1073899]

435. By Michael Hudson-Doyle on 2012-11-06

oops, remember how upstart works

[Michael Hudson-Doyle (mwhudson)]

Hm, even with the fix, this doesn't quite work -- lava-test-runner starts when the auto-serial-console script *starts*, which is not when the prompt appears. I guess if we really want to delay starting the test runner until the shell is accepting commands, we should just wait for the prompt... but that seems like an admission of defeat.

[lp:~mwhudson/lava-dispatcher/wait-for-console-for-lava_test_shell-bug-1073899]

436. By Michael Hudson-Doyle on 2012-11-06

a different approach: call run("true") (and so wait for a prompt) before run("sync") in power_off()

[Michael Hudson-Doyle (mwhudson)]

Tried something else. Seems to work.

[Dave Pigott (dpigott)]

Makes sense. +1

review: Approve

[Andy Doan (doanac)]

Can you elaborate on why you moved away from the original upstart .conf changes? Also, do we need a timeout specified with the runner.run('true') command?

[Michael Hudson-Doyle (mwhudson)]

Andy Doan <email address hidden> writes:

> Can you elaborate on why you moved away from the original upstart
> .conf changes?

https://code.launchpad.net/~mwhudson/lava-dispatcher/wait-for-console-for-lava_test_shell-bug-1073899/+merge/132976/comments/286075

> Also, do we need a timeout specified with the runner.run('true') command?

Maybe. Not sure what a good timeout would be -- I guess 30 s or so
would be safe enough.

Cheers,
mwh

[Paul Sokolovsky (pfalcon)]

I have to say that we tried different approaches trying to find "good" solution to this issue with James, but we tried to add extra commands (halt, etc.) *after* sync. I already prepared to submit "sleep" patch, but tried this one, and it works as expected over bunch of run I did. Please merge!

review: Approve

[Andy Doan (doanac)]

Michael and I discussed this yesterday and basically our new power_off logic is turning into a mess. I'm testing a quick patch right now that restores some sanity to this by making sure lava-test-shell just calls sync before it exits. That then solves the issue for all targets.

[Paul Sokolovsky (pfalcon)]

Ok, this hangs in queue for too long, and based on last comment, should have been fixed in another way long ago. Unless comments follow, I'm going to close this as rejected tomorrow.

Unmerged revisions

436. By Michael Hudson-Doyle on 2012-11-06

a different approach: call run("true") (and so wait for a prompt) before run("sync") in power_off()

435. By Michael Hudson-Doyle on 2012-11-06

oops, remember how upstart works

434. By Michael Hudson-Doyle on 2012-11-05

do not start lava-test-shell until the root shell on the serial console has started

=== modified file 'lava_dispatcher/device/target.py'
--- lava_dispatcher/device/target.py	2012-10-29 21:03:29 +0000
+++ lava_dispatcher/device/target.py	2012-11-06 01:13:21 +0000
@@ -107,6 +107,9 @@
         runner = self._get_runner(proc)
         try:
             logging.info('attempting a filesystem sync before power_off')
+            # We call run('true') first to ensure that we have a shell prompt
+            # to send the 'sync' command to.
+            runner.run('true')
             runner.run('sync', timeout=20)
         except:
             logging.exception('calling sync failed')