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

Issue 787152 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

console-ramoops seems missing from userspace

Project Member Reported by diand...@chromium.org, Nov 20 2017

Issue description

As discovered in bug #782325, it appears that console-ramoops is not accessible from userspace anymore.  AKA:

* It's not in chrome://system
* It's not collected by Feedback Reports

Needless to say that's a big problem since console-ramoops is a goto place when looking for crash info.

This is confirmed as early as R62 9901.66.0 but probably also applies to earlier builds.  It also seems to affect later builds.  I've been mostly poking around with peach_pit / peach_pi / gru_kevin.  Presumably it also must affect some other boards (if not all of them?)

 
Cc: groeck@chromium.org
Umm, WTF?

localhost ~ # ls -al /sys/fs/pstore/
total 0
dr-xr-xr-x. 2 root root 0 Nov 20 18:46 .
drwxr-xr-x. 8 root root 0 Nov 16 14:07 ..
localhost ~ # ls -al /dev/pstore/
total 0
drwx--x---.  2 root debugd      0 Nov 16 14:07 .
drwxr-xr-x. 16 root root     2780 Nov 16 14:07 ..
-r--r--r--.  1 root root   204485 Nov 16 14:07 console-ramoops

https://chromium-review.googlesource.com/753518

-  { "console-ramoops", "/bin/cat /dev/pstore/console-ramoops* 2> /dev/null" },
+  { "console-ramoops", "/bin/cat /sys/fs/pstore/console-ramoops* 2>/dev/null" },

Notably, we do some hacks because upstream changed the path name on us. Documentation might not match implementation.

But that's not in M62, so that can't be the only problem. Probably this?

https://chromium-review.googlesource.com/614701

Could the globbing somehow not be working? Notably it was only claimed to have been tested in the precq...
Ah, I didn't have this yet:

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/753516

That would properly mount /sys/fs/pstore, so CL:753518 should be fine too.

But maybe the globbing isn't working?

Comment 3 by groeck@chromium.org, Nov 21 2017

We carry along a patch which converts the upstream name back to console-ramoops (f655c177e5ad in chromeos-4.4; it would be great if we can drop that at some point). I also don't recall seeing a problem with pstore in my trybot runs for 4.14. Maybe something went wrong with the pstore backports ? I'm quite sure I tested those at the time on kevin, but on the other side I didn't know about /sys/fs/pstore at the time. Maybe that path is broken in chromeos-4.4 ?

Oh, it's definitely the globbing that's the problem:

# sudo -u debugd bash
debugd@localhost /root $ cat /dev/pstore/console-ramoops
... (lots of dmesg)
debugd@localhost /root $ cat /dev/pstore/console-ramoops*
cat: '/dev/pstore/console-ramoops*': No such file or directory


The problem is that the debugd user doesn't have read permissions on /dev/pstore/ (the directory), so it can't properly enumerate the glob contents (/dev/pstore/console-ramoops*).

We need the 'chmod 0710 /dev/pstore' to be something more like 'chmod 0750 ...' in /etc/init/pstore.conf

Comment 5 by vapier@chromium.org, Nov 21 2017

we should revert/drop any changes to pstore file naming.  i've landed a lot of CLs to deal with old/new names, and it should all work now.

similarly, i've been landing CLs to move everything to /sys/fs/pstore.

i have been assuming that globs work, so yes, we'll want to change to 0750.

Comment 6 by vapier@chromium.org, Nov 21 2017

Owner: vapier@chromium.org
Status: Started (was: Untriaged)
posted https://chromium-review.googlesource.com/780526
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 21 2017

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ace0eb09ecff68a19a3a598d3ae164114b4ae191

commit ace0eb09ecff68a19a3a598d3ae164114b4ae191
Author: Guenter Roeck <groeck@chromium.org>
Date: Tue Nov 21 09:59:13 2017

Revert "CHROMIUM: HACK: pstore: Rename console-ramoops-<id> to console-ramoops"

This reverts commit 94d896ab1fbc1f7d8fe9af314e074e5bb4f870dc.

