FYI Hiro, we've talked about this in this meeting.

Please tag with applicable OSs.  Thanks!

We already have code to skip files which can easily get corrupted.
https://chromium-review.googlesource.com/c/489869/
Probably we should expand the whitelist for Chrome browser cache files too.

When skip happens, will user be able to know which files were skipped after migration finishes?

 - No UI. If you must you can see the cryptohomed logs.
 - User is not worse than before, they had a file they could not open before, now they have a file that does not exist.

Do you have any data to back up the claim that the user is no worse off
than before?  I can think of a number of cases where the user is definitely
worse off than they were.  Specifically:
Apps that use a file as a marker and only check for presence
Apps that are set to sync broken data from the cloud but only when trying
to read existing data
Cases where IO errors throw a nice visible error telling the user what to
do but missing files cause subtle issues down the road (this is fairly
common with games where they just load all resources in a folder into
memory then reference resources by ID later)

There are certainly cases in the other direction as well, where missing
files is better than corrupt files.  Without more data I have no way of
knowing which is more common.

 - Comparing forced deletion of all data vs having potentially misbehaving apps here and there, I would let the user pick potentially misbehaving apps and an option to clear the profile themselves.

Most definitely agree, informing the user that we found bad files during
the migration and let them decide whether to back up/clear or not makes the
most sense.

2017年5月9日(火) 16:21 uekawa via monorail <monorail+v2.3705974674@chromium.org

proposed change:
https://chromium-review.googlesource.com/#/c/499948/

P1 is to allow user to successfully migrate.
UI improvement on notifying users is nice to have.

We should make sure we track which files are skipped and store the list locally so that we can do something more intelligent in the next update.

Taking this over.

Any update on this blocker? 

Most users on dev went through migration with M59, any concern moving this issue to be beta blocker instead?

most boards did not go through the migration on M59 dev, the most users who are going through migration are going through the first time on M60.

We should have other metrics in place so we are happier.

This means it will affect at most 5% users for those M60 devices (those that did not go through in M59, such as kevin and 10 others) will get their profile wiped.

https://chromium-review.googlesource.com/c/499948/
change has been sitting in cq 

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

commit 47b3d3a0fd3255c1aba925d5b2a024723dac9f4a
Author: Junichi Uekawa <uekawa@google.com>
Date: Tue May 16 06:34:43 2017

cryptohome: Skip known open failures.

We know that one pattern of unrecoverable ecryptfs corruption used to
happen.  Skip the files since they are no longer recoverable and not
readable.  The paths of skipped files are stored in the user's home
directory so that we can attempt to help the user restore their system
at a later time.

BUG= chromium:719761 
BUG=b:37729571
TEST=cros_workon_make --test  chromeos-base/cryptohome
TEST=manually
cryptohome --action=mount_ex --user=<mail> --key_label=foo --create --ecryptfs
cryptohome --action=status
dd if=/dev/zero of=/home/user/<shou>/unko bs=1M count=100
dd if=/dev/zero of=/home/.shadow/<shou>/vault/user/ECRYPTFS_FNEK_ENCRYPTED.xxxx \
  bs=1M count=3
head /home/.shadow/<shou>/mount/user/unko  # observe kernel error and EIO
cryptohome --action=unmount --user=<mail>
logged in, migrate.
Observe "Found file that cannot be opened with EIO,
skipping /home/.shadow/<shou>/temporary_mount/user/unko"
log message in /var/log/messages and a successful migration.

Change-Id: I4791eacf8982c488bc33c11c66fd476924479fe3
Reviewed-on: https://chromium-review.googlesource.com/506518
Reviewed-by: Josafat Garcia <josafat@chromium.org>
Commit-Queue: Josafat Garcia <josafat@chromium.org>
Tested-by: Josafat Garcia <josafat@chromium.org>

[modify] https://crrev.com/47b3d3a0fd3255c1aba925d5b2a024723dac9f4a/cryptohome/dircrypto_data_migrator/migration_helper.cc
[modify] https://crrev.com/47b3d3a0fd3255c1aba925d5b2a024723dac9f4a/cryptohome/dircrypto_data_migrator/migration_helper.h
[modify] https://crrev.com/47b3d3a0fd3255c1aba925d5b2a024723dac9f4a/cryptohome/dircrypto_data_migrator/migration_helper_unittest.cc

change is still sitting in CQ

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

commit 7c0bfa8548cad21fd1b2bba67329ba6941ebac35
Author: Junichi Uekawa <uekawa@google.com>
Date: Mon May 22 10:20:40 2017

cryptohome: Skip known open failures.

