Remove setting of the immutable bit

Using the immutable bit is problematic, especially in conjunction with
store optimisation.  For instance, if the garbage collector deletes a
file, it has to clear its immutable bit, but if the file has
additional hard links, we can't set the bit afterwards because we
don't know the remaining paths.

So now that we support having the entire Nix store as a read-only
mount, we may as well drop the immutable bit.  Unfortunately, we have
to keep the code to clear the immutable bit for backwards
compatibility.
This commit is contained in:
Eelco Dolstra 2012-09-19 16:17:54 -04:00
parent b9124a5c33
commit b9c2b4d5b4
5 changed files with 34 additions and 95 deletions

View file

@ -1355,11 +1355,6 @@ void DerivationGoal::buildDone()
/* Delete the chroot (if we were using one). */ /* Delete the chroot (if we were using one). */
autoDelChroot.reset(); /* this runs the destructor */ autoDelChroot.reset(); /* this runs the destructor */
/* Deleting the chroot will have caused the immutable bits on
hard-linked inputs to be cleared. So set them again. */
foreach (PathSet::iterator, i, regularInputPaths)
makeImmutable(*i);
/* Delete redirected outputs (when doing hash rewriting). */ /* Delete redirected outputs (when doing hash rewriting). */
foreach (PathSet::iterator, i, redirectedOutputs) foreach (PathSet::iterator, i, redirectedOutputs)
deletePath(*i); deletePath(*i);
@ -1766,11 +1761,8 @@ void DerivationGoal::startBuilder()
/* Hard-linking fails if we exceed the maximum /* Hard-linking fails if we exceed the maximum
link count on a file (e.g. 32000 of ext3), link count on a file (e.g. 32000 of ext3),
which is quite possible after a `nix-store which is quite possible after a `nix-store
--optimise'. It can also fail if another --optimise'. */
process called makeImmutable() on *i after we if (errno != EMLINK)
did makeMutable(). In those cases, make a copy
instead. */
if (errno != EMLINK && errno != EPERM)
throw SysError(format("linking `%1%' to `%2%'") % p % *i); throw SysError(format("linking `%1%' to `%2%'") % p % *i);
StringSink sink; StringSink sink;
dumpPath(*i, sink); dumpPath(*i, sink);
@ -1778,7 +1770,6 @@ void DerivationGoal::startBuilder()
restorePath(p, source); restorePath(p, source);
} }
makeImmutable(*i);
regularInputPaths.insert(*i); regularInputPaths.insert(*i);
} }
} }

View file

@ -517,8 +517,6 @@ void canonicalisePathMetaData(const Path & path, bool recurse)
foreach (Strings::iterator, i, names) foreach (Strings::iterator, i, names)
canonicalisePathMetaData(path + "/" + *i, true); canonicalisePathMetaData(path + "/" + *i, true);
} }
makeImmutable(path);
} }

View file

