New issue
Advanced search Search tips

Issue 873903 link

Starred by 0 users
Status: Fixed
Owner:
amistry@chromium.org
Closed: Oct 18
Cc:
weifangsun@chromium.org
slangley@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

Add metrics and better logs for mount failures.

Project Member Reported by amistry@chromium.org, Aug 14 
Right now, we only have largely anecdotal reports of mount flakiness, but not much information to work with. I propose we add UMA metrics and ERROR logs on certain code paths (in both chrome and cros-disks), to try and gather some more information about what's going on.

Comment 1 by slangley@chromium.org, Aug 15

Labels: M-70
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25b15981e8dee9ea951266e9da7c36facac2a243

commit 25b15981e8dee9ea951266e9da7c36facac2a243
Author: Anand K. Mistry <amistry@chromium.org>
Date: Thu Aug 16 01:19:13 2018

Split chromeos::MountError from cros_disks::MountErrorType.

Make chromeos::MountError contiguous and add explicit translation
between the two types. This will allow mount errors to be recorded in
UMA.

BUG= 873903 

Change-Id: Ib9225cf7befd0521cc329c47dd482e24cd1dd0de
Reviewed-on: https://chromium-review.googlesource.com/1174090
Commit-Queue: Anand Mistry <amistry@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583481}
[modify] https://crrev.com/25b15981e8dee9ea951266e9da7c36facac2a243/chromeos/dbus/cros_disks_client.cc
[modify] https://crrev.com/25b15981e8dee9ea951266e9da7c36facac2a243/chromeos/dbus/cros_disks_client.h
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 20 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9

commit 9647a6ed3bafddc5cf17a7de50627ff33b4edcf9
Author: Anand K. Mistry <amistry@chromium.org>
Date: Mon Aug 20 00:33:10 2018

Return unmount error from CrosDisksClient instead of success/fail.

Receiving the unmount error code will let Chrome properly handle various
types of errors, and produce better logging.

BUG= 873903 

Change-Id: I792a806440be6725ceba0663afc4b4ee3069d9bd
Reviewed-on: https://chromium-review.googlesource.com/1179109
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584348}
[modify] https://crrev.com/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9/chromeos/dbus/cros_disks_client.cc
[modify] https://crrev.com/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9/chromeos/dbus/cros_disks_client.h
[modify] https://crrev.com/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9/chromeos/dbus/fake_cros_disks_client.cc
[modify] https://crrev.com/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9/chromeos/dbus/fake_cros_disks_client.h
[modify] https://crrev.com/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9/chromeos/disks/disk_mount_manager.cc
[modify] https://crrev.com/9647a6ed3bafddc5cf17a7de50627ff33b4edcf9/chromeos/disks/disk_mount_manager_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 21 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62067965541204a49faef0a92e9e2f6e2d50fe8a

commit 62067965541204a49faef0a92e9e2f6e2d50fe8a
Author: Anand K. Mistry <amistry@chromium.org>
Date: Tue Aug 21 03:18:22 2018

Return error code from UnmountDeviceRecursively.

At the same time, fix up two outstanding TODOs:
1. A potential memory leak due to lack of callback data ownership.
2. Not correctly reporting unmount errors on a multi-parition device.

Also add some tests for UnmountDeviceRecursively.

BUG= 873903 

Change-Id: I3e99afa4e0185b18c1d7077a5c65fc225a5751b3
Reviewed-on: https://chromium-review.googlesource.com/1179421
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584627}
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.cc
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chrome/browser/extensions/api/image_writer_private/operation.h
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chrome/browser/extensions/api/image_writer_private/operation_chromeos.cc
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chrome/browser/extensions/api/image_writer_private/test_utils.cc
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chrome/browser/extensions/api/image_writer_private/test_utils.h
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chromeos/disks/disk_mount_manager.cc
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chromeos/disks/disk_mount_manager.h
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chromeos/disks/disk_mount_manager_unittest.cc
[modify] https://crrev.com/62067965541204a49faef0a92e9e2f6e2d50fe8a/chromeos/disks/mock_disk_mount_manager.h
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 23 

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

commit 36d2b301ff2388262d5af40d7d1895800a2b9c03
Author: Anand K Mistry <amistry@chromium.org>
Date: Thu Aug 23 14:36:56 2018

cros-disks: Return error code in Unmount method.

Support for this was added to Chrome, so now update cros-disks.

BUG= chromium:873903 
CQ-DEPEND=CL:1175512
TEST=platform_CrosDisksDBus autotest + manual verification on chell

