From e9a4abdb5d6fe8e128372a77d879b0187b1bacfe Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Tue, 11 Jan 2022 11:57:45 +0100 Subject: Make --repair-path also repair corrupt optimised links There already existed a smoke test for the link content length, but it appears that there exists some corruptions pernicious enough to replace the file content with zeros, and keeping the same length. --repair-path now goes as far as checking the content of the link, making it true to its name and actually repairing the path for such coruption cases. --- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/local-store.cc | 6 +++--- src/libstore/local-store.hh | 7 ++++--- src/libstore/optimise-store.cc | 31 ++++++++++++++++++----------- src/libutil/logging.hh | 2 +- 5 files changed, 28 insertions(+), 20 deletions(-) (limited to 'src') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 66d950b42..9ad28c6a9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2460,7 +2460,7 @@ void LocalDerivationGoal::registerOutputs() } if (curRound == nrRounds) { - localStore.optimisePath(actualPath); // FIXME: combine with scanForReferences() + localStore.optimisePath(actualPath, false); // FIXME: combine with scanForReferences() worker.markContentsGood(newInfo.path); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 1b8e77ead..9ebdfd6ed 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1307,7 +1307,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, canonicalisePathMetaData(realPath, -1); - optimisePath(realPath); // FIXME: combine with hashPath() + optimisePath(realPath, repair); // FIXME: combine with hashPath() registerValidPath(info); } @@ -1419,7 +1419,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name, canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath - optimisePath(realPath); + optimisePath(realPath, repair); ValidPathInfo info { dstPath, narHash.first }; info.narSize = narHash.second; @@ -1463,7 +1463,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, dumpString(s, sink); auto narHash = hashString(htSHA256, *sink.s); - optimisePath(realPath); + optimisePath(realPath, repair); ValidPathInfo info { dstPath, narHash }; info.narSize = sink.s->size(); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 115ea046a..04698ba07 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -172,8 +172,9 @@ public: void optimiseStore() override; - /* Optimise a single store path. */ - void optimisePath(const Path & path); + /* Optimise a single store path. Optionally, test the encountered + symlinks for corruption. */ + void optimisePath(const Path & path, bool repair); bool verifyStore(bool checkContents, RepairFlag repair) override; @@ -253,7 +254,7 @@ private: InodeHash loadInodeHash(); Strings readDirectoryIgnoringInodes(const Path & path, const InodeHash & inodeHash); - void optimisePath_(Activity * act, OptimiseStats & stats, const Path & path, InodeHash & inodeHash); + void optimisePath_(Activity * act, OptimiseStats & stats, const Path & path, InodeHash & inodeHash, bool repair); // Internal versions that are not wrapped in retry_sqlite. bool isValidPath_(State & state, const StorePath & path); diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index d95e54af1..c93d0706d 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -88,7 +88,7 @@ Strings LocalStore::readDirectoryIgnoringInodes(const Path & path, const InodeHa void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, - const Path & path, InodeHash & inodeHash) + const Path & path, InodeHash & inodeHash, bool repair) { checkInterrupt(); @@ -110,7 +110,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, if (S_ISDIR(st.st_mode)) { Strings names = readDirectoryIgnoringInodes(path, inodeHash); for (auto & i : names) - optimisePath_(act, stats, path + "/" + i, inodeHash); + optimisePath_(act, stats, path + "/" + i, inodeHash, repair); return; } @@ -151,7 +151,20 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, /* Check if this is a known hash. */ Path linkPath = linksDir + "/" + hash.to_string(Base32, false); - retry: + /* Maybe delete the link, if it has been corrupted. */ + if (pathExists(linkPath)) { + auto stLink = lstat(linkPath); + if (st.st_size != stLink.st_size + || (repair && hash != hashPath(htSHA256, linkPath).first)) + { + // XXX: Consider overwriting linkPath with our valid version. + warn("removing corrupted link '%s'", linkPath); + warn("There may be more corrupted paths." + "\nYou should run `nix-store --verify --check-contents --repair` to fix them all"); + unlink(linkPath.c_str()); + } + } + if (!pathExists(linkPath)) { /* Nope, create a hard link in the links directory. */ if (link(path.c_str(), linkPath.c_str()) == 0) { @@ -187,12 +200,6 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, return; } - if (st.st_size != stLink.st_size) { - warn("removing corrupted link '%s'", linkPath); - unlink(linkPath.c_str()); - goto retry; - } - printMsg(lvlTalkative, format("linking '%1%' to '%2%'") % path % linkPath); /* Make the containing directory writable, but only if it's not @@ -260,7 +267,7 @@ void LocalStore::optimiseStore(OptimiseStats & stats) if (!isValidPath(i)) continue; /* path was GC'ed, probably */ { Activity act(*logger, lvlTalkative, actUnknown, fmt("optimising path '%s'", printStorePath(i))); - optimisePath_(&act, stats, realStoreDir + "/" + std::string(i.to_string()), inodeHash); + optimisePath_(&act, stats, realStoreDir + "/" + std::string(i.to_string()), inodeHash, false); } done++; act.progress(done, paths.size()); @@ -278,12 +285,12 @@ void LocalStore::optimiseStore() stats.filesLinked); } -void LocalStore::optimisePath(const Path & path) +void LocalStore::optimisePath(const Path & path, bool repair) { OptimiseStats stats; InodeHash inodeHash; - if (settings.autoOptimiseStore) optimisePath_(nullptr, stats, path, inodeHash); + if (settings.autoOptimiseStore) optimisePath_(nullptr, stats, path, inodeHash, repair); } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index ce9c3dfed..5560d2bed 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -40,7 +40,7 @@ struct LoggerSettings : Config Setting showTrace{ this, false, "show-trace", R"( - Where Nix should print out a stack trace in case of Nix + Whether Nix should print out a stack trace in case of Nix expression evaluation errors. )"}; }; -- cgit v1.2.3