New issue
Advanced search Search tips

Issue 869478 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

crostini feedback logs empty because "cicerone_client --get_info" returns error

Project Member Reported by za...@chromium.org, Jul 31

Issue description

When looking at the crostini entry in "chrome://system", the information is <empty>. This never worked as merged because the change that added the crostini logs was only ever based off a slightly older patchset of the cicerone_client that had less command line parsing checks: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1112898/1/vm_tools/cicerone/client.cc#483 versus the version that got merged: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1112898/5/vm_tools/cicerone/client.cc#473

The comment "Every D-Bus method for cicerone requires owner ID, VM name, and container name." was correct as of the change prior to the one that added the --get_info command, but is no longer true, and the check is causing a false early exit.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1

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

commit ad0c64542e829b803f5d38c19cccec3d076183fc
Author: Zach Reizner <zachr@google.com>
Date: Wed Aug 01 07:05:32 2018

vm_tools: cicerone: run --get_info command before checking required flags

The required flags --owner_id, --vm_name, and --container_name are not
actually required for the --get_info command, which is meant to get info
about all running VMs and containers. This change does the --get_info
command before checking for those required flags so that crostini
feedback reports will actually get the info requested.

BUG= chromium:869478 
TEST=cicerone_client --get_info; chrome://system

Change-Id: I341a7766a4f08f53753a7e36a64e308508151425
Reviewed-on: https://chromium-review.googlesource.com/1157067
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/ad0c64542e829b803f5d38c19cccec3d076183fc/vm_tools/cicerone/client.cc

Labels: Merge-Request-69 OS-Chrome
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 2

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-69
Labels: -Merge-Review-69 Merge-Approved-69
Merge approved, M69.
Labels: -Merge-Approved-69
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 9

Labels: merge-merged-release-R69-10895.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/52b3ad95964cfed2eae54d8618fb60dd4dde0bd9

commit 52b3ad95964cfed2eae54d8618fb60dd4dde0bd9
Author: Zach Reizner <zachr@google.com>
Date: Thu Aug 09 23:55:44 2018

vm_tools: cicerone: run --get_info command before checking required flags

The required flags --owner_id, --vm_name, and --container_name are not
actually required for the --get_info command, which is meant to get info
about all running VMs and containers. This change does the --get_info
command before checking for those required flags so that crostini
feedback reports will actually get the info requested.

BUG= chromium:869478 
TEST=cicerone_client --get_info; chrome://system

Change-Id: I341a7766a4f08f53753a7e36a64e308508151425
Reviewed-on: https://chromium-review.googlesource.com/1157067
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
(cherry picked from commit ad0c64542e829b803f5d38c19cccec3d076183fc)
Reviewed-on: https://chromium-review.googlesource.com/1170116
Reviewed-by: Zach Reizner <zachr@chromium.org>
Commit-Queue: Zach Reizner <zachr@chromium.org>

[modify] https://crrev.com/52b3ad95964cfed2eae54d8618fb60dd4dde0bd9/vm_tools/cicerone/client.cc

Components: OS>Systems>Containers
Labels: Proj-Containers
@zachr is there more to do here or should this be closed? Looks like feedback logs now have Crostini data.
Ping...can this be closed?
Status: Verified (was: Started)

Sign in to add a comment