Discussion:
[PATCH] guix-daemon: Add option to disable garbage collection.
Roel Janssen
2018-04-03 10:12:42 UTC
Permalink
Dear Guix,

I have an interesting situation where the guix-daemon cannot see all
directories that (may) contain Guix profiles, and therefore is not able
to judge whether a GC root is gone and can be collected, or whether it's
just inaccessible.

To be on the safe side, I'd like to disable garbage collection on this
system. To achieve this, I wrote the attached patch. Even though “it
works for me”, I don't think it's good to be added as-is. At least I
need to figure out how to make the error message “Garbage collection is
disabled.” translatable.

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.

Thank you for your time.

Kind regards,
Roel Janssen
Adam Van Ymeren
2018-04-03 12:41:50 UTC
Permalink
Post by Roel Janssen
Dear Guix,
I have an interesting situation where the guix-daemon cannot see all
directories that (may) contain Guix profiles, and therefore is not able
to judge whether a GC root is gone and can be collected, or whether it's
just inaccessible.
To be on the safe side, I'd like to disable garbage collection on this
system.  To achieve this, I wrote the attached patch.  Even though “it
works for me”, I don't think it's good to be added as-is.  At least I
need to figure out how to make the error message “Garbage collection is
disabled.” translatable.
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.
Thank you for your time.
Kind regards,
Roel Janssen
Roel Janssen
2018-04-03 13:03:42 UTC
Permalink
Just out of curiosity, what is your situation where the daemon can't
see all GC roots? Are you sharing the store over NFS or something?
Yes. And the underlying storage system is configured in such a way that
“root” is not allowed to look into user's folders.

Kind regards,
Roel Janssen
Ludovic Courtès
2018-04-03 13:40:04 UTC
Permalink
Hello Roel,
Post by Roel Janssen
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?

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?

Thanks,
Ludo’.
Roel Janssen
2018-04-03 14:02:49 UTC
Permalink
Post by Ludovic Courtès
Hello Roel,
Post by Roel Janssen
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.
Post by Ludovic Courtès
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.
Post by Ludovic Courtès
Thanks,
Ludo’.
Thanks!

Kind regards,
Roel Janssen
Roel Janssen
2018-04-11 07:57:51 UTC
Permalink
Post by Roel Janssen
Post by Ludovic Courtès
Hello Roel,
Post by Roel Janssen
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.
Post by Ludovic Courtès
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
Roel Janssen
2018-04-17 21:00:51 UTC
Permalink
Hello there,

I'm not sure this made it to the mailing list. Is the proposed patch
fine to disable the GC for remote connections?

Thanks!

Kind regards,
Roel Janssen
Post by Roel Janssen
Post by Roel Janssen
Post by Ludovic Courtès
Hello Roel,
Post by Roel Janssen
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.
Post by Ludovic Courtès
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
From 00f489d6303720c65571fdf0bc9ee810a20f70e0 Mon Sep 17 00:00:00 2001
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;
Ludovic Courtès
2018-04-18 21:12:43 UTC
Permalink
Hello!
Post by Roel Janssen
I'm not sure this made it to the mailing list. Is the proposed patch
fine to disable the GC for remote connections?
It did make it to the list, I’ll look at it real soon.

Ludo’.
Ludovic Courtès
2018-04-19 09:06:42 UTC
Permalink
Hello Roel,
[...]
Post by Roel Janssen
Post by Roel Janssen
Post by Ludovic Courtès
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”.
Excellent!
Post by Roel Janssen
I assumed ‘collectGarbage()’ is the entry point for all garbage collection,
is that correct?
Yes.
Post by Roel Janssen
From 00f489d6303720c65571fdf0bc9ee810a20f70e0 Mon Sep 17 00:00:00 2001
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.
[...]
Post by Roel Janssen
--- 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;
+ }
I think this is unnecessary since the daemon already checks for that.
(It’s also nicer to keep ‘LocalStore’ unaware of the connection
details.)
Post by Roel Janssen
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);
I was wondering if we would like to allow some of the ‘GCAction’ values,
but maybe it’s better to disallow them altogether like this code does.
Post by Roel Janssen
@@ -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);
I think you can make ‘isRemoteConnection’ a static global variable in
nix-daemon.cc instead of adding it to ‘Settings’. So it would do
something like:

--8<---------------cut here---------------start------------->8---
/* Fork a child to handle the connection. */
startProcess([&]() {
close(fdSocket);

/* Background the daemon. */
if (setsid() == -1)
throw SysError(format("creating a new session"));

/* Restore normal handling of SIGCHLD. */
setSigChldAction(false);

/* For debugging, stuff the pid into argv[1]. */
if (clientPid != -1 && argvSaved[1]) {
string processName = std::to_string(clientPid);
strncpy(argvSaved[1], processName.c_str(), strlen(argvSaved[1]));
}

isRemoteConnection = …; /* <– this is the new line */

/* Store the client's user and group for this connection. This
has to be done in the forked process since it is per
connection. Setting these to -1 means: do not change. */
settings.clientUid = clientUid;
settings.clientGid = clientGid;
--8<---------------cut here---------------end--------------->8---

Last thing: could you add a couple of tests? tests/guix-daemon.sh
already has tests for ‘--listen’, so you could take inspiration from
those.

Thank you!

Ludo’.
Roel Janssen
2018-04-19 12:12:02 UTC
Permalink
Post by Ludovic Courtès
Hello Roel,
[...]
Post by Ludovic Courtès
Post by Roel Janssen
From 00f489d6303720c65571fdf0bc9ee810a20f70e0 Mon Sep 17 00:00:00 2001
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.
[...]
Post by Roel Janssen
--- 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;
+ }
I think this is unnecessary since the daemon already checks for that.
(It’s also nicer to keep ‘LocalStore’ unaware of the connection
details.)
Right. It is indeed unnecessary. In the updated patch, I no longer do this.
Post by Ludovic Courtès
Post by Roel Janssen
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);
I was wondering if we would like to allow some of the ‘GCAction’ values,
but maybe it’s better to disallow them altogether like this code does.
Could we please start with a “disable any GC” and start allowing cases
on a case-by-case basis? The reason I request this, is because it makes
it a lot easier to reason about it from a sysadmin point-of-view.

I'd like to think of it like this: “Garbage collection is effectively
turned off.”. With the current patch, we can reason about it this way.
Post by Ludovic Courtès
Post by Roel Janssen
@@ -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);
I think you can make ‘isRemoteConnection’ a static global variable in
nix-daemon.cc instead of adding it to ‘Settings’. So it would do
--8<---------------cut here---------------start------------->8---
/* Fork a child to handle the connection. */
startProcess([&]() {
close(fdSocket);
/* Background the daemon. */
if (setsid() == -1)
throw SysError(format("creating a new session"));
/* Restore normal handling of SIGCHLD. */
setSigChldAction(false);
/* For debugging, stuff the pid into argv[1]. */
if (clientPid != -1 && argvSaved[1]) {
string processName = std::to_string(clientPid);
strncpy(argvSaved[1], processName.c_str(), strlen(argvSaved[1]));
}
isRemoteConnection = 
; /* <– this is the new line */
/* Store the client's user and group for this connection. This
has to be done in the forked process since it is per
connection. Setting these to -1 means: do not change. */
settings.clientUid = clientUid;
settings.clientGid = clientGid;
--8<---------------cut here---------------end--------------->8---
Right. I implemented it using a static global variable in
nix-daemon.cc. This patch gets shorter and shorter. :)
Post by Ludovic Courtès
Last thing: could you add a couple of tests? tests/guix-daemon.sh
already has tests for ‘--listen’, so you could take inspiration from
those.
I included a test, but I don't know how I can properly run this test.
Could you elaborate on how I can test the test(s)?

Kind regards,
Roel Janssen
Ludovic Courtès
2018-04-19 14:47:15 UTC
Permalink
[...]
Post by Ludovic Courtès
Post by Roel Janssen
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);
I was wondering if we would like to allow some of the ‘GCAction’ values,
but maybe it’s better to disallow them altogether like this code does.
Could we please start with a “disable any GC” and start allowing cases
on a case-by-case basis?
Sure, that’s what I was suggesting. :-)
Post by Ludovic Courtès
Last thing: could you add a couple of tests? tests/guix-daemon.sh
already has tests for ‘--listen’, so you could take inspiration from
those.
I included a test, but I don't know how I can properly run this test.
Could you elaborate on how I can test the test(s)?
Run:

make check TESTS=tests/guix-daemon.sh

See
<https://www.gnu.org/software/guix/manual/html_node/Running-the-Test-Suite.html>.
From b29d3a90e1487ebda5ac5b6bc146f8c95218eab6 Mon Sep 17 00:00:00 2001
Date: Thu, 19 Apr 2018 14:01:49 +0200
Subject: [PATCH] guix-daemon: Disable garbage collection for remote hosts.
* nix/nix-daemon/nix-daemon.cc (performOp): Display appropriate error message;
(acceptConnection): Set isRemoteConnection when connection is over TCP.
Rather:

* nix/nix-daemon/nix-daemon.cc (isRemoteConnection): New variable.
(performOp): For wopCollectGarbage, throw an error when
isRemoteConnection is set.
(acceptConnection): Set isRemoteConnection when connection is not AF_UNIX.
+output=`GUIX_DAEMON_SOCKET="$socket" guix gc`
+if [[ "$output" != *"GUIX_DAEMON_SOCKET=$socket" ]];
+then
+ exit 1
+fi
Perhaps simply check the exit code of ‘guix gc’ and fail if it succeeds?

Like:

if guix gc; then false; else true; fi

Also please try to avoid Bash-specific constructs like [[ this ]].

Could you send an updated patch?