No longer needed.

BUG= chromium:787152 ,  chromium:755343 
TEST=Build and test various platforms to ensure that the new file name
	is handled correctly.

Change-Id: I00f8eefa1e9daa49ce9609220fb9499ddaa0d54b
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/780848
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/ace0eb09ecff68a19a3a598d3ae164114b4ae191/fs/pstore/inode.c

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 21 2017

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

commit 8d20df4f61eac3b18090d768f2f5d859ee96b996
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Nov 21 09:59:08 2017

init: pstore: grant read access to debugd

In order to support globbing to find all the console-ramoops names,
we need to grant read access to the debugd group.

BUG= chromium:755343 ,  chromium:787152 
TEST=`sudo -u debugd ls /dev/pstore/` works now

Change-Id: Icdfa657b1aef91579e78b5d9b4dcd6e1ab784007
Reviewed-on: https://chromium-review.googlesource.com/780526
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/8d20df4f61eac3b18090d768f2f5d859ee96b996/init/upstart/pstore.conf

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 21 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/62ad6dd8a5b75e316875358dd68e3c7795d67c6b

commit 62ad6dd8a5b75e316875358dd68e3c7795d67c6b
Author: Guenter Roeck <groeck@chromium.org>
Date: Tue Nov 21 12:25:09 2017

Revert "CHROMIUM: HACK: pstore: Rename console-ramoops-<id> to console-ramoops"

This reverts commit 94d896ab1fbc1f7d8fe9af314e074e5bb4f870dc.

No longer needed.

BUG= chromium:787152 ,  chromium:755343 
TEST=Build and test various platforms to ensure that the new file name
	is handled correctly.

Change-Id: I00f8eefa1e9daa49ce9609220fb9499ddaa0d54b
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit fc3e9c32efc6d95bd61c256b21d474baad8a4bf1
 and resolved conflicts)
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/780923
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/62ad6dd8a5b75e316875358dd68e3c7795d67c6b/fs/pstore/inode.c

I'm not working this week but hopefully someone can request merge as far as
we can get permission. Debugging feedback reports will be VERY hard if we
don't get this merged.
Labels: Merge-Request-63 Merge-Request-62
we only need to merge CL:780526 which should be trivial
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 21 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 13 Deleted

Comment 14 Deleted

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 22 2017

Labels: merge-merged-release-R63-10032.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a1befd450f1936817732f20c523a30c7e2d3a2c8

commit a1befd450f1936817732f20c523a30c7e2d3a2c8
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Nov 22 21:36:52 2017

init: pstore: grant read access to debugd

In order to support globbing to find all the console-ramoops names,
we need to grant read access to the debugd group.

BUG= chromium:755343 ,  chromium:787152 
TEST=`sudo -u debugd ls /dev/pstore/` works now

Change-Id: Icdfa657b1aef91579e78b5d9b4dcd6e1ab784007
Reviewed-on: https://chromium-review.googlesource.com/780526
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit commit 8d20df4f61eac3b18090d768f2f5d859ee96b996)
[Context adjustment, since this was refactored in the meantime for
moving to /sys/fs/pstore/ path]
Reviewed-on: https://chromium-review.googlesource.com/786272
Commit-Queue: Brian Norris <briannorris@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/a1befd450f1936817732f20c523a30c7e2d3a2c8/init/upstart/pstore.conf

Labels: -Merge-Request-62 -Merge-Review-63 merge-merged-63
Status: Fixed (was: Started)
Hmm, the merge approvals in #14 were deleted? I'm going to go out on a limb here and say that wasn't intentional.
Cc: bhthompson@chromium.org
Labels: -M-62
This didn't make it into M-62, so removing the tag.

+Bernie in case he's aware of any random M-62 builds that are happening where this might fit in, but probably we'll just have to wait till M-63 and skip looking in detail at feedback reports until then...
At this point I think we are late for 62, we have no more builds planned, we should see 63 stable in a couple weeks though.

Sign in to add a comment