Chris Marusich <cmmarus...@gmail.com> writes: > Ludovic Courtès <l...@gnu.org> writes: > >>> Should Guix do anything about this? We could change guix-daemon to take >>> correct action in the face of an XDEV error. We could also improve the >>> logging, since currently it silently swallows the XDEV error. >> >> I guess we could delete recursively right away upon EXDEV. It should be >> just two lines of code, right? > > I'll try making the change and report back. Yes, there are other cases > where we immediately delete without moving into the trash directory > (e.g., when the trash directory fails to be created), so it seems OK.
Here is a patch. Turns out it's was just a one line change! If nobody has any further feedback on it, I'll go ahead and merge it to the master branch in the next couple days. I tested it in one of the Docker containers provided by Stephen which exhibited the problem. I built the new Guix inside the Docker container and verified that a path which was previously unable to be GC'd due to the EXDEV error, was now able to be successfully GC'd. My understanding is that the only reason why the guix-daemon attempts to move dead directories to the trash directory is to save time on deleting, since large directories could take a while to fully delete. If there is any reason why it might be unsafe to delete the directories directly in case of EXDEV (I cannot think of any), please let me know. -- Chris
From 505481a6a22819a42320f693988c3f8e13ded080 Mon Sep 17 00:00:00 2001 From: Chris Marusich <cmmarus...@gmail.com> Date: Thu, 4 Jun 2020 23:26:19 -0700 Subject: [PATCH] daemon: Handle EXDEV when moving to trash directory. Fixes <https://bugs.gnu.org/41607>. Reported by Stephen Scheck <singularsyn...@gmail.com>. * nix/libstore/gc.cc (LocalStore::deletePathRecursive): When we try to move a dead directory into the trashDir using rename(2) but it returns an EXDEV error, just delete the directory instead. This can happen in a Docker container when the directory is not on the "top layer". --- nix/libstore/gc.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc index 8bc4e01eb0..845fe278c7 100644 --- a/nix/libstore/gc.cc +++ b/nix/libstore/gc.cc @@ -455,7 +455,10 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path) throw SysError(format("unable to rename `%1%' to `%2%'") % path % tmp); state.bytesInvalidated += size; } catch (SysError & e) { - if (e.errNo == ENOSPC) { + // In a Docker container, rename(2) returns EXDEV when the source + // and destination are not both on the "top layer". See: + // https://bugs.gnu.org/41607 + if (e.errNo == ENOSPC || e.errNo == EXDEV) { printMsg(lvlInfo, format("note: can't create move `%1%': %2%") % path % e.msg()); deleteGarbage(state, path); } -- 2.26.2
signature.asc
Description: PGP signature