Re: [PATCH] guix-daemon: Add option to disable garbage collection.

[Roel Janssen <roel@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]


Roel Janssen <roel@gnu.org> writes:

> Ludovic Courtès <ludovic.courtes@inria.fr> writes:
>
>> Hello Roel,
>>
>> Roel Janssen <roel@gnu.org> skribis:
>>
>>> The patch adds a “disableGarbageCollection” boolean variable to the
>>> guix-daemon settings, and on each occasion where a store item may be
>>> deleted, it checks this option.
>>>
>>> This option can be set using “--disable-gc”.
>>>
>>> It would be great if someone could review this and discuss whether
>>> this is the right way to implement such a feature.  And to point out
>>> what else would be needed to include this option in guix-daemon.
>>
>> I suppose the use case is when guix-daemon runs on a machine and is
>> accessed over TCP/IP (with GUIX_DAEMON_SOCKET=guix://…) from other
>> machines, right?
>
> That's right.
>
>> In this case, I thought guix-daemon could explicitly check whether the
>> peer is remote, and disable GC in that case.  That is, ‘guix gc’ would
>> still work locally on the machine that runs guix-daemon, but it would no
>> longer work remotely.
>>
>> How does that sound?
>
> That sounds like it solves our use-case, but only because in our
> case the access to the machine running guix-daemon is limited.
>
> So, even though I'm not sure how to implement this, your solution is
> fine with me.

I implemented the solution in the attached patch.  When a connection
does not come from the UNIX socket, it is treated as “remote”.  So,
local TCP connections would also be treated as “remote”.

I assumed ‘collectGarbage()’ is the entry point for all garbage collection,
is that correct?

Kind regards,
Roel Janssen


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-guix-daemon-Disable-garbage-collection-for-remote-ho.patch --]
[-- Type: text/x-patch, Size: 2683 bytes --]

From 00f489d6303720c65571fdf0bc9ee810a20f70e0 Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Wed, 11 Apr 2018 09:52:11 +0200
Subject: [PATCH] guix-daemon: Disable garbage collection for remote hosts.

* nix/libstore/gc.cc (collectGarbage): Check for remote connections.
* nix/libstore/globals.hh: Add isRemoteConnection setting.
* nix/nix-daemon/nix-daemon.cc (performOp): Display appropriate error message;
  (acceptConnection): Set isRemoteConnection when connection is over TCP.
---
 nix/libstore/gc.cc           | 4 ++++
 nix/libstore/globals.hh      | 4 ++++
 nix/nix-daemon/nix-daemon.cc | 6 ++++++
 3 files changed, 14 insertions(+)

diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc
index 72eff5242..1bc6eedb5 100644
--- a/nix/libstore/gc.cc
+++ b/nix/libstore/gc.cc
@@ -595,6 +595,10 @@ void LocalStore::removeUnusedLinks(const GCState & state)
 
 void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 {
+    if (settings.isRemoteConnection) {
+        return;
+    }
+
     GCState state(results);
     state.options = options;
     state.trashDir = settings.nixStore + "/trash";
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index 1293625e1..83efbcd50 100644
--- a/nix/libstore/globals.hh
+++ b/nix/libstore/globals.hh
@@ -81,6 +81,10 @@ struct Settings {
     uid_t clientUid;
     gid_t clientGid;
 
+    /* Whether the connection comes from a host other than the host running
+       guix-daemon. */
+    bool isRemoteConnection;
+
     /* Whether, if we cannot realise the known closure corresponding
        to a derivation, we should try to normalise the derivation
        instead. */
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index deb7003d7..65770ba95 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -529,6 +529,11 @@ static void performOp(bool trusted, unsigned int clientVersion,
     }
 
     case wopCollectGarbage: {
+        if (settings.isRemoteConnection) {
+            throw Error("Garbage collection is disabled for remote hosts.");
+            break;
+        }
+
         GCOptions options;
         options.action = (GCOptions::GCAction) readInt(from);
         options.pathsToDelete = readStorePaths<PathSet>(from);
@@ -934,6 +939,7 @@ static void acceptConnection(int fdSocket)
                    connection.  Setting these to -1 means: do not change.  */
                 settings.clientUid = clientUid;
 		settings.clientGid = clientGid;
+                settings.isRemoteConnection = (remoteAddr.ss_family != AF_UNIX);
 
                 /* Handle the connection. */
                 from.fd = remote;
-- 
2.16.3