New issue
Advanced search Search tips

Issue 807509 link

Starred by 1 user
Status: Fixed
Owner:
derat@chromium.org
Closed: Feb 2018
Cc:
tbroch@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature



Sign in to add a comment

powerd_setuid_helper should use initctl to go to runlevel 6 to reboot

Project Member Reported by derat@chromium.org, Jan 31 2018 
powerd_setuid_helper currently runs "initctl emit --no-wait runlevel RUNLEVEL=0 SHUTDOWN_REASON=foo" to shut the system down, but "shutdown -r now" to reboot it. I don't think there's any particular reason for the difference; it's just what it's always done.

This has the downside of leaving SHUTDOWN_REASON unset when rebooting, though, resulting in the pre-shutdown job logging a message like this:

  2018-01-30T18:17:24.715632-08:00 NOTICE pre-shutdown[7514]: Shutting down for reboot: unknown-reason

I think I can make powerd_setuid_helper instead run initctl to go to runlevel 6 in order to reboot. Then we can pass SHUTDOWN_REASON.
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 2 2018 

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

commit 195f346dffc7c2d5b306e2acfef5b95751e88328
Author: Daniel Erat <derat@chromium.org>
Date: Fri Feb 02 11:40:34 2018

power: Use initctl to reboot the system.

Update powerd_setuid_helper to use initctl to go to runlevel
6 in order to reboot the system. It previously just executed
the "reboot" command, which prevented the SHUTDOWN_REASON
variable from being passed to Upstart, which prevented the
pre-shutdown job from logging a useful explanation.

Also make the "unknown" reason instead get logged as
"other-request-to-powerd", and use "not-via-powerd" when
SHUTDOWN_REASON is unset (typically meaning that someone
called shutdown/poweroff/halt/reboot/etc. directly).

BUG= chromium:807509 
TEST=manual: ran "reboot", made RequestRestart d-bus method
     call, and held power button; saw expected actions
     and pre-shutdown logs in /var/log/messages

Change-Id: Ib129c77d42a5beee02dae78bccee96736f747597
Reviewed-on: https://chromium-review.googlesource.com/896606
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Todd Broch <tbroch@chromium.org>

[modify] https://crrev.com/195f346dffc7c2d5b306e2acfef5b95751e88328/power_manager/powerd/powerd_setuid_helper.cc
[modify] https://crrev.com/195f346dffc7c2d5b306e2acfef5b95751e88328/power_manager/common/power_constants.cc
[modify] https://crrev.com/195f346dffc7c2d5b306e2acfef5b95751e88328/power_manager/common/power_constants.h
[modify] https://crrev.com/195f346dffc7c2d5b306e2acfef5b95751e88328/power_manager/powerd/daemon.cc
[modify] https://crrev.com/195f346dffc7c2d5b306e2acfef5b95751e88328/init/upstart/pre-shutdown.conf
[modify] https://crrev.com/195f346dffc7c2d5b306e2acfef5b95751e88328/power_manager/powerd/daemon_unittest.cc

Comment 2 by derat@chromium.org, Feb 2 2018

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 14 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b1f5f9e57a47ae4632ff88ed11038b366d551dd

commit 2b1f5f9e57a47ae4632ff88ed11038b366d551dd
Author: Daniel Erat <derat@chromium.org>
Date: Wed Mar 14 23:13:28 2018

metrics: Add ShutdownReason value to enums.xml.

Document the "Other request to powerd" reason in the Chrome
OS ShutdownReason enum in enums.xml. This enum is defined in
powerd, i.e. outside of the Chromium repository.

Bug:  807509 
Change-Id: I7a07fba2770480c207f711580ddd9287ba71bfb3
Reviewed-on: https://chromium-review.googlesource.com/963486
Commit-Queue: Dan Erat <derat@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543233}
[modify] https://crrev.com/2b1f5f9e57a47ae4632ff88ed11038b366d551dd/tools/metrics/histograms/enums.xml
►

Sign in to add a comment