Reference: 
https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout#Extended_Attributes

How about storing the xattrs in the source file, instead of the destination file, so that these xattrs will get discarded at the same time as the source file itself?

#5 sounds better to me.

The main reason I went with storing them on the destination file was performance.  Storing them on the source file means we need an additional fsync on the source file to make sure the xattrs are synced before modifying the file.  However after starting on the implementation of removing them from the destination file, it looks like we may need that extra sync step in the removal anyway so it didn't actually buy us anything in the end.

Moving to storing on the source file would mean we have to support both (for cases where the migration was interrupted and continued after an update), so I'd strongly prefer to stick with the destination file for simplicity.

I was suspecting ENOSPC is happening on setxttr on destination file when there's plenty of free disk space left and actually running out of space on inode.
that it's happened on 3.18 devices and on ext4 crypto sounds somewhat fishy too.

Also, I wonder, does deleting them end up reclaiming the spilled out block?

The more I think about this the less compelling the overflow argument is. Ecryptfs also has its own header in the file, so we should be reclaiming a page worth of disk for every file we delete anyway.  In either case it seems that the selinux context and ext4 encryption xattr alone are enough to overflow in most circumstances, so removing the temporary xattrs won't help reclaim any space.

For the time being I'm going to add more logging in to the code to report the existing xattr size when NO_SPACE is encountered.

This bug requires manual review: M60 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), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

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

commit 3ddd3bc92b1abb19ef0dc36d3cd976bb420c3000
Author: Junichi Uekawa <uekawa@google.com>
Date: Fri Jun 16 07:08:58 2017

Add enum for remove attribute.

BUG= chromium:719287 

Change-Id: I41d13b2ca3e57077a4ac7c823f88e07d1ba1b81b
Reviewed-on: https://chromium-review.googlesource.com/537752
Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479978}
[modify] https://crrev.com/3ddd3bc92b1abb19ef0dc36d3cd976bb420c3000/tools/metrics/histograms/enums.xml

For some reason the CL didn't get linked.  The CL I am requesting to merge to M60 is
https://chromium-review.googlesource.com/c/527974/

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

commit 3e4676ce808978ec92451024a504022bcd69b4c9
Author: Dan Spaid <dspaid@google.com>
Date: Thu Jun 22 02:34:04 2017

cryptohome: remove temporary migration xattrs.

ecryptfs->dircrypto migration introduces temporary xattrs for storing
the atime and mtime of a file.  These are currently not cleaned up, and
may often be causing the file xattrs to overflow into a new storage
block.  This change removes them (after verifying that the times have
been committed to disk).

TEST=cros_workon_make --board=samus --test cryptohome
BUG= chromium:719287 

Change-Id: I15bd8fcce5d679cf54116d48f4530f3c458caa40
Reviewed-on: https://chromium-review.googlesource.com/527974
Commit-Ready: Dan Spaid <dspaid@chromium.org>
Tested-by: Dan Spaid <dspaid@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
(cherry picked from commit 8b3c842d88246ba966787ebe946cd175b74ca137)
Reviewed-on: https://chromium-review.googlesource.com/544355
Reviewed-by: Dan Spaid <dspaid@chromium.org>
Commit-Queue: Dan Spaid <dspaid@chromium.org>

[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/platform_unittest.cc
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/dircrypto_data_migrator/migration_helper_unittest.cc
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/dircrypto_data_migrator/migration_helper.cc
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/dircrypto_data_migrator/migration_helper.h
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/mock_platform.h
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/cryptohome_metrics.h
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/platform.cc
[modify] https://crrev.com/3e4676ce808978ec92451024a504022bcd69b4c9/cryptohome/platform.h

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