Merge branch 'overlayfs-store' into delete

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;
 

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<LocalOverlayStore, LocalOverlayStoreConfig> regLocalOverlayStore;
 
 }

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<std::shared_ptr<const Realisation>> 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;
 };
 
 }

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);
 }

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

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" ]]

tests/overlay-local-store/delete-duplicate.sh

@@ -0,0 +1,5 @@
+source common.sh
+
+requireEnvironment
+setupConfig
+execUnshare ./delete-duplicate-inner.sh

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

tests/overlay-local-store/remount.sh

@@ -0,0 +1,2 @@
+#!/bin/sh
+mount -o remount "$1"