Mark removable Drive caches for cryptohome.

components/drive/chromeos/file_cache.cc

Index: components/drive/chromeos/file_cache.cc
diff --git a/components/drive/chromeos/file_cache.cc b/components/drive/chromeos/file_cache.cc
index a2eb6eab02abacd59ef7fc3b50e6c9869f1bf6d2..b1003b56ca2a553b63e2662ff1418807ddc8f2af 100644
--- a/components/drive/chromeos/file_cache.cc
+++ b/components/drive/chromeos/file_cache.cc
@@ -4,7 +4,9 @@
 
 #include "components/drive/chromeos/file_cache.h"
 
-#include <unistd.h>
+#include <linux/fs.h>
+#include <sys/ioctl.h>
+#include <sys/xattr.h>
 
 #include <queue>
 #include <vector>
@@ -12,11 +14,13 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
+#include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
@@ -45,6 +49,72 @@ base::FilePath GetPathForId(const base::FilePath& cache_directory,
       base::FilePath::FromUTF8Unsafe(util::EscapeCacheFileName(id)));
 }
 
+// Sets extended file attribute as |name| |value| pair.
+bool SetExtendedFileAttributes(const base::FilePath& path,
+    const std::string& name, const std::string& value) {
+  return setxattr(path.value().c_str(), name.c_str(), value.c_str(),
+      value.size() + 1, 0) == 0;
+}
+
+// Changes attributes of the file with |flags|, e.g. FS_NODUMP_FL (cryptohome
+// will remove Drive caches with this attribute).
+// See linux/fs.h for available flags, and chattr(1) which does similar thing.
+// Returns whether operation succeeded.
+bool SetFileAttributes(const base::FilePath& path, int64_t flags) {
+  base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  if (!file.IsValid()) {
+    PLOG(ERROR) << "Failed to open file: " << path.value();
+    return false;
+  }
+  long f = static_cast<long>(flags);  // NOLINT long
+  if (ioctl(file.GetPlatformFile(), FS_IOC_SETFLAGS, &f) < 0) {
+    PLOG(ERROR) << "ioctl: " << path.value();
+    return false;
+  }
+  return true;
+}
+
+// Gets file attributes similarly to lsattr(1). Returns flags or -1 on error.
+// See linux/fs.h for the definition of the returned flags e.g. FS_NODUMP_FL.
+int64_t GetFileAttributes(const base::FilePath& path) {
+  base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  if (!file.IsValid()) {
+    PLOG(ERROR) << "Failed to open file: " << path.value();
+    return -1;
+  }
+  long flags = 0;  // NOLINT long
+  if (ioctl(file.GetPlatformFile(), FS_IOC_GETFLAGS, &flags) < 0) {
+    PLOG(ERROR) << "ioctl: " << path.value();
+    return -1;
+  }
+  return static_cast<int64_t>(flags);
+}
+
+// Marks the cache file to be removable by cryptohome.
+bool SetRemovable(const base::FilePath& path) {
+  int64_t flags = GetFileAttributes(path);
+  if (flags < 0) return false;
+  if ((flags & FS_NODUMP_FL) == FS_NODUMP_FL) return true;
+
+  return SetFileAttributes(path, flags | FS_NODUMP_FL);
+}
+
+// Marks the cache file to be unremovable by cryptohome.
+bool UnsetRemovable(const base::FilePath& path) {
+  int64_t flags = GetFileAttributes(path);
+  if (flags < 0) return false;
+  if ((flags & FS_NODUMP_FL) == 0) return true;
+
+  return SetFileAttributes(path, flags & ~(static_cast<int64_t>(FS_NODUMP_FL)));
+}
+
+// Marks |path| as drive cache dir.
+// Returns if the operation succeeded.
+bool MarkAsDriveCacheDir(const base::FilePath& path) {
+  return SetRemovable(path)
+      && SetExtendedFileAttributes(path, FileCache::kGCacheFilesAttribute, "");
+}
+
 typedef std::pair<base::File::Info, ResourceEntry> CacheInfo;
 
 class CacheInfoLatestCompare {
@@ -58,6 +128,9 @@ const size_t kMaxNumOfEvictedCacheFiles = 30000;
 
 }  // namespace
 