Thank you,
Ludo’.
Roel Janssen
2018-04-19 15:15:28 UTC
Permalink
[...]
Post by Roel Janssen
Post by Ludovic Courtès
Post by Roel Janssen
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);
I was wondering if we would like to allow some of the ‘GCAction’ values,
but maybe it’s better to disallow them altogether like this code does.
Could we please start with a “disable any GC” and start allowing cases
on a case-by-case basis?
Sure, that’s what I was suggesting. :-)
Post by Roel Janssen
Post by Ludovic Courtès
Last thing: could you add a couple of tests? tests/guix-daemon.sh
already has tests for ‘--listen’, so you could take inspiration from
those.
I included a test, but I don't know how I can properly run this test.
Could you elaborate on how I can test the test(s)?
make check TESTS=tests/guix-daemon.sh
See
<https://www.gnu.org/software/guix/manual/html_node/Running-the-Test-Suite.html>.
That is really nice. Thanks for pointing to the manual.
Post by Roel Janssen
From b29d3a90e1487ebda5ac5b6bc146f8c95218eab6 Mon Sep 17 00:00:00 2001
Date: Thu, 19 Apr 2018 14:01:49 +0200
Subject: [PATCH] guix-daemon: Disable garbage collection for remote hosts.
* nix/nix-daemon/nix-daemon.cc (performOp): Display appropriate error message;
(acceptConnection): Set isRemoteConnection when connection is over TCP.
* nix/nix-daemon/nix-daemon.cc (isRemoteConnection): New variable.
(performOp): For wopCollectGarbage, throw an error when
isRemoteConnection is set.
(acceptConnection): Set isRemoteConnection when connection is not AF_UNIX.
Post by Roel Janssen
+output=`GUIX_DAEMON_SOCKET="$socket" guix gc`
+if [[ "$output" != *"GUIX_DAEMON_SOCKET=$socket" ]];
+then
+ exit 1
+fi
Perhaps simply check the exit code of ‘guix gc’ and fail if it succeeds?
Right.
if guix gc; then false; else true; fi
Also please try to avoid Bash-specific constructs like [[ this ]].
Right.
Could you send an updated patch?
The attached patch should be fine.

Kind regards,
Roel Janssen
Ludovic Courtès
2018-04-19 15:26:36 UTC
Permalink
Heya,
From fcbe7ebb3d205cf7310700e62b78b9aafd94f76f Mon Sep 17 00:00:00 2001
Date: Thu, 19 Apr 2018 17:11:30 +0200
Subject: [PATCH] guix-daemon: Disable garbage collection for remote
connections.
* nix/nix-daemon/nix-daemon.cc (isRemoteConnection): New variable.
(performOp): For wopCollectGarbage, throw an error when isRemoteConnection
is set.
(acceptConnection): Set isRemoteConnection when connection is not AF_UNIX.
* tests/guix-daemon.sh: Add a test for the new behavior.
LGTM, thanks for the quick reply!

Ludo’.
Roel Janssen
2018-04-19 17:07:23 UTC
Permalink
Post by Ludovic Courtès
Heya,
From fcbe7ebb3d205cf7310700e62b78b9aafd94f76f Mon Sep 17 00:00:00 2001
Date: Thu, 19 Apr 2018 17:11:30 +0200
Subject: [PATCH] guix-daemon: Disable garbage collection for remote
connections.
* nix/nix-daemon/nix-daemon.cc (isRemoteConnection): New variable.
(performOp): For wopCollectGarbage, throw an error when isRemoteConnection
is set.
(acceptConnection): Set isRemoteConnection when connection is not AF_UNIX.
* tests/guix-daemon.sh: Add a test for the new behavior.
LGTM, thanks for the quick reply!
Awesome. Pushed in 5cefb13dd.

Thanks!

Kind regards,
Roel Janssen

Marius Bakke
2018-04-19 15:25:27 UTC
Permalink
Post by Roel Janssen
I included a test, but I don't know how I can properly run this test.
Could you elaborate on how I can test the test(s)?
[...]
Post by Roel Janssen
diff --git a/tests/guix-daemon.sh b/tests/guix-daemon.sh
index 6f91eb58b..438c79c26 100644
--- a/tests/guix-daemon.sh
+++ b/tests/guix-daemon.sh
@@ -194,6 +194,20 @@ do
kill "$daemon_pid"
done
+# Make sure garbage collection from a TCP connection does not work.
+
+socket="127.0.0.1:9999"
+guix-daemon --listen="$socket" &
+daemon_pid=$!
+
+output=`GUIX_DAEMON_SOCKET="$socket" guix gc`
+if [[ "$output" != *"GUIX_DAEMON_SOCKET=$socket" ]];
+then
+ exit 1
+fi
+
+kill "$daemon_pid"
+
# Log compression.
guix-daemon --listen="$socket" --disable-chroot --debug --log-compression=gzip &
Use `make check "TESTS=tests/guix-daemon.sh"` to only run this one test.
Loading...