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

Issue 697293 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

dump_vpd_log: issues when using --stdout and if stateful partition was not ready.

Project Member Reported by hungte@chromium.org, Mar 1 2017

Issue description

As gwendal commented in https://chromium-review.googlesource.com/#/c/417461/2/util/vpd_get_value@37 (traced due to issue crosbug.com/p/62342 ), we know that dump_vpd_log may be not working properly if /mnt/stateful_partition was not ready.

I also found few bugs that:
 - when using --stdout, /dev/stdout will be removed
 - when using --stdout, if it's executed before vpd.conf (or any invocation of dump_vpd_log without --stdout), then cache/vpd will be generate without filtered.txt, causing /var/log/vpd_2.0.txt to be invalid.

It seems to me that dump_vpd_log should be re-factored and address these issues, especially allowing correct output in --stdout if cache can't be created.
 
Also, dump_vpd_log calls flashrom directly, which I think can be prevented since vpd already handles that.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/5160b0ef4a8cfa104a19e8086832a2ba3c34b516

commit 5160b0ef4a8cfa104a19e8086832a2ba3c34b516
Author: Hung-Te Lin <hungte@chromium.org>
Date: Tue Mar 14 22:06:00 2017

dump_vpd_log: Prevent unexpected flashrom output corrupting --stdout.

Most programs call "dump_vpd_log --stdout" and parse that using sed
script, but there are also programs invoking dump_vpd_log without
explicit redirection, which caused output from flashrom being merged to
caller's output. To prevent that, we should redirect flashrom output to
stderr.

BRANCH=None
BUG= chromium:697293 
TEST=dump_vpd_log --stdout --force 2>/dev/null

Change-Id: Ifd80839f26baac29274060844c8a816863009d83
Reviewed-on: https://chromium-review.googlesource.com/448297
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/5160b0ef4a8cfa104a19e8086832a2ba3c34b516/util/dump_vpd_log

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/38f96a402488a860a7d662a1a8359660afc6c693

commit 38f96a402488a860a7d662a1a8359660afc6c693
Author: Hung-Te Lin <hungte@chromium.org>
Date: Tue Mar 14 22:05:59 2017

dump_vpd_log: Functionalize and minor style fixes.

Put main logic into function 'main', including temp file creation and
argument processing.
Also revised coding style in redirection: '> FILE' to '>FILE'.

No function changes except definition of shflags and arg processing (and
'set -e') is now executed before temp files creation. This is needed
because in the future we will change location of files according to the
flags.

BRANCH=None
BUG= chromium:697293 
TEST=dump_vpd_log --force; dump_vpd_log --stdout

Change-Id: I9d91afe0740002963aebdf62e75a969496dcdf5b
Reviewed-on: https://chromium-review.googlesource.com/448336
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/38f96a402488a860a7d662a1a8359660afc6c693/util/dump_vpd_log

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/34c421e55cea232555868b230f4ad1a600ea00ca

commit 34c421e55cea232555868b230f4ad1a600ea00ca
Author: Hung-Te Lin <hungte@chromium.org>
Date: Tue Mar 14 22:05:59 2017

dump_vpd_log: Always generate filtered files to correct --stdout behavior.

The --stdout had few issues:
 1. Will remove /dev/stdout on exit.
 2. Will not build filtered file until some programs invoked it without
     --stdout, which is a problem when few scripts try to do --clean
     then --stdout to get values (/var/log/vpd_2.0.txt will be not
     available until reboot).

This change tries to correct its behavior by few changes:
 1. FILTERED_FILE should be always created.
 2. --stdout should cat right file (by checking --full or not).
 3. generate_sed_filter does not need to process 'full', since that is
    identical to CACHE_FILE.
 4. Pass file path to sed command directly to avoid unnecessary
    redirection.

BRANCH=None
BUG= chromium:697293 
TEST=dump_vpd_log --clean
     dump_vpd_log --full --stdout # see full VPD entries
     ls -l /dev/stdout # available
     cat /var/log/vpd_2.0.txt # see filtered data
     dump_vpd_log --stdout # see filtered data

Change-Id: I526bceff817e6f1a1d147d8a96727351560c2a2d
Reviewed-on: https://chromium-review.googlesource.com/448337
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/34c421e55cea232555868b230f4ad1a600ea00ca/util/dump_vpd_log

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/9fd4e02fcd6c795b3f5c965f748a85cd170b6dbc

commit 9fd4e02fcd6c795b3f5c965f748a85cd170b6dbc
Author: Hung-Te Lin <hungte@chromium.org>
Date: Tue Mar 14 22:05:59 2017

dump_vpd_log: Move cache file generation into a standalone function.

Generation of cache file should be isolated for future refactoring.

BRANCH=None
BUG= chromium:697293 
TEST=dump_vpd_log --clean
     dump_vpd_log
     dump_vpd_log --stdout
     dump_vpd_log --stdout --full

Change-Id: Ib87294c662ba10c7a79b2a2fe3292d1650e04b90
Reviewed-on: https://chromium-review.googlesource.com/448338
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/9fd4e02fcd6c795b3f5c965f748a85cd170b6dbc/util/dump_vpd_log

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 14 2017

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

commit fb4dce09e3f27942b9ba63d4a77e3dd8169a08a0
Author: Hung-Te Lin <hungte@chromium.org>
Date: Tue Mar 14 22:05:59 2017

dump_vpd_log: Create temp files only when needed.

*_TMP was always created and deleted, which is not necessary when the
cache files were already created properly.

This change also delayed creation of temp files so it is possible to
allow dump_vpd_log to run without mounted stateful partition in the
future (if we change CACHE_DIR to a writable partition).

BRANCH=None
BUG= chromium:697293 
TEST=dump_vpd_log --clean
     dump_vpd_log --full --stdout
     dump_vpd_log --stdout

Change-Id: I73ab4415caa101d4879d44fe05d1946f2fd6ede1
Reviewed-on: https://chromium-review.googlesource.com/448340
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/fb4dce09e3f27942b9ba63d4a77e3dd8169a08a0/util/dump_vpd_log

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/133ff66e1dcb43a934a6dbdc8beeddbc59818d60

commit 133ff66e1dcb43a934a6dbdc8beeddbc59818d60
Author: Hung-Te Lin <hungte@chromium.org>
Date: Tue Mar 14 22:06:00 2017

dump_vpd_log: Allow --stdout even if stateful partition is not available.

For programs that want to access VPD data on legacy devices
(dump_vpd_log --stdout), we do want to print the values even if stateful
partition was not ready (for startup scripts).

BRANCH=None
BUG= chromium:697293 
TEST=dump_vpd_log --clean
     touch /mnt/stateful_partition/uncrypted/cache/vpd \
           /var/cache/vpd /var/cache/echo # So dump_vpd_log can't create folder.
     dump_vpd_log  # See faliure
     dump_vpd_log --stdout  # See correct VPD data.

Change-Id: I1d18ec3104080c5d8b6b76e44d3744e73bdf028c
Reviewed-on: https://chromium-review.googlesource.com/448376
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/133ff66e1dcb43a934a6dbdc8beeddbc59818d60/util/dump_vpd_log

Comment 8 by gkihumba@google.com, Mar 31 2017

Owner: hungte@chromium.org
Status: Available (was: Untriaged)
Status: Fixed (was: Available)

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

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 12 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment