From 29cf434a35d82529f56c085c9cd50858c148d086 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Dec 2006 01:31:45 +0000 Subject: [PATCH] * The determination of the root set should be made by the privileged process, so forward the operation. * Spam the user about GC misconfigurations (NIX-71). * findRoots: skip all roots that are unreadable - the warnings with which we spam the user should be enough. --- src/libstore/gc.cc | 124 +++++++++++++++++--------------- src/libstore/local-store.hh | 2 + src/libstore/remote-store.cc | 40 +++++++++-- src/libstore/remote-store.hh | 2 + src/libstore/store-api.hh | 9 +++ src/libstore/worker-protocol.hh | 11 +++ src/libutil/types.hh | 1 + src/libutil/util.cc | 1 + src/nix-worker/main.cc | 29 ++++---- 9 files changed, 140 insertions(+), 79 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index c9e51f447..e5217c9b8 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -91,12 +91,6 @@ void LocalStore::addIndirectRoot(const Path & path) } -typedef std::map Roots; - - -static void findRoots(Roots & roots, bool ignoreUnreadable); - - Path addPermRoot(const Path & _storePath, const Path & _gcRoot, bool indirect, bool allowOutsideRootsDir) { @@ -122,17 +116,17 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot, createSymlink(gcRoot, storePath, false); - /* Check that the root can be found by the garbage collector. */ - Roots roots; - findRoots(roots, true); - if (roots.find(gcRoot) == roots.end()) - printMsg(lvlError, - format( - "warning: the garbage collector does not find `%1%' as a root; " - "therefore, `%2%' might be removed by the garbage collector") - % gcRoot % storePath); } + /* Check that the root can be found by the garbage collector. */ + Roots roots = store->findRoots(); + if (roots.find(gcRoot) == roots.end()) + printMsg(lvlError, + format( + "warning: the garbage collector does not find `%1%' as a root; " + "therefore, `%2%' might be removed by the garbage collector") + % gcRoot % storePath); + /* Grab the global GC root, causing us to block while a GC is in progress. This prevents the set of permanent roots from increasing while a GC is in progress. */ @@ -308,58 +302,73 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds) static void findRoots(const Path & path, bool recurseSymlinks, - bool ignoreUnreadable, Roots & roots) + bool deleteStale, Roots & roots) { - struct stat st; - if (lstat(path.c_str(), &st) == -1) - throw SysError(format("statting `%1%'") % path); + try { + + struct stat st; + if (lstat(path.c_str(), &st) == -1) + throw SysError(format("statting `%1%'") % path); - printMsg(lvlVomit, format("looking at `%1%'") % path); + printMsg(lvlVomit, format("looking at `%1%'") % path); + + if (S_ISDIR(st.st_mode)) { + Strings names = readDirectory(path); + for (Strings::iterator i = names.begin(); i != names.end(); ++i) + findRoots(path + "/" + *i, recurseSymlinks, deleteStale, roots); + } + + else if (S_ISLNK(st.st_mode)) { + Path target = absPath(readLink(path), dirOf(path)); + + if (isInStore(target)) { + debug(format("found root `%1%' in `%2%'") + % target % path); + Path storePath = toStorePath(target); + if (store->isValidPath(storePath)) + roots[path] = storePath; + else + printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") + % path % storePath); + } + + else if (recurseSymlinks) { + if (pathExists(target)) + findRoots(target, false, deleteStale, roots); + else if (deleteStale) { + printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target); + /* Note that we only delete when recursing, i.e., + when we are still in the `gcroots' tree. We + never delete stuff outside that tree. */ + unlink(path.c_str()); + } + } + } - if (S_ISDIR(st.st_mode)) { - Strings names = readDirectory(path); - for (Strings::iterator i = names.begin(); i != names.end(); ++i) - findRoots(path + "/" + *i, recurseSymlinks, ignoreUnreadable, roots); } - else if (S_ISLNK(st.st_mode)) { - Path target = absPath(readLink(path), dirOf(path)); - - if (isInStore(target)) { - debug(format("found root `%1%' in `%2%'") - % target % path); - Path storePath = toStorePath(target); - if (store->isValidPath(storePath)) - roots[path] = storePath; - else - printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") - % path % storePath); - } - - else if (recurseSymlinks) { - struct stat st2; - if (lstat(target.c_str(), &st2) == 0) - findRoots(target, false, ignoreUnreadable, roots); - else if (ignoreUnreadable && errno == EACCES) - /* ignore */ ; - else if (errno == ENOENT || errno == ENOTDIR) { - printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target); - /* Note that we only delete when recursing, i.e., when - we are still in the `gcroots' tree. We never - delete stuff outside that tree. */ - unlink(path.c_str()); - } - else - throw SysError(format("statting `%1%'") % target); - } + catch (SysError & e) { + /* We only ignore permanent failures. */ + if (e.errNo == EACCES || e.errNo == ENOENT || e.errNo == ENOTDIR) + printMsg(lvlInfo, format("cannot read potential root `%1%'") % path); + else + throw; } } -static void findRoots(Roots & roots, bool ignoreUnreadable) +static Roots findRoots(bool deleteStale) { + Roots roots; Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str()); - findRoots(rootsDir, true, ignoreUnreadable, roots); + findRoots(rootsDir, true, deleteStale, roots); + return roots; +} + + +Roots LocalStore::findRoots() +{ + return nix::findRoots(false); } @@ -437,8 +446,7 @@ void collectGarbage(GCAction action, const PathSet & pathsToDelete, /* Find the roots. Since we've grabbed the GC lock, the set of permanent roots cannot increase now. */ - Roots rootMap; - findRoots(rootMap, false); + Roots rootMap = ignoreLiveness ? Roots() : findRoots(true); PathSet roots; for (Roots::iterator i = rootMap.begin(); i != rootMap.end(); ++i) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 251f8922f..a7b3926cc 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -64,6 +64,8 @@ public: void addIndirectRoot(const Path & path); void syncWithGC(); + + Roots findRoots(); }; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8dd87d046..6ddbb3e2a 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -18,6 +18,23 @@ namespace nix { +Path readStorePath(Source & from) +{ + Path path = readString(from); + assertStorePath(path); + return path; +} + + +PathSet readStorePaths(Source & from) +{ + PathSet paths = readStringSet(from); + for (PathSet::iterator i = paths.begin(); i != paths.end(); ++i) + assertStorePath(*i); + return paths; +} + + RemoteStore::RemoteStore() { string remoteMode = getEnv("NIX_REMOTE"); @@ -179,7 +196,7 @@ void RemoteStore::queryReferences(const Path & path, writeInt(wopQueryReferences, to); writeString(path, to); processStderr(); - PathSet references2 = readStringSet(from); + PathSet references2 = readStorePaths(from); references.insert(references2.begin(), references2.end()); } @@ -190,7 +207,7 @@ void RemoteStore::queryReferrers(const Path & path, writeInt(wopQueryReferrers, to); writeString(path, to); processStderr(); - PathSet referrers2 = readStringSet(from); + PathSet referrers2 = readStorePaths(from); referrers.insert(referrers2.begin(), referrers2.end()); } @@ -207,7 +224,7 @@ Path RemoteStore::addToStore(const Path & _srcPath, bool fixed, writeString(hashAlgo, to); dumpPath(srcPath, to); processStderr(); - Path path = readString(from); + Path path = readStorePath(from); return path; } @@ -221,7 +238,7 @@ Path RemoteStore::addTextToStore(const string & suffix, const string & s, writeStringSet(references, to); processStderr(); - Path path = readString(from); + Path path = readStorePath(from); return path; } @@ -270,6 +287,21 @@ void RemoteStore::syncWithGC() } +Roots RemoteStore::findRoots() +{ + writeInt(wopFindRoots, to); + processStderr(); + unsigned int count = readInt(from); + Roots result; + while (count--) { + Path link = readString(from); + Path target = readStorePath(from); + result[link] = target; + } + return result; +} + + void RemoteStore::processStderr() { unsigned int msg; diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 4b6983045..11e7e328b 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -53,6 +53,8 @@ public: void syncWithGC(); + Roots findRoots(); + private: AutoCloseFD fdSocket; FdSink to; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 795488d17..bda232d3d 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -2,6 +2,7 @@ #define __STOREAPI_H #include +#include #include @@ -33,6 +34,9 @@ struct Substitute typedef list Substitutes; +typedef std::map Roots; + + class StoreAPI { public: @@ -118,6 +122,11 @@ public: In either case the permanent root is seen by the collector. */ virtual void syncWithGC() = 0; + /* Find the roots of the garbage collector. Each root is a pair + (link, storepath) where `link' is the path of the symlink + outside of the Nix store that point to `storePath'. */ + virtual Roots findRoots() = 0; + }; diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index ff47fdd67..bb5d9d7b3 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -2,6 +2,9 @@ #define __WORKER_PROTOCOL_H +namespace nix { + + #define WORKER_MAGIC_1 0x6e697864 #define WORKER_MAGIC_2 0x6478696e @@ -21,6 +24,7 @@ typedef enum { wopAddTempRoot, wopAddIndirectRoot, wopSyncWithGC, + wopFindRoots, } WorkerOp; @@ -34,4 +38,11 @@ typedef enum { #define DEFAULT_SOCKET_PATH "/daemon.socket" +Path readStorePath(Source & from); +PathSet readStorePaths(Source & from); + + +} + + #endif /* !__WORKER_PROTOCOL_H */ diff --git a/src/libutil/types.hh b/src/libutil/types.hh index 257871a82..513aae039 100644 --- a/src/libutil/types.hh +++ b/src/libutil/types.hh @@ -34,6 +34,7 @@ public: class SysError : public Error { public: + int errNo; SysError(const format & f); }; diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 08385e5d9..ae5fe821e 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -35,6 +35,7 @@ Error & Error::addPrefix(const format & f) SysError::SysError(const format & f) : Error(format("%1%: %2%") % f.str() % strerror(errno)) + , errNo(errno) { } diff --git a/src/nix-worker/main.cc b/src/nix-worker/main.cc index 80db782e9..e772421f3 100644 --- a/src/nix-worker/main.cc +++ b/src/nix-worker/main.cc @@ -23,23 +23,6 @@ using namespace nix; #endif -static Path readStorePath(Source & from) -{ - Path path = readString(from); - assertStorePath(path); - return path; -} - - -static PathSet readStorePaths(Source & from) -{ - PathSet paths = readStringSet(from); - for (PathSet::iterator i = paths.begin(); i != paths.end(); ++i) - assertStorePath(*i); - return paths; -} - - static FdSource from(STDIN_FILENO); static FdSink to(STDOUT_FILENO); @@ -286,6 +269,18 @@ static void performOp(Source & from, Sink & to, unsigned int op) break; } + case wopFindRoots: { + startWork(); + Roots roots = store->findRoots(); + stopWork(); + writeInt(roots.size(), to); + for (Roots::iterator i = roots.begin(); i != roots.end(); ++i) { + writeString(i->first, to); + writeString(i->second, to); + } + break; + } + default: throw Error(format("invalid operation %1%") % op); }