diff --git a/ChangeLog.txt b/ChangeLog.txt index cce8200b..dac747b7 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,5 +1,8 @@ Version 0.9.4 (unreleased) --------------- +Improvements: +* Ciphertext blocks are split into subdirectories (before, all were on top level) to reduce number of files per directory. Some unix tools don't work well with directories with too many entries. + Fixed Bugs: * Renaming a file to an existing file (i.e. overwriting an existing file) didn't free the allocated memory for the overwritten file * Renaming a file to an existing file could hurt an invariant in the directory layout (directory entries have to be sorted) and doing so could cause files to seemingly disappear. diff --git a/src/blockstore/implementations/ondisk/OnDiskBlock.cpp b/src/blockstore/implementations/ondisk/OnDiskBlock.cpp index 29fe9fca..cbae87fa 100644 --- a/src/blockstore/implementations/ondisk/OnDiskBlock.cpp +++ b/src/blockstore/implementations/ondisk/OnDiskBlock.cpp @@ -56,8 +56,13 @@ void OnDiskBlock::resize(size_t newSize) { _dataChanged = true; } +bf::path OnDiskBlock::_getFilepath(const bf::path &rootdir, const Key &key) { + string keyStr = key.ToString(); + return rootdir / keyStr.substr(0,3) / keyStr.substr(3); +} + optional> OnDiskBlock::LoadFromDisk(const bf::path &rootdir, const Key &key) { - auto filepath = rootdir / key.ToString(); + auto filepath = _getFilepath(rootdir, key); try { boost::optional data = _loadFromDisk(filepath); if (data == none) { @@ -70,7 +75,8 @@ optional> OnDiskBlock::LoadFromDisk(const bf::path &root } optional> OnDiskBlock::CreateOnDisk(const bf::path &rootdir, const Key &key, Data data) { - auto filepath = rootdir / key.ToString(); + auto filepath = _getFilepath(rootdir, key); + bf::create_directory(filepath.parent_path()); if (bf::exists(filepath)) { return none; } @@ -81,12 +87,15 @@ optional> OnDiskBlock::CreateOnDisk(const bf::path &root } void OnDiskBlock::RemoveFromDisk(const bf::path &rootdir, const Key &key) { - auto filepath = rootdir / key.ToString(); + auto filepath = _getFilepath(rootdir, key); ASSERT(bf::is_regular_file(filepath), "Block not found on disk"); bool retval = bf::remove(filepath); if (!retval) { LOG(ERROR) << "Couldn't find block " << key.ToString() << " to remove"; } + if (bf::is_empty(filepath.parent_path())) { + bf::remove(filepath.parent_path()); + } } void OnDiskBlock::_storeToDisk() const { diff --git a/src/blockstore/implementations/ondisk/OnDiskBlock.h b/src/blockstore/implementations/ondisk/OnDiskBlock.h index 205984a8..09dd7297 100644 --- a/src/blockstore/implementations/ondisk/OnDiskBlock.h +++ b/src/blockstore/implementations/ondisk/OnDiskBlock.h @@ -41,6 +41,7 @@ private: static bool _isAcceptedCryfsHeader(const cpputils::Data &data); static bool _isOtherCryfsHeader(const cpputils::Data &data); static void _checkHeader(std::istream *str); + static boost::filesystem::path _getFilepath(const boost::filesystem::path &rootdir, const Key &key); const boost::filesystem::path _filepath; cpputils::Data _data; diff --git a/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp b/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp index 04679f01..538bd568 100644 --- a/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp +++ b/src/blockstore/implementations/ondisk/OnDiskBlockStore.cpp @@ -7,6 +7,7 @@ using cpputils::Data; using cpputils::unique_ref; using boost::optional; using boost::none; +using std::vector; namespace bf = boost::filesystem; @@ -22,8 +23,37 @@ OnDiskBlockStore::OnDiskBlockStore(const boost::filesystem::path &rootdir) throw std::runtime_error("Base directory is not a directory"); } //TODO Test for read access, write access, enter (x) access, and throw runtime_error in case +#ifndef CRYFS_NO_COMPATIBILITY + _migrateBlockStore(); +#endif } +#ifndef CRYFS_NO_COMPATIBILITY +void OnDiskBlockStore::_migrateBlockStore() { + vector blocksToMigrate; + for (auto entry = bf::directory_iterator(_rootdir); entry != bf::directory_iterator(); ++entry) { + if (bf::is_regular_file(entry->path()) && _isValidBlockKey(entry->path().filename().native())) { + blocksToMigrate.push_back(entry->path().filename().native()); + } + } + if (blocksToMigrate.size() != 0) { + std::cout << "Migrating CryFS filesystem..." << std::flush; + for (auto key : blocksToMigrate) { + Key::FromString(key); // Assert that it can be parsed as a key + string dir = key.substr(0, 3); + string file = key.substr(3); + bf::create_directory(dir); + bf::rename(_rootdir / key, _rootdir / dir / file); + } + std::cout << "done" << std::endl; + } +} + +bool OnDiskBlockStore::_isValidBlockKey(const string &key) { + return key.size() == 32 && key.find_first_not_of("0123456789ABCDEF") == string::npos; +} +#endif + //TODO Do I have to lock tryCreate/remove and/or load? Or does ParallelAccessBlockStore take care of that? optional> OnDiskBlockStore::tryCreate(const Key &key, Data data) { @@ -46,7 +76,11 @@ void OnDiskBlockStore::remove(unique_ref block) { } uint64_t OnDiskBlockStore::numBlocks() const { - return std::distance(bf::directory_iterator(_rootdir), bf::directory_iterator()); + uint64_t count = 0; + for (auto entry = bf::directory_iterator(_rootdir); entry != bf::directory_iterator(); ++entry) { + count += std::distance(bf::directory_iterator(entry->path()), bf::directory_iterator()); + } + return count; } uint64_t OnDiskBlockStore::estimateNumFreeBytes() const { diff --git a/src/blockstore/implementations/ondisk/OnDiskBlockStore.h b/src/blockstore/implementations/ondisk/OnDiskBlockStore.h index 21f0049b..42325d19 100644 --- a/src/blockstore/implementations/ondisk/OnDiskBlockStore.h +++ b/src/blockstore/implementations/ondisk/OnDiskBlockStore.h @@ -24,6 +24,10 @@ public: private: const boost::filesystem::path _rootdir; +#ifndef CRYFS_NO_COMPATIBILITY + void _migrateBlockStore(); + bool _isValidBlockKey(const std::string &key); +#endif DISALLOW_COPY_AND_ASSIGN(OnDiskBlockStore); }; diff --git a/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Specific.cpp b/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Specific.cpp index 28465aba..5745e104 100644 --- a/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Specific.cpp +++ b/test/blockstore/implementations/ondisk/OnDiskBlockStoreTest_Specific.cpp @@ -7,6 +7,7 @@ using ::testing::Test; using cpputils::TempDir; using cpputils::Data; using std::ifstream; +using blockstore::Key; using namespace blockstore::ondisk; @@ -23,8 +24,8 @@ public: return blockStore.create(initData)->key(); } - uint64_t getPhysicalBlockSize(const blockstore::Key &key) { - ifstream stream((baseDir.path() / key.ToString()).c_str()); + uint64_t getPhysicalBlockSize(const Key &key) { + ifstream stream((baseDir.path() / key.ToString().substr(0,3) / key.ToString().substr(3)).c_str()); stream.seekg(0, stream.end); return stream.tellg(); } @@ -56,3 +57,11 @@ TEST_F(OnDiskBlockStoreTest, PhysicalBlockSize_positive) { auto baseSize = getPhysicalBlockSize(key); EXPECT_EQ(10*1024u, blockStore.blockSizeFromPhysicalBlockSize(baseSize)); } + +TEST_F(OnDiskBlockStoreTest, NumBlocksIsCorrectAfterAddingTwoBlocksWithSameKeyPrefix) { + const Key key1 = Key::FromString("4CE72ECDD20877A12ADBF4E3927C0A13"); + const Key key2 = Key::FromString("4CE72ECDD20877A12ADBF4E3927C0A14"); + EXPECT_NE(boost::none, blockStore.tryCreate(key1, cpputils::Data(0))); + EXPECT_NE(boost::none, blockStore.tryCreate(key2, cpputils::Data(0))); + EXPECT_EQ(2u, blockStore.numBlocks()); +} diff --git a/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp b/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp index 663899df..2098fdc7 100644 --- a/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp +++ b/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp @@ -23,7 +23,7 @@ public: // Don't create the temp file yet (therefore pass false to the TempFile constructor) : dir(), key(Key::FromString("1491BB4932A389EE14BC7090AC772972")), - file(dir.path() / key.ToString(), false) { + file(dir.path() / key.ToString().substr(0,3) / key.ToString().substr(3), false) { } TempDir dir; Key key; diff --git a/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp b/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp index e49856ae..be96e3a3 100644 --- a/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp +++ b/test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp @@ -26,7 +26,7 @@ public: // Don't create the temp file yet (therefore pass false to the TempFile constructor) : dir(), key(Key::FromString("1491BB4932A389EE14BC7090AC772972")), - file(dir.path() / key.ToString(), false), + file(dir.path() / key.ToString().substr(0,3) / key.ToString().substr(3), false), randomData(DataFixture::generate(GetParam())) { } TempDir dir;