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

Issue 616620 link

Starred by 7 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Add fallback path to flashrom/mosys/ectool file lock

Reported by dhend...@chromium.org, Jun 1 2016

Issue description

Some system utilities such as flashrom, mosys, and ectool utilize one or more lockfiles to control resource contention. A few weeks ago we switched from using the old-style "/var/run/lock" as our temp directory in favor of "/run" which is better defined in the Filesystem Hierarchy Standard 3.0: https://chromium-review.googlesource.com/#/c/329996/

However, old systems which do not conform to FHS 3.0 might still receive a newer version of flashrom during an auto-update and attempt to use it, which will fail.

As a band-aid we should just fall-back to /tmp as our lockfile directory. /tmp is fine because:
- It exists on every Linux system we care about.
- We'll only encounter this corner case during auto-update, so the system is already booted and filesystems mounted.
- This doesn't apply to Android, so no worries there.

 
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 3 2016

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

commit 4b680119cc1de2dda7c0b625c4fea1d1e964189a
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:51:23 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: Idbe63a574474ec35a5c3b6fe2b0fb3b672941492
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348850
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/4b680119cc1de2dda7c0b625c4fea1d1e964189a/util/lock/file_lock.c

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/2c7fbec0d1319d9426fbdce88105c72b1234c49a

commit 2c7fbec0d1319d9426fbdce88105c72b1234c49a
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:32:33 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: Icba3381bfdc23673346d895b87b25afe3bfb8450
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348840
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>

[modify] https://crrev.com/2c7fbec0d1319d9426fbdce88105c72b1234c49a/core/file_lock.c

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 3 2016

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

commit 467b82583f0419ea1b49b9e7e739ad257b1d6528
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:49:53 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: I64710cf2f9e131f6ea82b0a7bf18ea090e2ebbda
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348841
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/467b82583f0419ea1b49b9e7e739ad257b1d6528/file_lock.c

Status: Fixed (was: Started)
Patches are in. Perhaps we should cherry-pick them into the release branch?
Cc: bhthompson@chromium.org gkihumba@chromium.org
Labels: Merge-Request-52
I did some more digging and I think this bug applies to auto-updates which occur from any version prior to 5981.0 (when CL:186240 landed) to 8001.0-8407.0.

I think it would be good to cherry-pick these fixes into M52. The ship has almost already sailed on M51, but we might be able to add some AU trickery to avoid issues by forcing an updated to something in between 5981.0 and 8000.0 first.

https://chromium-review.googlesource.com/#/c/350153/
https://chromium-review.googlesource.com/#/c/350125/
https://chromium-review.googlesource.com/#/c/350202/

Comment 7 by tin...@google.com, Jun 7 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Owner: dhend...@chromium.org
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 7 2016

Labels: merge-merged-release-R52-8350.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/7be7ca66eb78928d9427aa379207524d35c5eb43

commit 7be7ca66eb78928d9427aa379207524d35c5eb43
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:49:53 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: I64710cf2f9e131f6ea82b0a7bf18ea090e2ebbda
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348841
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
(cherry picked from commit 467b82583f0419ea1b49b9e7e739ad257b1d6528)
Reviewed-on: https://chromium-review.googlesource.com/350153

[modify] https://crrev.com/7be7ca66eb78928d9427aa379207524d35c5eb43/file_lock.c

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 7 2016

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

commit 5b125be7660c78b3235ccf50ffe9b8b080ec97de
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:51:23 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: Idbe63a574474ec35a5c3b6fe2b0fb3b672941492
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348850
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
(cherry picked from commit 4b680119cc1de2dda7c0b625c4fea1d1e964189a)
Reviewed-on: https://chromium-review.googlesource.com/350202

[modify] https://crrev.com/5b125be7660c78b3235ccf50ffe9b8b080ec97de/util/lock/file_lock.c

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/a763a5954fc45317f220b79e85570178bc1bae4f

commit a763a5954fc45317f220b79e85570178bc1bae4f
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:32:33 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: Icba3381bfdc23673346d895b87b25afe3bfb8450
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348840
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
(cherry picked from commit 2c7fbec0d1319d9426fbdce88105c72b1234c49a)
Reviewed-on: https://chromium-review.googlesource.com/350125

[modify] https://crrev.com/a763a5954fc45317f220b79e85570178bc1bae4f/core/file_lock.c

Project Member

Comment 11 by sheriffbot@chromium.org, Jun 10 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Approved -Merge-Approved-52
Adding some background info discussed in an e-mail thread:
We expect the failure to occur when the gap is large. I can guess, but haven't tested these exact versions. In short:
Old style: /var/run/lock exists
New style: /run/lock exists
Workaround implemented in the last set of patches: If /run/lock does not exist then fall back to using /tmp instead (since /var/run isn't mounted early enough to satisfy the recovery image requirement).

Where the cutoff is between the "old" and "new" style is I'm not 100% sure. The new filesystem layout was officially released in June 2015, however it looks like we've been migrating toward using /run since 2014 when CL:186240 was merged.

So I would guess that this will impact any system older than 5981.0 when CL:186240 landed: https://crosland.corp.google.com/cl?q=chromium:186240

...

Basically we want to avoid updating from anything older than 5981.0 (when CL:186240 landed) to any version from 8001.0 to 8407.0.

Comment 14 by de...@chromium.org, Jun 16 2016

For the last line: If any of those versions (8001 to 8407) is still serving then backport the fix to the branch. If not... well, that's it.
Cc: scunning...@chromium.org dhend...@chromium.org vwang@chromium.org
 Issue 621781  has been merged into this issue.

Comment 16 by stevenh@google.com, Jun 22 2016

Can this fix be ported to R51 and distributed on Stable channel?  R52 is scheduled to go to Stable on 7/26, more than a month from now.

We have received many customer feedback reports of Panther units that are not updating due to this issue.

Labels: Merge-Request-51
Re #16: SGTM.

Comment 19 by tin...@google.com, Jun 22 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M51), manual review required.
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 25 2016

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

