New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 657109 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 586542



Sign in to add a comment

dut-control power_state:rec with EC console opens fails

Project Member Reported by aaboagye@chromium.org, Oct 18 2016

Issue description

If a user has the EC console open and then uses dut-control to set power_state:rec, it fails with an error complaining about resource temporarily unavailable.

This is because in order for the power state driver to reliably enter recovery mode, it is now checking to see that the commands it sent actually took effect. (Previously there was no checking whatsoever) Julius had the suggestion of freezing the processes instead of requiring the user to close their terminals.

This might work, but I think it'd require launching servod within a chroot entered with --no-ns-pid.

 
> This might work, but I think it'd require launching servod within a chroot entered with --no-ns-pid.

I'd say that's fine. You can find a long and fruitless argument between Mike and me about PID namespaces in the chroot in  issue 444931 . My personal position is that namespacing PIDs without namespacing /dev makes no sense and breaks the ability of processes to interact in the way they need to to arbitrate shared device resources. I couldn't convince Mike to get rid of it, but I think he essentially said that we are free to disable it whenever it doesn't jive with our requirements. I've since added code to force everyone using flash_ec to do so and haven't heard any complaints.

Alternatively, since this is in servod which also creates the PTYs in the first place, you would have other options if you really care about PID namespaces. For example, you could make servod export another pty (ec_uart_service_pty or some-such) and add a dut-control option to switch which of the two PTYs is currently routed through to the UART. Then everything that needs to programmatically send/receive EC commands could turn that option and access the other PTY, while minicom on the main PTY just hangs and buffers any input. I just didn't do that back then because I was lazy and it seemed complicated, but it should work well in the end.
Blockedon: 586542
Labels: -Pri-3 Pri-2
Status: Assigned (was: Untriaged)
I do like the second option and it fits more within the "spirit" of EC-3PO. One of the plans that I had for EC-3PO is to have separate command channels for various commands (such as the autotest framework that might query things through the EC console). The power state driver would be a great fit for that as well.

However, it will be quite sometime until that's all ready. We could pursue the process freezing option as a sort of stopgap if this is truly a great annoyance. Although, I'm planning to resume development on EC-3PO now.
Cc: aaboagye@chromium.org kevcheng@chromium.org nsanders@chromium.org
 Issue 628766  has been merged into this issue.
I'd like a stopgap, or a temporary revert of whatever broke my use case (not sure how feasible that is... did that resolve a real problem somewhere or did you just add it "just in case"?). I think most people keep their EC consoles up all the time and breaking power_state:rec for all of them for however long this may take is pretty annoying.
Status: Started (was: Assigned)
It did solve real issues of devices not reliably entering recovery mode. But sure, I'll pursue the process freezing as a stopgap.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/6e21d3f52240c01959ec6d5209e8fbcbd1f3158a

commit 6e21d3f52240c01959ec6d5209e8fbcbd1f3158a
Author: Aseda Aboagye <aaboagye@google.com>
Date: Fri Oct 21 18:37:56 2016

fwgdb: Move Terminal Freezer to hdctools.

This commit removes the terminal freezer code which has been relocated
to the hdctools repository.

BUG= chromium:657109 
BRANCH=None
TEST=With other pending changes, run `dut-control power_state:rec` and
verify that no extra prints are shown during the normal invocation of
servod.

CQ-DEPEND=CL:401529

Change-Id: I63b4d80b733f29c39d510ea6cc6e16b3d1e91197
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/401530
Commit-Ready: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/6e21d3f52240c01959ec6d5209e8fbcbd1f3158a/scripts/fwgdb.py

Status: Verified (was: Started)
Alright, the CLs have landed and I verified locally when uploading the CLs that the original issue is now fixed. From now on once you grab the latest hdctools, you shouldn't have to close the EC console when servod is mucking around with the PTYs. Whenever I get to the point where separate channels are in place for EC-3PO, I'll migrate this to that format.

Lastly, I made a typo in my other CL, so I'm posting the description here.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/7e53a2538846f7ca7f03656f570d00f83ea8bcea

commit 7e53a2538846f7ca7f03656f570d00f83ea8bcea
Author: Aseda Aboagye <aaboagye@google.com>
Date: Fri Oct 21 18:40:46 2016

pty_driver: Freeze terminals when using regexp.

The pty driver has the ability to have regular expressions to check the
output after running some command.  However, if a user has a serial
terminal open, the command will fail because the serial console output
will go to the user's terminal instead of being caught by the pty_driver
functions.

This commit attempts to freeze any terminals that are in use when a
regexp is set.  The TerminalFreezer class has been moved here from it's
former home in chromite.

BUG= chromium:647109 
BRANCH=None
TEST=Open minicom for the EC console on kevin and cyan.  Verify that
`dut-control power_state:rec` succeeds without having to close minicom.
TEST=Verify servod starts successfully on a servo v3.

Change-Id: I2a4ee4e47cc5cf735f98b770a319ab04cab5f580
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/401529
Commit-Ready: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/7e53a2538846f7ca7f03656f570d00f83ea8bcea/servo/terminal_freezer.py
[modify] https://crrev.com/7e53a2538846f7ca7f03656f570d00f83ea8bcea/servo/drv/pty_driver.py
[modify] https://crrev.com/7e53a2538846f7ca7f03656f570d00f83ea8bcea/servo/servod.py

Sign in to add a comment