+// static
+const char FileCache::kGCacheFilesAttribute[] = "user.GCacheFiles";
+
 FileCache::FileCache(ResourceMetadataStorage* storage,
                      const base::FilePath& cache_file_directory,
                      base::SequencedTaskRunner* blocking_task_runner,
@@ -282,6 +355,15 @@ FileError FileCache::Store(const std::string& id,
   cache_state->set_is_present(true);
   if (md5.empty())
     cache_state->set_is_dirty(true);
+
+  if (!cache_state->is_pinned() && !cache_state->is_dirty()) {
+    if (!SetRemovable(dest_path))
+      return FILE_ERROR_FAILED;
+  } else {
+    if (!UnsetRemovable(dest_path))
+      return FILE_ERROR_FAILED;
+  }
+
   return storage_->PutEntry(entry);
 }
 
@@ -292,8 +374,17 @@ FileError FileCache::Pin(const std::string& id) {
   FileError error = storage_->GetEntry(id, &entry);
   if (error != FILE_ERROR_OK)
     return error;
+
   entry.mutable_file_specific_info()->mutable_cache_state()->set_is_pinned(
       true);
+
+  base::FilePath file_path = GetCacheFilePath(entry.local_id());
+  // Cache file can be created later.
+  if (entry.file_specific_info().cache_state().is_present()) {
+    if (!UnsetRemovable(file_path))
+      return FILE_ERROR_FAILED;
+  }
+
   return storage_->PutEntry(entry);
 }
 
@@ -310,6 +401,10 @@ FileError FileCache::Unpin(const std::string& id) {
   if (entry.file_specific_info().cache_state().is_present()) {
     entry.mutable_file_specific_info()->mutable_cache_state()->set_is_pinned(
         false);
+    if (!entry.file_specific_info().cache_state().is_dirty()) {
+      if (!SetRemovable(GetCacheFilePath(entry.local_id())))
+        return FILE_ERROR_FAILED;
+    }
   } else {
     // Remove the existing entry if we are unpinning a non-present file.
     entry.mutable_file_specific_info()->clear_cache_state();
@@ -376,6 +471,9 @@ FileError FileCache::OpenForWrite(
   }
 
   entry.mutable_file_specific_info()->mutable_cache_state()->set_is_dirty(true);
+  if (!UnsetRemovable(GetCacheFilePath(entry.local_id())))
+    return FILE_ERROR_FAILED;
+
   entry.mutable_file_specific_info()->mutable_cache_state()->clear_md5();
   error = storage_->PutEntry(entry);
   if (error != FILE_ERROR_OK)
@@ -447,6 +545,11 @@ FileError FileCache::ClearDirty(const std::string& id) {
 
   entry.mutable_file_specific_info()->mutable_cache_state()->set_is_dirty(
       false);
+  if (!entry.file_specific_info().cache_state().is_pinned()) {
+    if (!SetRemovable(GetCacheFilePath(entry.local_id())))
+      return FILE_ERROR_FAILED;
+  }
+
   return storage_->PutEntry(entry);
 }
 
@@ -513,6 +616,13 @@ bool FileCache::Initialize() {
 
   if (!RenameCacheFilesToNewFormat())
     return false;
+
+  // Run this every time to resolve inconsistency between metadata
+  // and file attributes which possibly occurs on abrupt power failure.
+  if (!MigrateCacheFilesResolvingInconsistency()) {
+    return false;
+  }
+
   return true;
 }
 
@@ -684,47 +794,51 @@ bool FileCache::RenameCacheFilesToNewFormat() {
   return true;
 }
 
-// static
-bool FileCache::MigrateCacheFiles(const base::FilePath& from,
-                                  const base::FilePath& to_files,
-                                  const base::FilePath& to_links,
-                                  ResourceMetadataStorage* metadata_storage) {
+bool FileCache::MigrateCacheFilesResolvingInconsistency() {
   std::unique_ptr<ResourceMetadataStorage::Iterator> it =
-      metadata_storage->GetIterator();
+      storage_->GetIterator();
+
   for (; !it->IsAtEnd(); it->Advance()) {
-    const ResourceEntry& entry = it->GetValue();
-    if (!entry.file_specific_info().cache_state().is_present()) {
-      continue;
-    }
+    ResourceEntry entry = it->GetValue();
+    FileCacheEntry* file_cache_entry =
+        entry.mutable_file_specific_info()->mutable_cache_state();
 
-    // Ignore missing cache file case since it never succeeds.
-    // TODO(yawano): handle this case at metadata validation in FileCache.
-    const base::FilePath move_from = GetPathForId(from, entry.local_id());
-    if (!base::PathExists(move_from)) {
-      continue;
-    }
+    const base::FilePath filepath = GetPathForId(cache_file_directory_,
+        entry.local_id());
+    const bool file_is_present = base::PathExists(filepath);
+
+    // Delete file if the file is present but metadata says not.
+    // It happens only on abrupt shutdown.
+    if (file_is_present && !file_cache_entry->is_present()) {
+      LOG(WARNING) << "File is present but metadata's state was inconsistent.";
 
-    // Create hard link to cache file if it's pinned or dirty. cryptohome will
-    // not delete a cache file if there is a hard link to it.
-    const FileCacheEntry& file_cache_entry =
-        entry.file_specific_info().cache_state();
-    if (file_cache_entry.is_pinned() || file_cache_entry.is_dirty()) {
-      const base::FilePath link_path = GetPathForId(to_links, entry.local_id());
-      int link_result = link(move_from.AsUTF8Unsafe().c_str(),
-                             link_path.AsUTF8Unsafe().c_str());
-      if (link_result != 0 && errno != EEXIST) {
+      if (!base::DeleteFile(filepath, false /* recursive */))

[hashimoto, 2016/05/11 06:29:12]

Sorry for not saying this earlier, but I realized that this code is not needed
as FreeDiskSpaceIfNeededFor() is already deleting files without corresponding
metadata entries.
FreeDiskSpaceIfNeededFor() uses base::FileEnumerator so it also deletes files
which cannot be found with the storage iterator.

[oka, 2016/05/11 09:35:04]

Discussed offline. Keeping this logic will be OK, since 1.
FreeDiskSpaceIfNeededFor do nothing if there's enough space. 2. We do nothing
special to detect this case.

         return false;
+      continue;
+    }
+    if (!file_is_present) {
+      // Update metatadata if there is no file but metadata says there is.
+      // It happens when cryptohome removed the file.
+      if (file_cache_entry->is_present()) {
+        file_cache_entry->set_is_present(false);
+        file_cache_entry->set_is_pinned(false);
+        file_cache_entry->set_is_dirty(false);
+        file_cache_entry->clear_md5();
+        if (storage_->PutEntry(entry) != FILE_ERROR_OK)
+          return false;
       }
+      continue;
     }
 
-    // Move cache file.
-    const base::FilePath move_to = GetPathForId(to_files, entry.local_id());
-    if (!base::Move(move_from, move_to)) {
-      return false;
+    // Update file attribues for cryptohome.

[hashimoto, 2016/05/11 06:29:12]

As the proposed code is using "continue" in many places, it's a bit difficult to
figure out this line can be reached only when file_is_present &&
file_cache_entry->is_present().

How about structuring the code like this without using "continue"?

if (base::PathExists()) {
  if (file_cache_entry->is_present()) {
    (set attributes)
  } else {
    (remove file)
  }
} else {
  if (file_cache_entry->is_present()) {
    (update metadata)
  }
}

[oka, 2016/05/11 09:35:04]

Done.

+    if (file_cache_entry->is_pinned() || file_cache_entry->is_dirty()) {
+      if (!UnsetRemovable(filepath)) return false;
+    } else {
+      if (!SetRemovable(filepath)) return false;
     }
   }
 
-  return true;
+  return MarkAsDriveCacheDir(cache_file_directory_);
 }
 
 void FileCache::CloseForWrite(const std::string& id) {