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

Issue 625872 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 833730
Owner: ----
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

cryptohome: migrate from using xattr to identify pass through directories.

Project Member Reported by gwendal@chromium.org, Jul 5 2016

Issue description

in recent design, new pass-thru directories use extended attributes to be located:

user/GCache/v1/files: user.GCacheFiles
root/android-data/data/data/ : user.AndroidCache

See
https://docs.google.com/document/d/1Kdq-b5kpudbotQ1q2GRNd52f-oZUrf7mfuZb1XUqH4U/edit#heading=h.sa3sh9r88p48
and
go/arc-disk-full-design

Instead, ecyptfs has been designed with a pass-thru directory feature, adapted to ext4 crypto.
If the directory is created in the fault, it appears as-is in the mount.

In the current code, when we want to create a pass-thru directory in vault/ but a directory with the same name exists in mount/, we forcibly remove it, we don't have to.

To do the migration:
1 - be sure the directory to migrate away from xattr is in
kTrackedDirs local variable of Mount::CreateTrackedSubdirectories()
2 - if mount/ directory exists
2.1 if no xattr, remove
2.2 if xattr
2.2.1 if ext4 crypto:
2.2.1.1. Do not remove/recreate, just get the inode number of future reference
2.2.2 if ecryptfs:
2.2.2.1: in the parent vault directory, look for the directory with the same xattr.
2.2.2.2 rename found directory to passthru name
2.2.3 remove xattr




 
Ignore user.AndroidCache, it is used on each app cache directory.
But we need to migrate root/android-data/data/data/ to a tracked directory.
Temporary add an xattr for it (like we do for user/GCache/v1/files) for tracking (in 2.1 or earlier) instead of deleting.

Comment 2 by loyso@chromium.org, Mar 27 2018

Cc: loyso@chromium.org
Labels: OS-Chrome
FYI
Android switched to using "gid" to mark cache directories, which is less expensive to read (and track through quota).

Comment 4 by loyso@chromium.org, Apr 3 2018

Components: Internals>Media>Encrypted
Labels: CrOSFilesFeature-LowStorage

Comment 5 by loyso@chromium.org, Apr 3 2018

Components: -Internals>Media>Encrypted Internals>Cast>FileManager

Comment 6 by amp@chromium.org, Apr 3 2018

Is this really a casting issue?

Internals>Cast>FileManager is for casting from the files app on CrOS, but I don't see how this issue relates?
Components: -Internals>Cast>FileManager Platform>Apps>FileManager
I think it should be Platform>Apps>FileManager.

Comment 8 by loyso@chromium.org, Apr 4 2018

Sorry, my mistake. Platform>Apps>FileManager it is.

Comment 9 by loyso@chromium.org, Apr 16 2018

Labels: Needs-Feedback
gwendal@, is this issue still applicable?
Do we need to migrate root/android-data/data/data/ to use xattr (so cryptohomed doesn't erase it in low-disk-space scenarios, but sees it as 'pinned')?
Currently for android
root/android-data/data/data/*/cache are marked with 

const char kAndroidCacheInodeAttribute[] = "user.inode_cache";


to mark them as deletable.



In the new scheme of things since Android O, caches are marked with cache gids which are cheaper to scan than xattrs (because of where they are stored).



The original description of this bug is somewhat obsolete and unrelated at this point.

Comment 11 by loyso@google.com, Apr 17 2018

thank you for explaining this! could you describe the problem at present so I can file a new task? I guess, we should support Ext4-quota management and gids to clear arc++ caches for logged-out users properly.

Comment 12 by uekawa@google.com, Apr 17 2018

Action plan would be something like 
 - migrate to use ext4 quota to track storage usage per cache type.
 - migrate existing files and codebase to use gid (or project?) instead of xattr to track cache type to take advantage of ext4 quota features.
 - remove the xattr scanning code path from cache management as a final cleanup step.


Comment 13 by loyso@google.com, Apr 17 2018

Mergedinto: 833730
Status: Duplicate (was: Untriaged)
Closing this issue in favor of the 833730 (per comment #10)

Comment 14 by loyso@google.com, Apr 17 2018

Note: We might need to backport any kernel patches for Ext4 quota.

Sign in to add a comment