From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] gnu: Add NFS related services. Date: Fri, 30 Sep 2016 14:02:37 +0200 Message-ID: <87shshoasi.fsf@gnu.org> References: <87bmzs6n1s.fsf@gnu.org> <1474791717-1839-1-git-send-email-jmd@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:41313) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpwWd-0003eI-GA for guix-devel@gnu.org; Fri, 30 Sep 2016 08:02:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpwWX-00040D-Dr for guix-devel@gnu.org; Fri, 30 Sep 2016 08:02:46 -0400 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:44246) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpwWX-000409-Ar for guix-devel@gnu.org; Fri, 30 Sep 2016 08:02:41 -0400 In-Reply-To: <1474791717-1839-1-git-send-email-jmd@gnu.org> (John Darrington's message of "Sun, 25 Sep 2016 10:21:57 +0200") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: John Darrington Cc: guix-devel@gnu.org John Darrington skribis: > Another draft for review ... Could you please include an iteration number in the subject line, and a terse summary of the changes compared to the previous iteration? That would be greatly helpful=E2=80=94I=E2=80=99m getting lost in a maze of= unrelated patch series and sometimes have a hard time remembering where we are and what it is that I=E2=80=99m doing here. ;-) > > > > > * gnu/services/nfs.scm (pipefs-service-type): New Variable, > (gss-service-type): New Variable, (idmap-service-type) New Variable. > --- > doc/guix.texi | 98 ++++++++++++++++++++++++++++++++++-- > gnu/services/nfs.scm | 138 +++++++++++++++++++++++++++++++++++++++++++++= ++++-- > 2 files changed, 230 insertions(+), 6 deletions(-) Please also mention the idmap things, the doc/guix.texi changes, etc. > +@subsubheading GSS Daemon Service > +@cindex GSSD > +@cindex GSS > + > +The GSS daemon provides strong security for RPC based protocols. =E2=80=9CThe @dfn{global security system} (GSS) daemon provides =E2=80=A6= =E2=80=9D >=20=20 > (define-record-type* > rpcbind-configuration make-rpcbind-configuration > @@ -38,11 +58,11 @@ > (shepherd-service-type > 'rpcbind > (lambda (config) > - (define pkg > + (define nfs-utils > (rpcbind-configuration-rpcbind config)) >=20=20 > (define rpcbind-command > - #~(list (string-append #$pkg "/bin/rpcbind") "-f" > + #~(list (string-append #$nfs-utils "/bin/rpcbind") "-f" Should have been part of a previous patch I guess, but that=E2=80=99s fine. > +(define-record-type* > + pipefs-configuration make-pipefs-configuration > + pipefs-configuration? > + (mount-point pipefs-configuration-mount-point > + (default default-pipefs-dir))) Seems to me we don=E2=80=99t even need ; a string wou= ld be enough, no? > +(define-record-type* > + gss-configuration make-gss-configuration > + gss-configuration? > + (pipefs-dir gss-configuration-pipefs-dir > + (default default-pipefs-dir)) s/dir/directory/ > +(define-record-type* > + idmap-configuration make-idmap-configuration > + idmap-configuration? > + (pipefs-dir idmap-configuration-pipefs-dir > + (default default-pipefs-dir)) > + (domain idmap-configuration-domain > + (default #f)) > + (nfs-utils idmap-configuration-idmap > + (default nfs-utils))) > + > +(define idmap-service-type > + (shepherd-service-type > + 'idmap > + (lambda (config) > + > + (define nfs-utils > + (idmap-configuration-idmap config)) > + > + (define pipefs-dir > + (idmap-configuration-pipefs-dir config)) > + > + (define conf-file "/etc/guix-idmapd.conf") > + > + (define idmap-command > + #~(list (string-append #$nfs-utils "/sbin/rpc.idmapd") "-f" > + "-p" #$pipefs-dir > + "-c" #$conf-file)) > + > + (define domain (idmap-configuration-domain config)) > + > + (shepherd-service > + (documentation "Start the RPC IDMAP daemon.") > + (requirement '(rpcbind-daemon rpc-pipefs)) > + (provision '(idmap-daemon)) > + > + (start #~(lambda () > + (let ((pid (primitive-fork))) > + (if (zero? pid) > + (begin > + (call-with-output-file #$conf-file > + (lambda (port) > + (format port "\n[General]\n") > + (if #$domain > + (format port "Domain =3D ~a\n" #$domain= )) > + (format port "\n[Mapping]\n") > + (format port "Nobody-User =3D nobody\n") > + (format port "Nobody-Group =3D nogroup\n"))) > + (exec-command #$idmap-command)) > + pid)))) I think the configuration file should be created elsewhere, in the store: (define (idmap-config-file config) (plain-file "idmap.conf" (string-append "[General]" =E2=80=A6))) and then: (define idmap-command #~(list =E2=80=A6 "-c" #$(idmap-config-file config))) (shepherd-service ;; =E2=80=A6 (start #~(make-forkexec-constructor #$idmap-command))) In general we should avoid populating /etc. Could you send an updated patch? Overall this seems to be almost ready, no? Since this is a pretty involved service composition, I think it would be fruitful in the future to add a full test case in (gnu tests nfs) where we would export an NFS tree and mount it. Thank you! Ludo=E2=80=99.