Change-Id: Icc236628867805d1e06e7ce241f2dd66a7ab8de9
Reviewed-on: https://chromium-review.googlesource.com/1175510
Commit-Ready: Anand Mistry <amistry@chromium.org>
Tested-by: Anand Mistry <amistry@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/36d2b301ff2388262d5af40d7d1895800a2b9c03/cros-disks/dbus_bindings/org.chromium.CrosDisks.xml
[modify] https://crrev.com/36d2b301ff2388262d5af40d7d1895800a2b9c03/cros-disks/cros_disks_server.cc
[modify] https://crrev.com/36d2b301ff2388262d5af40d7d1895800a2b9c03/cros-disks/cros_disks_server.h
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/966caf77cad88f3e81b9ac0efe740af1dedb2fc4

commit 966caf77cad88f3e81b9ac0efe740af1dedb2fc4
Author: Anand K Mistry <amistry@chromium.org>
Date: Thu Aug 23 14:36:55 2018

platform_CrosDisksDBus: Verify error code in unmount test

BUG= chromium:873903 
TEST=run platform_CrosDisksDBus

Change-Id: Ia66b7321ecfb818c1311f86263f52b726638ad09
Reviewed-on: https://chromium-review.googlesource.com/1175512
Commit-Ready: Anand Mistry <amistry@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/966caf77cad88f3e81b9ac0efe740af1dedb2fc4/client/site_tests/platform_CrosDisksDBus/platform_CrosDisksDBus.py
[modify] https://crrev.com/966caf77cad88f3e81b9ac0efe740af1dedb2fc4/client/cros/cros_disks.py
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 5 

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

commit 26c3a8a3a8a02922977688cacc22505680daf976
Author: Anand K Mistry <amistry@chromium.org>
Date: Wed Sep 05 12:11:42 2018

cros-disks: Have finer grained errors for starting avfsd.

Instead of a boolean success/fail, return an error code from
StartAVFS(). Currently, only one error code,
MOUNT_ERROR_MOUNT_PROGRAM_FAILED, is split out. Other errors may be
changed in the future if needed.

BUG= chromium:873903 
TEST=platform_CrosDisks.* autotests

Change-Id: If5d03056a3d42ab5a5c2bffcb0af10fd7d436c20
Reviewed-on: https://chromium-review.googlesource.com/1203332
Commit-Ready: Anand Mistry <amistry@chromium.org>
Tested-by: Anand Mistry <amistry@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/26c3a8a3a8a02922977688cacc22505680daf976/cros-disks/archive_manager.h
[modify] https://crrev.com/26c3a8a3a8a02922977688cacc22505680daf976/cros-disks/archive_manager.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 12 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b572e5e119dedae5b17b3098fe62c50f8286507b

commit b572e5e119dedae5b17b3098fe62c50f8286507b
Author: Anand K. Mistry <amistry@chromium.org>
Date: Wed Sep 12 06:41:59 2018

Add UMA for MountType,MountError.

The cartesian product of MountType and MountError is flattened into a
single dimension as per the suggestion in the histograms README.

BUG= 873903 

Change-Id: Ibafa929d9c612cbe355ad6f4db57fb003f6d6b35
Reviewed-on: https://chromium-review.googlesource.com/1203518
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590606}
[modify] https://crrev.com/b572e5e119dedae5b17b3098fe62c50f8286507b/chromeos/dbus/cros_disks_client.cc
[modify] https://crrev.com/b572e5e119dedae5b17b3098fe62c50f8286507b/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/b572e5e119dedae5b17b3098fe62c50f8286507b/tools/metrics/histograms/histograms.xml
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 24 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd7d41f8b70dd5a42846daf75c575b1626d9b42a

commit dd7d41f8b70dd5a42846daf75c575b1626d9b42a
Author: Anand K. Mistry <amistry@chromium.org>
Date: Mon Sep 24 03:30:58 2018

Add UMA for tracking Mount/Unmount times.

BUG= 873903 

Change-Id: I61394811e9264a2ea22cedacd7c782bbc4051bf9
Reviewed-on: https://chromium-review.googlesource.com/1233055
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593473}
[modify] https://crrev.com/dd7d41f8b70dd5a42846daf75c575b1626d9b42a/chromeos/dbus/cros_disks_client.cc
[modify] https://crrev.com/dd7d41f8b70dd5a42846daf75c575b1626d9b42a/tools/metrics/histograms/histograms.xml

Comment 11 by slangley@chromium.org, Sep 24

Labels: -M-70 M-71

Comment 12 by slangley@chromium.org, Oct 18 

Is this fixed?

Comment 13 by amistry@chromium.org, Oct 18

Status: Fixed (was: Assigned)
For now...
►

Sign in to add a comment