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

Issue 655546 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

CRAS: Reland "let minijail parent exit" with proper fix

Project Member Reported by cychiang@chromium.org, Oct 13 2016

Issue description

As shown in
https://wmatrix.googleplex.com/matrix/unfiltered?suites=chameleon_audio_perbuild&days_back=20&hide_missing=True

All tests failed because the test can not restart cras.

The CL:
https://chromium-review.googlesource.com/#/c/395786/ caused the problem.

On any board that is >  8891.
restart cras
stop cras
start cras

all can not finish.

This seems to be related to https://bugs.launchpad.net/upstart/+bug/406397
The solution is that CRAS needs to call fork twice suggested in https://bugs.launchpad.net/upstart/+bug/406397/comments/2.

I should revert the CL first.
 
Cc: chinyue@chromium.org
https://chromium-review.googlesource.com/#/c/397101/ is waiting to be merged.
Cc: ka...@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/2824a233c15e9228b103e12ac8c61eed888f157b

commit 2824a233c15e9228b103e12ac8c61eed888f157b
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Thu Oct 13 09:03:08 2016

Revert "init: let minijail parent exit"

This reverts commit 2bfa009afc6a518121fbde9266e73a60f1edac02.

The commit cause "start cras" "stop cras" "restart cras" stuck forever.

BUG= chromium:655546 
TEST="restart cras" runs fine.

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
Change-Id: Id51ccd216cfb46c4ce3daaadc4456e568ef1c606
Reviewed-on: https://chromium-review.googlesource.com/397101
Reviewed-by: Chinyue Chen <chinyue@chromium.org>

[modify] https://crrev.com/2824a233c15e9228b103e12ac8c61eed888f157b/init/cras.conf
[modify] https://crrev.com/2824a233c15e9228b103e12ac8c61eed888f157b/init/cras.sh

Issue 655677 has been merged into this issue.
Summary: CRAS: Reland "let minijail parent exit" with proper fix (was: CRAS: restart cras can not finish)
The fix has been merged. Change the issue name to reflect the remaining issue.

Comment 7 by vapier@chromium.org, Oct 13 2016

pretty sure the problem is due to the `mosys` call in the init script.  it forks which confuses upstart and then exits.

i've asked in the past: why does that code exist at all ?  why can't samus install a normal /etc/cras/get_device_config_dir hook like other boards ?
I think we can move that mosys command into get_device_config_dir for samus.
I will make a CL. Thanks!
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/0ec989bec89d3b56e68fb53d18fac6dd38c90af5

commit 0ec989bec89d3b56e68fb53d18fac6dd38c90af5
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Fri Oct 14 08:52:51 2016

audio_config: Use get_device_config_dir to decide config directory

Put two dsp configs into pre_mp_a and post_mp_a.
Align the usage of get_device_config_dif like other boards.

BUG= chromium:655546 
TEST=emerge-samus chromeos-bsp-samus

Change-Id: I5958486d8705c02871edb39d7892142cc32e885d
Reviewed-on: https://chromium-review.googlesource.com/398661
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/0ec989bec89d3b56e68fb53d18fac6dd38c90af5/overlay-samus/chromeos-base/chromeos-bsp-samus/files/audio-config/cras-config/post_mp_a/bdw-rt5677
[add] https://crrev.com/0ec989bec89d3b56e68fb53d18fac6dd38c90af5/overlay-samus/chromeos-base/chromeos-bsp-samus/files/audio-config/cras-config/get_device_config_dir
[rename] https://crrev.com/0ec989bec89d3b56e68fb53d18fac6dd38c90af5/overlay-samus/chromeos-base/chromeos-bsp-samus/files/audio-config/cras-config/post_mp_a/dsp.ini
[copy] https://crrev.com/0ec989bec89d3b56e68fb53d18fac6dd38c90af5/overlay-samus/chromeos-base/chromeos-bsp-samus/files/audio-config/cras-config/pre_mp_a/bdw-rt5677
[rename] https://crrev.com/0ec989bec89d3b56e68fb53d18fac6dd38c90af5/overlay-samus/chromeos-base/chromeos-bsp-samus/chromeos-bsp-samus-0.0.2-r15.ebuild
[rename] https://crrev.com/0ec989bec89d3b56e68fb53d18fac6dd38c90af5/overlay-samus/chromeos-base/chromeos-bsp-samus/files/audio-config/cras-config/pre_mp_a/dsp.ini

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/5d14d522d6a07922ca98d225ff7041c60724523e

commit 5d14d522d6a07922ca98d225ff7041c60724523e
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Fri Oct 14 08:56:47 2016

init: Remove logic for samus

The config directory will be handled by get_device_config_dir installed
from chromeos-bsp-samus.

BUG= chromium:655546 
TEST=on samus, check dsp can be loaded.

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
Change-Id: I0c47737e4e1996ffe4bacb31f28c149b2c33ba7d
Reviewed-on: https://chromium-review.googlesource.com/398938
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/5d14d522d6a07922ca98d225ff7041c60724523e/init/cras.sh

what is left to do here?
Labels: -Pri-1 Pri-2
Status: WontFix (was: Assigned)
i don't think it's possible with the current design of CRAS to pull this off.  namely, the /etc/cras/get_device_config_dir script hook will break things.

we'd have to change how we communicate custom changes and board-specific settings.  either a config file that cras itself would read, or something like powerd, or some other system.

if that day ever comes, we can revisit this.

Sign in to add a comment