From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52510) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f6tLR-0007av-Dc for guix-patches@gnu.org; Fri, 13 Apr 2018 03:42:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f6tLO-0000Ff-74 for guix-patches@gnu.org; Fri, 13 Apr 2018 03:42:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:41127) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1f6tLO-0000FN-2A for guix-patches@gnu.org; Fri, 13 Apr 2018 03:42:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1f6tLN-0002h3-K6 for guix-patches@gnu.org; Fri, 13 Apr 2018 03:42:01 -0400 Subject: [bug#29732] [PATCH 1/1] services: Add dhcpd-service-type and . Resent-Message-ID: From: Chris Marusich References: <20171216083528.2081-1-cmmarusich@gmail.com> <20171216085242.2309-1-cmmarusich@gmail.com> <871sh6rafl.fsf@lassieur.org> Date: Fri, 13 Apr 2018 00:41:38 -0700 In-Reply-To: <871sh6rafl.fsf@lassieur.org> ("=?UTF-8?Q?Cl=C3=A9ment?= Lassieur"'s message of "Tue, 27 Feb 2018 11:31:10 +0100") Message-ID: <87efjjwo25.fsf@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 29732@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Cl=C3=A9ment and Ludo, Thank you for the review! Cl=C3=A9ment Lassieur writes: >> + (ip-version dhcpd-ip-version ; either "4" or "6" >> + (default "4")) > > Upstream defaults to "6". I guess it's because they want to encourage > people to use IPv6. Maybe we should respect their choice and default to > "6" as well? Are you sure about this? The manual (man 8 dhcpd) says: COMMAND LINE OPTIONS -4 Run as a DHCP server. This is the default and cannot be combined with -6. -6 Run as a DHCPv6 server. This cannot be combined with -4. -4o6 port Participate in the DHCPv4 over DHCPv6 protocol specified by RFC 7341. This associates a DHCPv4 and a DHCPv6 server to allow the v4 server to receive v4 requests that were encapsulated in a v6 packet. Communication between the two servers is done on a pair of UDP sockets bound to ::1 port and port + 1. Both servers must be launched using the same port argument. I agree we should respect the default that upstream sets. Where did you see it indicated that upstream sets the default to 6? In addition, I didn't even know about the -4o6 option, but thankfully my patch would still allow someone to specify that as the ip-version, too. > I wonder, does it make sense to run two instances of the daemon, one for > IPv4 and another for IPv6? Would having the IP version included in the > pid file name make it possible? (dhcpd-4.pid and dhcp-6.pid) That would make it possible, yes. However, I think that it should be up to the user to decide whether or not to run two services. If they want to run two (or three) dhcpd services for the different IP protocol versions, then they should be able to do so easily. I might need to adjust the "provision" part of the dhcpd-shepherd-service to allow that, though. I'm not sure what happens if two services try to provision the same "provision" symbol - probably nothing good. >> + (interfaces dhcpd-interfaces ; list of strings, e.g. (list "enp0s25") >> + (default '()))) > > I don't really understand the logic of the indentation and alignment > of this whole block :-). I try to follow the indentation style mentioned in the Guix manual (see: (guix) Coding Style), but I may have let a few rough spots slip through. If you have specific suggestions for how to improve the indentation, I would be glad to hear them. >> +(define dhcpd-activation >> + (match-lambda >> + (($ package config-file _ run-directory lease= -file _ _) >> + #~(begin >> + (unless (file-exists? #$run-directory) >> + (mkdir #$run-directory)) > > mkdir-p I guess Yeah, that's probably better. I'll see about adjusting it. >> + ;; According to the DHCP manual (man dhcpd.leases), the lease >> + ;; database must be present for dhcpd to start successfully. >> + (unless (file-exists? #$lease-file) >> + (with-output-to-file #$lease-file >> + (lambda _ (display "")))) >> + ;; Validate the config. >> + (zero? (system* #$(file-append package "/sbin/dhcpd") "-t" "-c= f" #$config-file)))))) >> + >> +(define dhcpd-service-type >> + (service-type >> + (name 'dhcpd) >> + (extensions >> + (list (service-extension shepherd-root-service-type dhcpd-shepherd-= service) >> + (service-extension activation-service-type dhcpd-activation))= ))) >> + >> (define %ntp-servers >> ;; Default set of NTP servers. These URLs are managed by the NTP Pool= project. >> ;; Within Guix, Leo Famulari is the administrativ= e contact > > Also, could you stick to 80 columns? Certainly - looks like I accidentally missed a few long lines. I've got a new patch almost ready to share which will improve the formatting and, most importantly, add a couple tests! I'll try to clean it up and post it by the end of January. ;-) =2D-=20 Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAlrQXzIACgkQ3UCaFdgi Rp0ZxxAA0F5WDEhS5F50Mp0osoX+kiuwpD7ou7a0pIgDAGmEEM/fP/GJBVkBLoaj msqiXu5pUfyiuUvMvsDUR14ferGD9bJU0TgyF/lXGYgy3zPOfE2H0fRN0tDWKAho sF/yGvpSscW/TMg/qaqjshuazGAypB4kqDqvJTRucqNwWffFvY9i7iYNi0QGPI5/ 4X33sS+/hZZSrezSJNwAyqLU4U7msG5pJWyjrgF4MsH6wGB6p2fefBpwHbYAXhtX fsoThTQLfXhIlyNcXfChXOPa5EIJ1iJ15mBI5+TVBAM7LHSZtyvIBe/JgjeAzFbE JNd5v3/49Oa1iXDRIWb5f9mElHG3jdZshWweRROGdSUZA58rvUMxpJ7CxQKt09jh kU6LViDnsrIkRxxzCuKJCYXn2vLhiyfUR5DLEC0RWTJ36t/apgo2Hk/Hx/b4pVt3 Y90zs4WPWGA0+8jHWzRXihk4uCUFpnTTiyTxeVUWC5jYz9s4Pc/6bHL/j0r+4mM0 E8dR8ROqbDmcZsWz0Bw618ZVXi6z0b54jjJdir+tmCyyAtt+6g2GkKDY2Md/RDAj Af/wPJZQ5s4rNHhu6jpw0676ZLDSbk6TJN9LW1YzC+e4ISEK0Qf1zxHqNegYtTem fcXDcBY+fNEJJqHyUxiO28JcqVNTVtqUozA2FpSlpjGos0mAkAI= =wDsf -----END PGP SIGNATURE----- --=-=-=--