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

Issue 784651 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task



Sign in to add a comment

powerd should read a directory for lockfiles

Project Member Reported by derat@chromium.org, Nov 14 2017

Issue description

powerd currently has a hardcoded list of lockfiles (generally written by firmware-updating tools) to check before it tries to suspend or shut down the system. It'd be cleaner to just check a directory for files; then powerd won't need to be updated whenever another updater is added.

(from discussion on https://crrev.com/c/756333)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a13054c6d9eebcbd874bcd2451163a9c9d032e3e

commit a13054c6d9eebcbd874bcd2451163a9c9d032e3e
Author: Daniel Erat <derat@chromium.org>
Date: Sat Nov 25 04:31:22 2017

power: Check for lockfiles in /run/lock/power_override.

Introduce a new system::LockfileChecker class within powerd
that checks for the existence of lockfiles that should
prevent powerd from suspending or shutting down the system.

This moves some more code out of the Daemon class and also
adds support for a new /run/lock/power_override directory
that can be used to add additional lockfiles in the future
without needing to update a list within powerd.

BUG= chromium:784651 ,b:35550315
TEST=added tests; also manually tested by writing powerd's
     PID to a file under /run/lock/power_override and
     verifying that powerd holds off on suspending until the
     lockfile is removed

Change-Id: Ie4f3642350f01cd57bd6b56cabd53ec775da17fa
Reviewed-on: https://chromium-review.googlesource.com/768311
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/init/shared/powerd-pre-start.sh
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/common/util.cc
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/main.cc
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/power_manager.gyp
[add] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/system/lockfile_checker.h
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/init/systemd/powerd_directories.conf
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/docs/faq.md
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/daemon_delegate.h
[add] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/system/lockfile_checker_stub.h
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/daemon.cc
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/common/util_unittest.cc
[add] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/system/lockfile_checker_unittest.cc
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/daemon.h
[add] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/system/lockfile_checker_stub.cc
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/common/util.h
[modify] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/daemon_unittest.cc
[add] https://crrev.com/a13054c6d9eebcbd874bcd2451163a9c9d032e3e/power_manager/powerd/system/lockfile_checker.cc

Comment 2 by derat@chromium.org, Nov 25 2017

I'll keep this open to track updating existing programs to write to the directory.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/131afcd81abb60720aaaa39bbeb7861373da8d0a

commit 131afcd81abb60720aaaa39bbeb7861373da8d0a
Author: Daniel Erat <derat@chromium.org>
Date: Thu Dec 07 04:24:12 2017

ec: Create lockfile in /run/lock/power_override.

Create a lockfile at
/run/lock/power_override/battery_tool.lock rather than
/run/lock/battery_tool_powerd.lock so that powerd doesn't
need to special-case the file's path.

BUG= chromium:784651 
BRANCH=None
TEST=None

Change-Id: I151cf26d635dc969d113e9d80c93177985a7ab2f
Signed-off-by: Daniel Erat <derat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/809921
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

[modify] https://crrev.com/131afcd81abb60720aaaa39bbeb7861373da8d0a/util/powerd_lock.c

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3b510d3800b083ba96ad8e6568d8ec22fe8bed58

commit 3b510d3800b083ba96ad8e6568d8ec22fe8bed58
Author: Daniel Erat <derat@chromium.org>
Date: Sat Feb 03 10:37:54 2018

init/power: Create power override dir in chromeos_startup.

Create /run/lock/power_override in the chromeos_startup
script rather than in the powerd job's pre-start stanza.
This directory should exist if flashrom runs early in the
boot process before powerd has started.

BUG= chromium:784651 
TEST=manual: verified that lockfiles can be created from
     within chromeos_startup

Change-Id: Icdc2983707c19480abbde020276c0e2b4ec2e958
Reviewed-on: https://chromium-review.googlesource.com/886831
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/3b510d3800b083ba96ad8e6568d8ec22fe8bed58/power_manager/init/shared/powerd-pre-start.sh
[modify] https://crrev.com/3b510d3800b083ba96ad8e6568d8ec22fe8bed58/init/chromeos_startup
[modify] https://crrev.com/3b510d3800b083ba96ad8e6568d8ec22fe8bed58/power_manager/init/systemd/powerd_directories.conf

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/4166d34d881200733da1ac8b04fb1d12f7b180de

commit 4166d34d881200733da1ac8b04fb1d12f7b180de
Author: Daniel Erat <derat@chromium.org>
Date: Sat Feb 03 10:37:54 2018

flashrom: Move lock file to /run/lock/power_override.

Make flashrom create a lockfile at
/run/lock/power_override/flashrom.lock rather than
/run/lock/flashrom_powerd.lock so that powerd doesn't need
to special-case the file's location.

BUG= chromium:784651 
BRANCH=None
CQ-DEPEND=Icdc2983707c19480abbde020276c0e2b4ec2e958
TEST=ran "flashrom -w <file>" and verified that lockfile was
     created at /run/lock/power_override/flashrom.lock, then
     moved the power_override dir away and checked that
     flashrom doesn't log an error

Change-Id: I5b85e13c3a71ab33a08add33c2b815d6bfd49208
Signed-off-by: Daniel Erat <derat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/809853
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/4166d34d881200733da1ac8b04fb1d12f7b180de/power.c

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/42fa285ca02fdd6acdae1de40b5e0d78d06d4cf0

commit 42fa285ca02fdd6acdae1de40b5e0d78d06d4cf0
Author: Daniel Erat <derat@chromium.org>
Date: Sun Feb 04 15:31:55 2018

autotest: Update path in power_DeferForFlashrom.

Make the power_DeferForFlashrom watch for the updated
/run/lock/power_override/flashrom.lock lockfile written by
flashrom.

BUG= chromium:784651 
TEST=none
TBR=hungte@chromium.org

Change-Id: I7eae253c5f894156b1b27d6c09f0821a0192c42b
Reviewed-on: https://chromium-review.googlesource.com/900553
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/42fa285ca02fdd6acdae1de40b5e0d78d06d4cf0/server/site_tests/power_DeferForFlashrom/power_DeferForFlashrom.py

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/d6fca5dd1937a187e6d71b350b6c0f760be7862d

commit d6fca5dd1937a187e6d71b350b6c0f760be7862d
Author: Daniel Erat <derat@chromium.org>
Date: Tue Feb 06 03:09:05 2018

flashrom: Retain support for /run/lock/flashrom_powerd.lock.

Make flashrom fall back to creating a lock file at
/run/lock/flashrom_powerd.lock if /run/lock/power_override
doesn't exist. This is needed to support the case during
system updates where a recent version of flashrom is running
on an old system image.

BUG= chromium:784651 
BRANCH=None
TEST=manual: "flashrom -w" creates a file in
     /run/lock/power_override if it exists or at
     /run/lock/flashrom_powerd.lock otherwise

Change-Id: Ia3bba0a01f40f5245c4b667de1ca638febe48acc
Signed-off-by: Daniel Erat <derat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/902169
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/d6fca5dd1937a187e6d71b350b6c0f760be7862d/power.c

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a7cfbed63756215cc555640d8ad6278d99025173

commit a7cfbed63756215cc555640d8ad6278d99025173
Author: Daniel Erat <derat@chromium.org>
Date: Tue Feb 06 03:09:04 2018

power: Stop checking for one-off lockfiles.

Make powerd stop checking for lockfiles at
/run/lock/flashrom_powerd.lock and
/run/lock/battery_tool_powerd.lock. These are created in the
/run/lock/power_override directory now.

BUG= chromium:784651 
TEST=unit tests pass; no remaining references to old files
     in codebase

Change-Id: Ia6492683eb279454e88971aa9832b7ea00117233
Reviewed-on: https://chromium-review.googlesource.com/900554
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a7cfbed63756215cc555640d8ad6278d99025173/power_manager/powerd/daemon.cc
[modify] https://crrev.com/a7cfbed63756215cc555640d8ad6278d99025173/power_manager/docs/suspend_resume.md

Comment 9 by derat@chromium.org, Feb 6 2018

Status: Fixed (was: Started)

Sign in to add a comment