diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 8f5ce0994..e4c7d072d 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -653,7 +653,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) results.paths.insert(path); uint64_t bytesFreed; - deleteGCPath(realPath, bytesFreed); + deleteStorePath(realPath, bytesFreed); results.bytesFreed += bytesFreed; diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index e8ca8074e..30008de67 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -182,15 +182,38 @@ void LocalOverlayStore::registerValidPaths(const ValidPathInfos & infos) } -void LocalOverlayStore::deleteGCPath(const Path & path, uint64_t & bytesFreed) +void LocalOverlayStore::collectGarbage(const GCOptions & options, GCResults & results) +{ + LocalStore::collectGarbage(options, results); + + remountIfNecessary(); +} + + +void LocalOverlayStore::deleteStorePath(const Path & path, uint64_t & bytesFreed) { auto mergedDir = realStoreDir.get() + "/"; if (path.substr(0, mergedDir.length()) != mergedDir) { warn("local-overlay: unexpected gc path '%s' ", path); return; } - if (pathExists(toUpperPath({path.substr(mergedDir.length())}))) { - LocalStore::deleteGCPath(path, bytesFreed); + + StorePath storePath = {path.substr(mergedDir.length())}; + auto upperPath = toUpperPath(storePath); + + if (pathExists(upperPath)) { + debug("upper exists: %s", path); + if (lowerStore->isValidPath(storePath)) { + debug("lower exists: %s", storePath.to_string()); + // Path also exists in lower store. + // We must delete via upper layer to avoid creating a whiteout. + deletePath(upperPath, bytesFreed); + _remountRequired = true; + } else { + // Path does not exist in lower store. + // So we can delete via overlayfs and not need to remount. + LocalStore::deleteStorePath(path, bytesFreed); + } } } @@ -208,14 +231,18 @@ void LocalOverlayStore::optimiseStore() for (auto & path : paths) { if (lowerStore->isValidPath(path)) { + uint64_t bytesFreed = 0; // Deduplicate store path - deletePath(toUpperPath(path)); + deleteStorePath(Store::toRealPath(path), bytesFreed); } done++; act.progress(done, paths.size()); } + + remountIfNecessary(); } + Path LocalOverlayStore::toRealPathForHardLink(const StorePath & path) { return lowerStore->isValidPath(path) @@ -224,6 +251,20 @@ Path LocalOverlayStore::toRealPathForHardLink(const StorePath & path) } +void LocalOverlayStore::remountIfNecessary() +{ + if (!_remountRequired) return; + + if (remountHook.get().empty()) { + warn("'%s' needs remounting, set remount-hook to do this automatically", realStoreDir.get()); + } else { + runProgram(remountHook, false, {realStoreDir}); + } + + _remountRequired = false; +} + + static RegisterStoreImplementation regLocalOverlayStore; } diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh index 21658c6ab..1c1d54a8f 100644 --- a/src/libstore/local-overlay-store.hh +++ b/src/libstore/local-overlay-store.hh @@ -41,6 +41,13 @@ struct LocalOverlayStoreConfig : virtual LocalStoreConfig default, but can be disabled if needed. )"}; + const PathSetting remountHook{(StoreConfig*) this, "", "remount-hook", + R"( + Script or program to run when overlay filesystem needs remounting. + + TODO: Document this in more detail. + )"}; + const std::string name() override { return "Experimental Local Overlay Store"; } std::string doc() override @@ -111,7 +118,9 @@ private: void queryRealisationUncached(const DrvOutput&, Callback> callback) noexcept override; - void deleteGCPath(const Path & path, uint64_t & bytesFreed) override; + void collectGarbage(const GCOptions & options, GCResults & results) override; + + void deleteStorePath(const Path & path, uint64_t & bytesFreed) override; void optimiseStore() override; @@ -125,6 +134,10 @@ private: * Deletion only effects the upper layer, so we ignore lower-layer referrers. */ void queryGCReferrers(const StorePath & path, StorePathSet & referrers) override; + + void remountIfNecessary(); + + std::atomic_bool _remountRequired = false; }; } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 21acb3c38..2c18867f5 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -457,7 +457,7 @@ AutoCloseFD LocalStore::openGCLock() } -void LocalStore::deleteGCPath(const Path & path, uint64_t & bytesFreed) +void LocalStore::deleteStorePath(const Path & path, uint64_t & bytesFreed) { deletePath(path, bytesFreed); } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index c049de02e..514ac256b 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -247,7 +247,7 @@ public: * The default implementation simply calls `deletePath`, but it can be * overridden by stores that wish to provide their own deletion behaviour. */ - virtual void deleteGCPath(const Path & path, uint64_t & bytesFreed); + virtual void deleteStorePath(const Path & path, uint64_t & bytesFreed); /** * Optimise the disk space usage of the Nix store by hard-linking diff --git a/tests/overlay-local-store/delete-duplicate-inner.sh b/tests/overlay-local-store/delete-duplicate-inner.sh new file mode 100644 index 000000000..6053451d8 --- /dev/null +++ b/tests/overlay-local-store/delete-duplicate-inner.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash + +set -eu -o pipefail + +set -x + +source common.sh + +# Avoid store dir being inside sandbox build-dir +unset NIX_STORE_DIR +unset NIX_STATE_DIR + +storeDirs + +initLowerStore + +mountOverlayfs + +# Add to overlay before lower to ensure file is duplicated +upperPath=$(nix-store --store "$storeB" --add delete-duplicate.sh) +lowerPath=$(nix-store --store "$storeA" --add delete-duplicate.sh) +[[ "$upperPath" = "$lowerPath" ]] + +# Check there really are two files with different inodes +upperInode=$(stat -c %i "$storeBRoot/$upperPath") +lowerInode=$(stat -c %i "$storeA/$lowerPath") +[[ "$upperInode" != "$lowerInode" ]] + +# Now delete file via the overlay store +nix-store --store "$storeB&remount-hook=$PWD/remount.sh" --delete "$upperPath" + +# Check there is no longer a file in upper layer +expect 1 stat "$storeBTop/${upperPath##/nix/store/}" + +# Check that overlay file is now the one in lower layer +upperInode=$(stat -c %i "$storeBRoot/$upperPath") +lowerInode=$(stat -c %i "$storeA/$lowerPath") +[[ "$upperInode" = "$lowerInode" ]] diff --git a/tests/overlay-local-store/delete-duplicate.sh b/tests/overlay-local-store/delete-duplicate.sh new file mode 100644 index 000000000..0c0b1a3b2 --- /dev/null +++ b/tests/overlay-local-store/delete-duplicate.sh @@ -0,0 +1,5 @@ +source common.sh + +requireEnvironment +setupConfig +execUnshare ./delete-duplicate-inner.sh diff --git a/tests/overlay-local-store/local.mk b/tests/overlay-local-store/local.mk index 8bfb22ae7..19ae6a3d0 100644 --- a/tests/overlay-local-store/local.mk +++ b/tests/overlay-local-store/local.mk @@ -5,6 +5,7 @@ overlay-local-store-tests := \ $(d)/bad-uris.sh \ $(d)/add-lower.sh \ $(d)/delete-refs.sh \ + $(d)/delete-duplicate.sh \ $(d)/gc.sh \ $(d)/verify.sh \ $(d)/optimise.sh diff --git a/tests/overlay-local-store/remount.sh b/tests/overlay-local-store/remount.sh new file mode 100755 index 000000000..0b06debb5 --- /dev/null +++ b/tests/overlay-local-store/remount.sh @@ -0,0 +1,2 @@ +#!/bin/sh +mount -o remount "$1"