@ -33,8 +33,7 @@ struct MakeReadOnly
~MakeReadOnly() ~MakeReadOnly()
{ {
try { try {
/* This will make the path read-only (and restore the /* This will make the path read-only. */
immutable bit on platforms that support it). */
if (path != "") canonicalisePathMetaData(path, false); if (path != "") canonicalisePathMetaData(path, false);
} catch (...) { } catch (...) {
ignoreException(); ignoreException();
@ -43,14 +42,6 @@ struct MakeReadOnly
}; };
struct MakeImmutable
{
Path path;
MakeImmutable(const Path & path) : path(path) { }
~MakeImmutable() { makeImmutable(path); }
};
void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path) void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
{ {
checkInterrupt(); checkInterrupt();
@ -101,7 +92,6 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
if (!pathExists(linkPath)) { if (!pathExists(linkPath)) {
/* Nope, create a hard link in the links directory. */ /* Nope, create a hard link in the links directory. */
makeMutable(path); makeMutable(path);
MakeImmutable mk1(path);
if (link(path.c_str(), linkPath.c_str()) == 0) return; if (link(path.c_str(), linkPath.c_str()) == 0) return;
if (errno != EEXIST) if (errno != EEXIST)
throw SysError(format("cannot link `%1%' to `%2%'") % linkPath % path); throw SysError(format("cannot link `%1%' to `%2%'") % linkPath % path);
@ -134,57 +124,41 @@ void LocalStore::optimisePath_(OptimiseStats & stats, const Path & path)
MakeReadOnly makeReadOnly(mustToggle ? dirOf(path) : ""); MakeReadOnly makeReadOnly(mustToggle ? dirOf(path) : "");
/* If linkPath is immutable, we can't create hard links to it, /* If linkPath is immutable, we can't create hard links to it,
so make it mutable first (and make it immutable again when so make it mutable first. We also have to make path mutable,
we're done). We also have to make path mutable, otherwise otherwise rename() will fail to delete it. */
rename() will fail to delete it. */
makeMutable(path); makeMutable(path);
MakeImmutable mk2(path); makeMutable(linkPath);
/* Another process might be doing the same thing (creating a new Path tempLink = (format("%1%/.tmp-link-%2%-%3%")
link to linkPath) and make linkPath immutable before we're % settings.nixStore % getpid() % rand()).str();
done. In that case, just retry. */
unsigned int retries = 1024;
while (--retries > 0) {
makeMutable(linkPath);
MakeImmutable mk1(linkPath);
Path tempLink = (format("%1%/.tmp-link-%2%-%3%") if (link(linkPath.c_str(), tempLink.c_str()) == -1) {
% settings.nixStore % getpid() % rand()).str(); if (errno == EMLINK) {
/* Too many links to the same file (>= 32000 on most file
if (link(linkPath.c_str(), tempLink.c_str()) == -1) { systems). This is likely to happen with empty files.
if (errno == EMLINK) { Just shrug and ignore. */
/* Too many links to the same file (>= 32000 on most if (st.st_size)
file systems). This is likely to happen with empty printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
files. Just shrug and ignore. */ return;
if (st.st_size)
printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
return;
}
if (errno == EPERM) continue;
throw SysError(format("cannot link `%1%' to `%2%'") % tempLink % linkPath);
} }
throw SysError(format("cannot link `%1%' to `%2%'") % tempLink % linkPath);
/* Atomically replace the old file with the new hard link. */
if (rename(tempLink.c_str(), path.c_str()) == -1) {
if (unlink(tempLink.c_str()) == -1)
printMsg(lvlError, format("unable to unlink `%1%'") % tempLink);
if (errno == EMLINK) {
/* Some filesystems generate too many links on the
rename, rather than on the original link.
(Probably it temporarily increases the st_nlink
field before decreasing it again.) */
if (st.st_size)
printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
return;
}
if (errno == EPERM) continue;
throw SysError(format("cannot rename `%1%' to `%2%'") % tempLink % path);
}
break;
} }
if (retries == 0) throw Error(format("cannot link `%1%' to `%2%'") % path % linkPath); /* Atomically replace the old file with the new hard link. */
if (rename(tempLink.c_str(), path.c_str()) == -1) {
if (unlink(tempLink.c_str()) == -1)
printMsg(lvlError, format("unable to unlink `%1%'") % tempLink);
if (errno == EMLINK) {
/* Some filesystems generate too many links on the rename,
rather than on the original link. (Probably it
temporarily increases the st_nlink field before
decreasing it again.) */
if (st.st_size)
printMsg(lvlInfo, format("`%1%' has maximum number of links") % linkPath);
return;
}
throw SysError(format("cannot rename `%1%' to `%2%'") % tempLink % path);
}
stats.filesLinked++; stats.filesLinked++;
stats.bytesFreed += st.st_size; stats.bytesFreed += st.st_size;

View file

@ -16,7 +16,7 @@
namespace nix { namespace nix {
void changeMutable(const Path & path, bool mut) void makeMutable(const Path & path)
{ {
#if defined(FS_IOC_SETFLAGS) && defined(FS_IOC_GETFLAGS) && defined(FS_IMMUTABLE_FL) #if defined(FS_IOC_SETFLAGS) && defined(FS_IOC_GETFLAGS) && defined(FS_IMMUTABLE_FL)
@ -38,30 +38,12 @@ void changeMutable(const Path & path, bool mut)
/* Silently ignore errors getting/setting the immutable flag so /* Silently ignore errors getting/setting the immutable flag so
that we work correctly on filesystems that don't support it. */ that we work correctly on filesystems that don't support it. */
if (ioctl(fd, FS_IOC_GETFLAGS, &flags)) return; if (ioctl(fd, FS_IOC_GETFLAGS, &flags)) return;
old = flags; old = flags;
flags &= ~FS_IMMUTABLE_FL;
if (mut) flags &= ~FS_IMMUTABLE_FL;
else flags |= FS_IMMUTABLE_FL;
if (old == flags) return; if (old == flags) return;
if (ioctl(fd, FS_IOC_SETFLAGS, &flags)) return; if (ioctl(fd, FS_IOC_SETFLAGS, &flags)) return;
#endif #endif
} }
void makeImmutable(const Path & path)
{
changeMutable(path, false);
}
void makeMutable(const Path & path)
{
changeMutable(path, true);
}
} }

View file

@ -4,12 +4,6 @@
namespace nix { namespace nix {
/* Make the given path immutable, i.e., prevent it from being modified
in any way, even by root. This is a no-op on platforms that do not
support this, or if the calling user is not privileged. On Linux,
this is implemented by doing the equivalent of chattr +i path. */
void makeImmutable(const Path & path);
/* Make the given path mutable. */ /* Make the given path mutable. */
void makeMutable(const Path & path); void makeMutable(const Path & path);