We know that one pattern of unrecoverable ecryptfs corruption used to
happen.  Skip the files since they are no longer recoverable and not
readable.  The paths of skipped files are stored in the user's home
directory so that we can attempt to help the user restore their system
at a later time.

BUG= chromium:719761 
BUG=b:37729571
TEST=cros_workon_make --test  chromeos-base/cryptohome
TEST=manually
cryptohome --action=mount_ex --user=<mail> --key_label=foo --create --ecryptfs
cryptohome --action=status
dd if=/dev/zero of=/home/user/<shou>/unko bs=1M count=100
dd if=/dev/zero of=/home/.shadow/<shou>/vault/user/ECRYPTFS_FNEK_ENCRYPTED.xxxx \
  bs=1M count=3
head /home/.shadow/<shou>/mount/user/unko  # observe kernel error and EIO
cryptohome --action=unmount --user=<mail>
logged in, migrate.
Observe "Found file that cannot be opened with EIO,
skipping /home/.shadow/<shou>/temporary_mount/user/unko"
log message in /var/log/messages and a successful migration.

Change-Id: I4791eacf8982c488bc33c11c66fd476924479fe3
Reviewed-on: https://chromium-review.googlesource.com/499948
Commit-Ready: Junichi Uekawa <uekawa@chromium.org>
Tested-by: Dan Spaid <dspaid@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>

[modify] https://crrev.com/7c0bfa8548cad21fd1b2bba67329ba6941ebac35/cryptohome/dircrypto_data_migrator/migration_helper.cc
[modify] https://crrev.com/7c0bfa8548cad21fd1b2bba67329ba6941ebac35/cryptohome/dircrypto_data_migrator/migration_helper.h
[modify] https://crrev.com/7c0bfa8548cad21fd1b2bba67329ba6941ebac35/cryptohome/dircrypto_data_migrator/migration_helper_unittest.cc

Seems like all CLs landed so this should be marked as fixed 

Junichi, please confirm

Requesting merge to M-59 as we have an issue causing Caroline to go through this code path on M-59 which we didn't expect.

Ping, this is blocking some other patches that have already been merge approved.

We can merge this, however there are no further 59 releases planned, so this is unlikely to go anywhere. 

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

commit ac82718924f1d5dbdea274f4370d31660c6a9ed8
Author: Junichi Uekawa <uekawa@google.com>
Date: Tue Jul 18 22:59:30 2017

cryptohome: Skip known open failures.

We know that one pattern of unrecoverable ecryptfs corruption used to
happen.  Skip the files since they are no longer recoverable and not
readable.  The paths of skipped files are stored in the user's home
directory so that we can attempt to help the user restore their system
at a later time.

BUG= chromium:719761 
BUG=b:37729571
TEST=cros_workon_make --test  chromeos-base/cryptohome
TEST=manually
cryptohome --action=mount_ex --user=<mail> --key_label=foo --create --ecryptfs
cryptohome --action=status
dd if=/dev/zero of=/home/user/<shou>/unko bs=1M count=100
dd if=/dev/zero of=/home/.shadow/<shou>/vault/user/ECRYPTFS_FNEK_ENCRYPTED.xxxx \
  bs=1M count=3
head /home/.shadow/<shou>/mount/user/unko  # observe kernel error and EIO
cryptohome --action=unmount --user=<mail>
logged in, migrate.
Observe "Found file that cannot be opened with EIO,
skipping /home/.shadow/<shou>/temporary_mount/user/unko"
log message in /var/log/messages and a successful migration.

Change-Id: I4791eacf8982c488bc33c11c66fd476924479fe3
Reviewed-on: https://chromium-review.googlesource.com/499948
Commit-Ready: Junichi Uekawa <uekawa@chromium.org>
Tested-by: Dan Spaid <dspaid@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
(cherry picked from commit 7c0bfa8548cad21fd1b2bba67329ba6941ebac35)
Reviewed-on: https://chromium-review.googlesource.com/575251
Reviewed-by: Dan Spaid <dspaid@chromium.org>
Commit-Queue: Dan Spaid <dspaid@chromium.org>

[modify] https://crrev.com/ac82718924f1d5dbdea274f4370d31660c6a9ed8/cryptohome/dircrypto_data_migrator/migration_helper.cc
[modify] https://crrev.com/ac82718924f1d5dbdea274f4370d31660c6a9ed8/cryptohome/dircrypto_data_migrator/migration_helper.h
[modify] https://crrev.com/ac82718924f1d5dbdea274f4370d31660c6a9ed8/cryptohome/dircrypto_data_migrator/migration_helper_unittest.cc

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