commit ea66c64c3bcdba7d82ef9c5778d494a90332641c
Author: David Hendricks <dhendrix@chromium.org>
Date: Thu Jun 09 20:28:58 2016

power: update flashrom_powerd.lock path

This updates the directory for flashrom_powerd.lock to use "/run/lock"
instead of "/var/lock" as per FHS 3.0.

BUG= chromium:616620 
CQ-DEPEND=CL:351271
BRANCH=none
TEST=compiled

Change-Id: I26ece6b26f8cafa70d9fd0fe1a5047cc4a6299df
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/351360
Reviewed-by: Dan Erat <derat@chromium.org>

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

Project Member

Comment 21 by bugdroid1@chromium.org, Jun 25 2016

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

commit f8bcbca2f702fe7df6051e2a9781464a2ef8a448
Author: David Hendricks <dhendrix@chromium.org>
Date: Thu Jun 09 23:44:27 2016

sb_firmware: update lockfile path

This updates the lockfile path for FHS 3.0 since powerd as well
as other pieces of software are migrating over.

BUG= chromium:616620 
CQ-DEPEND=CL:351271
BRANCH=none
TEST=compiled

Change-Id: I6aa5fa30225e45039316e4a3af0e50cdef0fdf4e
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/351345
Reviewed-by: Dan Erat <derat@chromium.org>

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

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 25 2016

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

commit 7b082e5d513d987db4683b07d03f7942b5c134bb
Author: David Hendricks <dhendrix@chromium.org>
Date: Thu Jun 09 20:31:09 2016

power: update lock file paths

This updates the directory for flashrom and battery_tool lockfiles
from "/var/lock" to "/run/lock" as per FHS 3.0.

BUG= chromium:616620 
CQ-DEPEND=CL:351360,CL:351345
BRANCH=none
TEST=compiled

Change-Id: Ic0c227755807516cbe1c5f40b471d07fd5693ddd
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/351271
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/7b082e5d513d987db4683b07d03f7942b5c134bb/power_manager/powerd/daemon.cc

Labels: -Merge-Review-51 Merge-Approved-51
Lets land these, we may do another 51 after all.
Labels: -Hotlist-Merge-review
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 6 2016

Labels: merge-merged-release-R51-8172.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mosys/+/ebe388fb910f086933aba3400ca7a0fddeda09bd

commit ebe388fb910f086933aba3400ca7a0fddeda09bd
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:32:33 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: Icba3381bfdc23673346d895b87b25afe3bfb8450
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348840
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
(cherry picked from commit 2c7fbec0d1319d9426fbdce88105c72b1234c49a)
Reviewed-on: https://chromium-review.googlesource.com/354713
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/ebe388fb910f086933aba3400ca7a0fddeda09bd/core/file_lock.c

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 6 2016

Labels: merge-merged-release-R51-8172.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/afca2a763e5435cc53485b5fdd079f3443f4832d

commit afca2a763e5435cc53485b5fdd079f3443f4832d
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:49:53 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: I64710cf2f9e131f6ea82b0a7bf18ea090e2ebbda
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348841
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
(cherry picked from commit 467b82583f0419ea1b49b9e7e739ad257b1d6528)
Reviewed-on: https://chromium-review.googlesource.com/354712
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/afca2a763e5435cc53485b5fdd079f3443f4832d/file_lock.c

Project Member

Comment 27 by bugdroid1@chromium.org, Jul 6 2016

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

commit e63fa9927f218ae26558a00f5de6b674a41a2ef0
Author: David Hendricks <dhendrix@chromium.org>
Date: Wed Jun 01 22:51:23 2016

file_lock: Add fallback directory

This adds a fallback directory in case SYSTEM_LOCKFILE_DIR is
unavailable. Since this is a band-aid meant to help older systems
auto-update, the fallback path is hardcoded to "/tmp" as to avoid
polluting the overall lockfile API.

BUG= chromium:616620 
BRANCH=none
TEST=Tested on veyron_jaq by removing /run/lock and seeing
mosys, flashrom, and ectool run successfully with
firmware_utility_lock in /tmp.

Change-Id: Idbe63a574474ec35a5c3b6fe2b0fb3b672941492
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/348850
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
(cherry picked from commit 4b680119cc1de2dda7c0b625c4fea1d1e964189a)
Reviewed-on: https://chromium-review.googlesource.com/354780
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/e63fa9927f218ae26558a00f5de6b674a41a2ef0/util/lock/file_lock.c

Cc: -scunning...@chromium.org dchan@chromium.org dhadd...@chromium.org
Labels: OS-Chrome
 Issue 626166  has been merged into this issue.
Performed AU from 5500.100.9(M34) => 8172.62.0(51) and its successfully updated. Thanks.

Project Member

Comment 31 by sheriffbot@chromium.org, Jul 10 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 34 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 35 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 36 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 37 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 38 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 40 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment