[PATCH] rpcbind: systemd socket activation

[PATCH] rpcbind: systemd socket activation

[Steve Dickson]

This is a re-post of Tom's patch he posted a while back.
I made the following changes.

* Reorganized the changes so the diff is more readable
  by using a goto instead of indenting 268 lines.

* Removed the systemd scripts because they just didn't
  work and won't going to work. I did spend some time 
  on them but debugging was not worth the time since 
  the current ones worked. I'm more than willing to 
  revisit this down the line.

Finally I would like to thank Tom for all of his help! 
It was much appreciated!

Tom Gundersen (1):
  rpcbind: add support for systemd socket activation

 Makefile.am   |  6 +++++
 configure.ac  | 10 ++++++++
 src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 91 insertions(+), 6 deletions(-)

-- 
2.1.0

[PATCH] rpcbind: add support for systemd socket activation

[Steve Dickson]

From: Tom Gundersen <teg@jklm.no>

Making rpcbind sockect activated will greatly simplify
its integration in systemd systems. In essence, other services
may now assume that rpcbind is always available, even during very
early boot. This means that we no longer need to worry about any
ordering dependencies.

This is based on a patch originally posted by Lennart Poettering:
<http://permalink.gmane.org/gmane.linux.nfs/33774>.

That patch was not merged due to the lack of a shared library and
as systemd was seen to be too Fedora specific.

Systemd now provides a shared library, and it is (or very soon will
be) the default init system on all the major Linux distributions.

This version of the patch has three changes from the original:

 * It uses the shared library.
 * It comes with unit files.
 * It is rebased on top of master.

Please review the patch with "git show -b" or otherwise ignoring the
whitespace changes, or it will be extremely difficult to read.

v4: reorganized the changes to make the diff easier to read
    remove systemd scripts.

v3: rebase
    fix typos
    listen on /run/rpcbind.sock, rather than /var/run/rpcbind.sock (the
    latter is a symlink to the former, but this means the socket can be
    created before /var is mounted)
    NB: this version has been compile-tested only as I no longer use
    rpcbind myself
v2: correctly enable systemd code at compile time
    handle the case where not all the required sockets were supplied
    listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
    do not daemonize

Original-patch-by: Lennart Poettering <lennart@poettering.net>
Cc: Steve Dickson <steved@redhat.com>
Cc: systemd-devel@lists.freedesktop.org
Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
Signed-off-by: Tom Gundersen <teg@jklm.no>
Signed-off-by: Steve Dickson <steved@redhat.com>
---
 Makefile.am   |  6 +++++
 configure.ac  | 10 ++++++++
 src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8715082..c99566d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,6 +41,12 @@ rpcbind_SOURCES = \
 	src/warmstart.c
 rpcbind_LDADD = $(TIRPC_LIBS)
 
+if SYSTEMD
+AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
+
+rpcbind_LDADD += $(SYSTEMD_LIBS)
+endif
+
 rpcinfo_SOURCES =       src/rpcinfo.c
 rpcinfo_LDADD   =       $(TIRPC_LIBS)
 
diff --git a/configure.ac b/configure.ac
index 5a88cc7..fdad639 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,16 @@ AC_SUBST([nss_modules], [$with_nss_modules])
 
 PKG_CHECK_MODULES([TIRPC], [libtirpc])
 
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemdsystemunitdir],
+  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
+  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
+  if test "x$with_systemdsystemunitdir" != xno; then
+    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
+  fi
+AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
+
 AS_IF([test x$enable_libwrap = xyes], [
 	AC_CHECK_LIB([wrap], [hosts_access], ,
 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
diff --git a/src/rpcbind.c b/src/rpcbind.c
index e3462e3..f7c71ee 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -56,6 +56,9 @@
 #include <netinet/in.h>
 #endif
 #include <arpa/inet.h>
+#ifdef SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
 #include <fcntl.h>
 #include <netdb.h>
 #include <stdio.h>
@@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
 	struct sockaddr_un sun;
 	mode_t oldmask;
+	int n;
         res = NULL;
 
 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
 			fprintf(stderr, "[%d] - %s\n", i, *s);
 	}
 #endif
+	if (!__rpc_nconf2sockinfo(nconf, &si)) {
+		syslog(LOG_ERR, "cannot get information for %s",
+		    nconf->nc_netid);
+		return (1);
+	}
+
+#ifdef SYSTEMD
+	n = sd_listen_fds(0);
+	if (n < 0) {
+		syslog(LOG_ERR, "failed to acquire systemd sockets: %s", strerror(-n));
+		return 1;
+	}
+
+	/* Try to find if one of the systemd sockets we were given match
+	 * our netconfig structure. */
+
+	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+		struct __rpc_sockinfo si_other;
+		union {
+			struct sockaddr sa;
+			struct sockaddr_un un;
+			struct sockaddr_in in4;
+			struct sockaddr_in6 in6;
+			struct sockaddr_storage storage;
+		} sa;
+		socklen_t addrlen = sizeof(sa);
+
+		if (!__rpc_fd2sockinfo(fd, &si_other)) {
+			syslog(LOG_ERR, "cannot get information for fd %i", fd);
+			return 1;
+		}
+
+		if (si.si_af != si_other.si_af ||
+                    si.si_socktype != si_other.si_socktype ||
+                    si.si_proto != si_other.si_proto)
+			continue;
+
+		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+			syslog(LOG_ERR, "failed to query socket name: %s",
+                               strerror(errno));
+			goto error;
+		}
+
+		/* Copy the address */
+		taddr.addr.maxlen = taddr.addr.len = addrlen;
+		taddr.addr.buf = malloc(addrlen);
+		if (taddr.addr.buf == NULL) {
+			syslog(LOG_ERR,
+                               "cannot allocate memory for %s address",
+                               nconf->nc_netid);
+			goto error;
+		}
+		memcpy(taddr.addr.buf, &sa, addrlen);
+
+		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+		if (my_xprt == (SVCXPRT *)NULL) {
+			syslog(LOG_ERR, "%s: could not create service",
+                               nconf->nc_netid);
+			goto error;
+		}
+	}
+
+	/*
+	 * If none of the systemd sockets matched, we set up the socket in
+	 * the normal way:
+	 */
+#endif
+	if (my_xprt != NULL)
+		goto got_socket;
 
 	/*
 	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
@@ -327,12 +401,6 @@ init_transport(struct netconfig *nconf)
 		}
 	}
 
-	if (!__rpc_nconf2sockinfo(nconf, &si)) {
-		syslog(LOG_ERR, "cannot get information for %s",
-		    nconf->nc_netid);
-		return (1);
-	}
-
 	if ((strcmp(nconf->nc_netid, "local") == 0) ||
 	    (strcmp(nconf->nc_netid, "unix") == 0)) {
 		memset(&sun, 0, sizeof sun);
@@ -569,6 +637,7 @@ init_transport(struct netconfig *nconf)
 			goto error;
 		}
 	}
+got_socket:
 
 #ifdef PORTMAP
 	/*
-- 
2.1.0

Re: [systemd-devel] [PATCH] rpcbind: add support for systemd socket activation

[Zbigniew Jędrzejewski-Szmek]

On Fri, Nov 21, 2014 at 11:43:47AM -0500, Steve Dickson wrote:
> From: Tom Gundersen <teg@jklm.no>
> 
> Making rpcbind sockect activated will greatly simplify
> its integration in systemd systems. In essence, other services
> may now assume that rpcbind is always available, even during very
> early boot. This means that we no longer need to worry about any
> ordering dependencies.
> 
> This is based on a patch originally posted by Lennart Poettering:
> <http://permalink.gmane.org/gmane.linux.nfs/33774>.
> 
> That patch was not merged due to the lack of a shared library and
> as systemd was seen to be too Fedora specific.
> 
> Systemd now provides a shared library, and it is (or very soon will
> be) the default init system on all the major Linux distributions.
> 
> This version of the patch has three changes from the original:
> 
>  * It uses the shared library.
>  * It comes with unit files.
>  * It is rebased on top of master.
> 
> Please review the patch with "git show -b" or otherwise ignoring the
> whitespace changes, or it will be extremely difficult to read.

Thanks for working on this... Looks fine to me. Two suggestions
below.

> v4: reorganized the changes to make the diff easier to read
>     remove systemd scripts.
> 
> v3: rebase
>     fix typos
>     listen on /run/rpcbind.sock, rather than /var/run/rpcbind.sock (the
>     latter is a symlink to the former, but this means the socket can be
>     created before /var is mounted)
>     NB: this version has been compile-tested only as I no longer use
>     rpcbind myself
> v2: correctly enable systemd code at compile time
>     handle the case where not all the required sockets were supplied
>     listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
>     do not daemonize
> 
> Original-patch-by: Lennart Poettering <lennart@poettering.net>
> Cc: Steve Dickson <steved@redhat.com>
> Cc: systemd-devel@lists.freedesktop.org
> Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
> Signed-off-by: Tom Gundersen <teg@jklm.no>
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  Makefile.am   |  6 +++++
>  configure.ac  | 10 ++++++++
>  src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 91 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 8715082..c99566d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -41,6 +41,12 @@ rpcbind_SOURCES = \
>  	src/warmstart.c
>  rpcbind_LDADD = $(TIRPC_LIBS)
>  
> +if SYSTEMD
> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
> +
> +rpcbind_LDADD += $(SYSTEMD_LIBS)
> +endif
> +
>  rpcinfo_SOURCES =       src/rpcinfo.c
>  rpcinfo_LDADD   =       $(TIRPC_LIBS)
>  
> diff --git a/configure.ac b/configure.ac
> index 5a88cc7..fdad639 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -36,6 +36,16 @@ AC_SUBST([nss_modules], [$with_nss_modules])
>  
>  PKG_CHECK_MODULES([TIRPC], [libtirpc])
>  
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemdsystemunitdir],
> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
> +  if test "x$with_systemdsystemunitdir" != xno; then
> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
> +    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
libsystemd-daemon got subsummed by libsystemd. So you should use something
like

   PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [],
      [PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [],
       AC_MSG_ERROR([libsystemd support requested but found]))])

to get the new libary in preference to the old one.

> +  fi
> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
> +
>  AS_IF([test x$enable_libwrap = xyes], [
>  	AC_CHECK_LIB([wrap], [hosts_access], ,
>  		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index e3462e3..f7c71ee 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -56,6 +56,9 @@
>  #include <netinet/in.h>
>  #endif
>  #include <arpa/inet.h>
> +#ifdef SYSTEMD
> +#include <systemd/sd-daemon.h>
> +#endif
>  #include <fcntl.h>
>  #include <netdb.h>
>  #include <stdio.h>
> @@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
>  	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
>  	struct sockaddr_un sun;
>  	mode_t oldmask;
> +	int n;
>          res = NULL;
>  
>  	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
> @@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
>  			fprintf(stderr, "[%d] - %s\n", i, *s);
>  	}
>  #endif
> +	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> +		syslog(LOG_ERR, "cannot get information for %s",
> +		    nconf->nc_netid);
> +		return (1);
> +	}
> +
> +#ifdef SYSTEMD
> +	n = sd_listen_fds(0);
> +	if (n < 0) {
> +		syslog(LOG_ERR, "failed to acquire systemd sockets: %s", strerror(-n));
> +		return 1;
> +	}
> +
> +	/* Try to find if one of the systemd sockets we were given match
> +	 * our netconfig structure. */
> +
> +	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
> +		struct __rpc_sockinfo si_other;
> +		union {
> +			struct sockaddr sa;
> +			struct sockaddr_un un;
> +			struct sockaddr_in in4;
> +			struct sockaddr_in6 in6;
> +			struct sockaddr_storage storage;
> +		} sa;
> +		socklen_t addrlen = sizeof(sa);
> +
> +		if (!__rpc_fd2sockinfo(fd, &si_other)) {
> +			syslog(LOG_ERR, "cannot get information for fd %i", fd);
> +			return 1;
> +		}
> +
> +		if (si.si_af != si_other.si_af ||
> +                    si.si_socktype != si_other.si_socktype ||
> +                    si.si_proto != si_other.si_proto)
> +			continue;
If I understand correctly, this ignores the socket if it is 
different than expected. Shouldn't this result in an error instead?

> +
> +		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
> +			syslog(LOG_ERR, "failed to query socket name: %s",
> +                               strerror(errno));
> +			goto error;
> +		}
> +
> +		/* Copy the address */
> +		taddr.addr.maxlen = taddr.addr.len = addrlen;
> +		taddr.addr.buf = malloc(addrlen);
> +		if (taddr.addr.buf == NULL) {
> +			syslog(LOG_ERR,
> +                               "cannot allocate memory for %s address",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +		memcpy(taddr.addr.buf, &sa, addrlen);
> +
> +		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> +                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +		if (my_xprt == (SVCXPRT *)NULL) {
> +			syslog(LOG_ERR, "%s: could not create service",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +	}
> +
> +	/*
> +	 * If none of the systemd sockets matched, we set up the socket in
> +	 * the normal way:
> +	 */
> +#endif
> +	if (my_xprt != NULL)
> +		goto got_socket;
>  
>  	/*
>  	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
> @@ -327,12 +401,6 @@ init_transport(struct netconfig *nconf)
>  		}
>  	}
>  
> -	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> -		syslog(LOG_ERR, "cannot get information for %s",
> -		    nconf->nc_netid);
> -		return (1);
> -	}
> -
>  	if ((strcmp(nconf->nc_netid, "local") == 0) ||
>  	    (strcmp(nconf->nc_netid, "unix") == 0)) {
>  		memset(&sun, 0, sizeof sun);
> @@ -569,6 +637,7 @@ init_transport(struct netconfig *nconf)
>  			goto error;
>  		}
>  	}
> +got_socket:
>  
>  #ifdef PORTMAP
>  	/*

Zbyszek

Re: [systemd-devel] [PATCH] rpcbind: add support for systemd socket activation

[Steve Dickson]

Hello,

On 11/22/2014 09:24 PM, Zbigniew Jędrzejewski-Szmek wrote:
> On Fri, Nov 21, 2014 at 11:43:47AM -0500, Steve Dickson wrote:
>> From: Tom Gundersen <teg@jklm.no>
>>
>> Making rpcbind sockect activated will greatly simplify
>> its integration in systemd systems. In essence, other services
>> may now assume that rpcbind is always available, even during very
>> early boot. This means that we no longer need to worry about any
>> ordering dependencies.
>>
>> This is based on a patch originally posted by Lennart Poettering:
>> <http://permalink.gmane.org/gmane.linux.nfs/33774>.
>>
>> That patch was not merged due to the lack of a shared library and
>> as systemd was seen to be too Fedora specific.
>>
>> Systemd now provides a shared library, and it is (or very soon will
>> be) the default init system on all the major Linux distributions.
>>
>> This version of the patch has three changes from the original:
>>
>>  * It uses the shared library.
>>  * It comes with unit files.
>>  * It is rebased on top of master.
>>
>> Please review the patch with "git show -b" or otherwise ignoring the
>> whitespace changes, or it will be extremely difficult to read.
> 
> Thanks for working on this... Looks fine to me. Two suggestions
> below.
np... 

> 
>> v4: reorganized the changes to make the diff easier to read
>>     remove systemd scripts.
>>
>> v3: rebase
>>     fix typos
>>     listen on /run/rpcbind.sock, rather than /var/run/rpcbind.sock (the
>>     latter is a symlink to the former, but this means the socket can be
>>     created before /var is mounted)
>>     NB: this version has been compile-tested only as I no longer use
>>     rpcbind myself
>> v2: correctly enable systemd code at compile time
>>     handle the case where not all the required sockets were supplied
>>     listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
>>     do not daemonize
>>
>> Original-patch-by: Lennart Poettering <lennart@poettering.net>
>> Cc: Steve Dickson <steved@redhat.com>
>> Cc: systemd-devel@lists.freedesktop.org
>> Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
>> Signed-off-by: Tom Gundersen <teg@jklm.no>
>> Signed-off-by: Steve Dickson <steved@redhat.com>
>> ---
>>  Makefile.am   |  6 +++++
>>  configure.ac  | 10 ++++++++
>>  src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>  3 files changed, 91 insertions(+), 6 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 8715082..c99566d 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -41,6 +41,12 @@ rpcbind_SOURCES = \
>>  	src/warmstart.c
>>  rpcbind_LDADD = $(TIRPC_LIBS)
>>  
>> +if SYSTEMD
>> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
>> +
>> +rpcbind_LDADD += $(SYSTEMD_LIBS)
>> +endif
>> +
>>  rpcinfo_SOURCES =       src/rpcinfo.c
>>  rpcinfo_LDADD   =       $(TIRPC_LIBS)
>>  
>> diff --git a/configure.ac b/configure.ac
>> index 5a88cc7..fdad639 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -36,6 +36,16 @@ AC_SUBST([nss_modules], [$with_nss_modules])
>>  
>>  PKG_CHECK_MODULES([TIRPC], [libtirpc])
>>  
>> +PKG_PROG_PKG_CONFIG
>> +AC_ARG_WITH([systemdsystemunitdir],
>> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
>> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
>> +  if test "x$with_systemdsystemunitdir" != xno; then
>> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
>> +    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> libsystemd-daemon got subsummed by libsystemd. So you should use something
> like
> 
>    PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [],
>       [PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [],
>        AC_MSG_ERROR([libsystemd support requested but found]))])
> 
> to get the new libary in preference to the old one.
Got it... 

> 
>> +  fi
>> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
>> +
>>  AS_IF([test x$enable_libwrap = xyes], [
>>  	AC_CHECK_LIB([wrap], [hosts_access], ,
>>  		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
>> diff --git a/src/rpcbind.c b/src/rpcbind.c
>> index e3462e3..f7c71ee 100644
>> --- a/src/rpcbind.c
>> +++ b/src/rpcbind.c
>> @@ -56,6 +56,9 @@
>>  #include <netinet/in.h>
>>  #endif
>>  #include <arpa/inet.h>
>> +#ifdef SYSTEMD
>> +#include <systemd/sd-daemon.h>
>> +#endif
>>  #include <fcntl.h>
>>  #include <netdb.h>
>>  #include <stdio.h>
>> @@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
>>  	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
>>  	struct sockaddr_un sun;
>>  	mode_t oldmask;
>> +	int n;
>>          res = NULL;
>>  
>>  	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
>> @@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
>>  			fprintf(stderr, "[%d] - %s\n", i, *s);
>>  	}
>>  #endif
>> +	if (!__rpc_nconf2sockinfo(nconf, &si)) {
>> +		syslog(LOG_ERR, "cannot get information for %s",
>> +		    nconf->nc_netid);
>> +		return (1);
>> +	}
>> +
>> +#ifdef SYSTEMD
>> +	n = sd_listen_fds(0);
>> +	if (n < 0) {
>> +		syslog(LOG_ERR, "failed to acquire systemd sockets: %s", strerror(-n));
>> +		return 1;
>> +	}
>> +
>> +	/* Try to find if one of the systemd sockets we were given match
>> +	 * our netconfig structure. */
>> +
>> +	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
>> +		struct __rpc_sockinfo si_other;
>> +		union {
>> +			struct sockaddr sa;
>> +			struct sockaddr_un un;
>> +			struct sockaddr_in in4;
>> +			struct sockaddr_in6 in6;
>> +			struct sockaddr_storage storage;
>> +		} sa;
>> +		socklen_t addrlen = sizeof(sa);
>> +
>> +		if (!__rpc_fd2sockinfo(fd, &si_other)) {
>> +			syslog(LOG_ERR, "cannot get information for fd %i", fd);
>> +			return 1;
>> +		}
>> +
>> +		if (si.si_af != si_other.si_af ||
>> +                    si.si_socktype != si_other.si_socktype ||
>> +                    si.si_proto != si_other.si_proto)
>> +			continue;
> If I understand correctly, this ignores the socket if it is 
> different than expected. Shouldn't this result in an error instead?
No. The only systemd fd is the "local" interface. The tcp,tcp6,udp and udp6
(see /etc/netconfig) interfaces are not systemd fds so they need to be 
created in the normal way. 

Thanks for the cycles!

steved.

> 
>> +
>> +		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
>> +			syslog(LOG_ERR, "failed to query socket name: %s",
>> +                               strerror(errno));
>> +			goto error;
>> +		}
>> +
>> +		/* Copy the address */
>> +		taddr.addr.maxlen = taddr.addr.len = addrlen;
>> +		taddr.addr.buf = malloc(addrlen);
>> +		if (taddr.addr.buf == NULL) {
>> +			syslog(LOG_ERR,
>> +                               "cannot allocate memory for %s address",
>> +                               nconf->nc_netid);
>> +			goto error;
>> +		}
>> +		memcpy(taddr.addr.buf, &sa, addrlen);
>> +
>> +		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
>> +                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
>> +		if (my_xprt == (SVCXPRT *)NULL) {
>> +			syslog(LOG_ERR, "%s: could not create service",
>> +                               nconf->nc_netid);
>> +			goto error;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * If none of the systemd sockets matched, we set up the socket in
>> +	 * the normal way:
>> +	 */
>> +#endif
>> +	if (my_xprt != NULL)
>> +		goto got_socket;
>>  
>>  	/*
>>  	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
>> @@ -327,12 +401,6 @@ init_transport(struct netconfig *nconf)
>>  		}
>>  	}
>>  
>> -	if (!__rpc_nconf2sockinfo(nconf, &si)) {
>> -		syslog(LOG_ERR, "cannot get information for %s",
>> -		    nconf->nc_netid);
>> -		return (1);
>> -	}
>> -
>>  	if ((strcmp(nconf->nc_netid, "local") == 0) ||
>>  	    (strcmp(nconf->nc_netid, "unix") == 0)) {
>>  		memset(&sun, 0, sizeof sun);
>> @@ -569,6 +637,7 @@ init_transport(struct netconfig *nconf)
>>  			goto error;
>>  		}
>>  	}
>> +got_socket:
>>  
>>  #ifdef PORTMAP
>>  	/*
> 
> Zbyszek
> 

[PATCH] rpcbind: systemd socket activation (v2)

[Steve Dickson]

This is based on a patch originally posted by Lennart Poettering:
<http://permalink.gmane.org/gmane.linux.nfs/33774>.

That patch was not merged due to the lack of a shared library and
as systemd was seen to be too Fedora specific.

Systemd now provides a shared library, and it is (or very soon will
be) the default init system on all the major Linux distributions.

This version of the patch has three changes from the original:

 * It uses the shared library.
 * It comes with unit files.
 * It is rebased on top of master.

Please review the patch with "git show -b" or otherwise ignoring the
whitespace changes, or it will be extremely difficult to read.

v5: incorporated comments on the PKG_CHECK_MODULES macro.

v4: reorganized the changes to make the diff easier to read
	remove systemd scripts.

v3: rebase
	fix typos
	listen on /run/rpcbind.sock, rather than /var/run/rpcbind.sock (the
	latter is a symlink to the former, but this means the socket can be
	created before /var is mounted)
	NB: this version has been compile-tested only as I no longer use
	rpcbind myself
v2: correctly enable systemd code at compile time
	handle the case where not all the required sockets were supplied
	listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
	do not daemonize

Tom Gundersen (1):
  rpcbind: add support for systemd socket activation

 Makefile.am   |  6 +++++
 configure.ac  | 12 +++++++++
 src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 93 insertions(+), 6 deletions(-)

-- 
1.9.3

[PATCH] rpcbind: add support for systemd socket activation

[Steve Dickson]

From: Tom Gundersen <teg@jklm.no>

Making rpcbind sockect activated will greatly simplify
its integration in systemd systems. In essence, other services
may now assume that rpcbind is always available, even during very
early boot. This means that we no longer need to worry about any
ordering dependencies.

Original-patch-by: Lennart Poettering <lennart@poettering.net>
Cc: systemd-devel@lists.freedesktop.org
Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
Signed-off-by: Tom Gundersen <teg@jklm.no>
Signed-off-by: Steve Dickson <steved@redhat.com>
---
 Makefile.am   |  6 +++++
 configure.ac  | 12 +++++++++
 src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8715082..c99566d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,6 +41,12 @@ rpcbind_SOURCES = \
 	src/warmstart.c
 rpcbind_LDADD = $(TIRPC_LIBS)
 
+if SYSTEMD
+AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
+
+rpcbind_LDADD += $(SYSTEMD_LIBS)
+endif
+
 rpcinfo_SOURCES =       src/rpcinfo.c
 rpcinfo_LDADD   =       $(TIRPC_LIBS)
 
diff --git a/configure.ac b/configure.ac
index 5a88cc7..967eb05 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,18 @@ AC_SUBST([nss_modules], [$with_nss_modules])
 
 PKG_CHECK_MODULES([TIRPC], [libtirpc])
 
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemdsystemunitdir],
+  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
+  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
+  if test "x$with_systemdsystemunitdir" != xno; then
+    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+     PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [],
+	   [PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [],
+	   AC_MSG_ERROR([libsystemd support requested but found]))])
+  fi
+AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
+
 AS_IF([test x$enable_libwrap = xyes], [
 	AC_CHECK_LIB([wrap], [hosts_access], ,
 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
diff --git a/src/rpcbind.c b/src/rpcbind.c
index e3462e3..f7c71ee 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -56,6 +56,9 @@
 #include <netinet/in.h>
 #endif
 #include <arpa/inet.h>
+#ifdef SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
 #include <fcntl.h>
 #include <netdb.h>
 #include <stdio.h>
@@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
 	struct sockaddr_un sun;
 	mode_t oldmask;
+	int n;
         res = NULL;
 
 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
 			fprintf(stderr, "[%d] - %s\n", i, *s);
 	}
 #endif
+	if (!__rpc_nconf2sockinfo(nconf, &si)) {
+		syslog(LOG_ERR, "cannot get information for %s",
+		    nconf->nc_netid);
+		return (1);
+	}
+
+#ifdef SYSTEMD
+	n = sd_listen_fds(0);
+	if (n < 0) {
+		syslog(LOG_ERR, "failed to acquire systemd sockets: %s", strerror(-n));
+		return 1;
+	}
+
+	/* Try to find if one of the systemd sockets we were given match
+	 * our netconfig structure. */
+
+	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+		struct __rpc_sockinfo si_other;
+		union {
+			struct sockaddr sa;
+			struct sockaddr_un un;
+			struct sockaddr_in in4;
+			struct sockaddr_in6 in6;
+			struct sockaddr_storage storage;
+		} sa;
+		socklen_t addrlen = sizeof(sa);
+
+		if (!__rpc_fd2sockinfo(fd, &si_other)) {
+			syslog(LOG_ERR, "cannot get information for fd %i", fd);
+			return 1;
+		}
+
+		if (si.si_af != si_other.si_af ||
+                    si.si_socktype != si_other.si_socktype ||
+                    si.si_proto != si_other.si_proto)
+			continue;
+
+		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+			syslog(LOG_ERR, "failed to query socket name: %s",
+                               strerror(errno));
+			goto error;
+		}
+
+		/* Copy the address */
+		taddr.addr.maxlen = taddr.addr.len = addrlen;
+		taddr.addr.buf = malloc(addrlen);
+		if (taddr.addr.buf == NULL) {
+			syslog(LOG_ERR,
+                               "cannot allocate memory for %s address",
+                               nconf->nc_netid);
+			goto error;
+		}
+		memcpy(taddr.addr.buf, &sa, addrlen);
+
+		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+		if (my_xprt == (SVCXPRT *)NULL) {
+			syslog(LOG_ERR, "%s: could not create service",
+                               nconf->nc_netid);
+			goto error;
+		}
+	}
+
+	/*
+	 * If none of the systemd sockets matched, we set up the socket in
+	 * the normal way:
+	 */
+#endif
+	if (my_xprt != NULL)
+		goto got_socket;
 
 	/*
 	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
@@ -327,12 +401,6 @@ init_transport(struct netconfig *nconf)
 		}
 	}
 
-	if (!__rpc_nconf2sockinfo(nconf, &si)) {
-		syslog(LOG_ERR, "cannot get information for %s",
-		    nconf->nc_netid);
-		return (1);
-	}
-
 	if ((strcmp(nconf->nc_netid, "local") == 0) ||
 	    (strcmp(nconf->nc_netid, "unix") == 0)) {
 		memset(&sun, 0, sizeof sun);
@@ -569,6 +637,7 @@ init_transport(struct netconfig *nconf)
 			goto error;
 		}
 	}
+got_socket:
 
 #ifdef PORTMAP
 	/*
-- 
1.9.3

Re: [PATCH] rpcbind: add support for systemd socket activation

[Steve Dickson]

On 11/25/2014 12:05 PM, Steve Dickson wrote:
> From: Tom Gundersen <teg@jklm.no>
> 
> Making rpcbind sockect activated will greatly simplify
> its integration in systemd systems. In essence, other services
> may now assume that rpcbind is always available, even during very
> early boot. This means that we no longer need to worry about any
> ordering dependencies.
> 
> Original-patch-by: Lennart Poettering <lennart@poettering.net>
> Cc: systemd-devel@lists.freedesktop.org
> Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
> Signed-off-by: Tom Gundersen <teg@jklm.no>
> Signed-off-by: Steve Dickson <steved@redhat.com>
Committed... 

steved.
> ---
>  Makefile.am   |  6 +++++
>  configure.ac  | 12 +++++++++
>  src/rpcbind.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 93 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 8715082..c99566d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -41,6 +41,12 @@ rpcbind_SOURCES = \
>  	src/warmstart.c
>  rpcbind_LDADD = $(TIRPC_LIBS)
>  
> +if SYSTEMD
> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
> +
> +rpcbind_LDADD += $(SYSTEMD_LIBS)
> +endif
> +
>  rpcinfo_SOURCES =       src/rpcinfo.c
>  rpcinfo_LDADD   =       $(TIRPC_LIBS)
>  
> diff --git a/configure.ac b/configure.ac
> index 5a88cc7..967eb05 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -36,6 +36,18 @@ AC_SUBST([nss_modules], [$with_nss_modules])
>  
>  PKG_CHECK_MODULES([TIRPC], [libtirpc])
>  
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemdsystemunitdir],
> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
> +  if test "x$with_systemdsystemunitdir" != xno; then
> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
> +     PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [],
> +	   [PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [],
> +	   AC_MSG_ERROR([libsystemd support requested but found]))])
> +  fi
> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
> +
>  AS_IF([test x$enable_libwrap = xyes], [
>  	AC_CHECK_LIB([wrap], [hosts_access], ,
>  		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index e3462e3..f7c71ee 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -56,6 +56,9 @@
>  #include <netinet/in.h>
>  #endif
>  #include <arpa/inet.h>
> +#ifdef SYSTEMD
> +#include <systemd/sd-daemon.h>
> +#endif
>  #include <fcntl.h>
>  #include <netdb.h>
>  #include <stdio.h>
> @@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
>  	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
>  	struct sockaddr_un sun;
>  	mode_t oldmask;
> +	int n;
>          res = NULL;
>  
>  	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
> @@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
>  			fprintf(stderr, "[%d] - %s\n", i, *s);
>  	}
>  #endif
> +	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> +		syslog(LOG_ERR, "cannot get information for %s",
> +		    nconf->nc_netid);
> +		return (1);
> +	}
> +
> +#ifdef SYSTEMD
> +	n = sd_listen_fds(0);
> +	if (n < 0) {
> +		syslog(LOG_ERR, "failed to acquire systemd sockets: %s", strerror(-n));
> +		return 1;
> +	}
> +
> +	/* Try to find if one of the systemd sockets we were given match
> +	 * our netconfig structure. */
> +
> +	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
> +		struct __rpc_sockinfo si_other;
> +		union {
> +			struct sockaddr sa;
> +			struct sockaddr_un un;
> +			struct sockaddr_in in4;
> +			struct sockaddr_in6 in6;
> +			struct sockaddr_storage storage;
> +		} sa;
> +		socklen_t addrlen = sizeof(sa);
> +
> +		if (!__rpc_fd2sockinfo(fd, &si_other)) {
> +			syslog(LOG_ERR, "cannot get information for fd %i", fd);
> +			return 1;
> +		}
> +
> +		if (si.si_af != si_other.si_af ||
> +                    si.si_socktype != si_other.si_socktype ||
> +                    si.si_proto != si_other.si_proto)
> +			continue;
> +
> +		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
> +			syslog(LOG_ERR, "failed to query socket name: %s",
> +                               strerror(errno));
> +			goto error;
> +		}
> +
> +		/* Copy the address */
> +		taddr.addr.maxlen = taddr.addr.len = addrlen;
> +		taddr.addr.buf = malloc(addrlen);
> +		if (taddr.addr.buf == NULL) {
> +			syslog(LOG_ERR,
> +                               "cannot allocate memory for %s address",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +		memcpy(taddr.addr.buf, &sa, addrlen);
> +
> +		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> +                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +		if (my_xprt == (SVCXPRT *)NULL) {
> +			syslog(LOG_ERR, "%s: could not create service",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +	}
> +
> +	/*
> +	 * If none of the systemd sockets matched, we set up the socket in
> +	 * the normal way:
> +	 */
> +#endif
> +	if (my_xprt != NULL)
> +		goto got_socket;
>  
>  	/*
>  	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
> @@ -327,12 +401,6 @@ init_transport(struct netconfig *nconf)
>  		}
>  	}
>  
> -	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> -		syslog(LOG_ERR, "cannot get information for %s",
> -		    nconf->nc_netid);
> -		return (1);
> -	}
> -
>  	if ((strcmp(nconf->nc_netid, "local") == 0) ||
>  	    (strcmp(nconf->nc_netid, "unix") == 0)) {
>  		memset(&sun, 0, sizeof sun);
> @@ -569,6 +637,7 @@ init_transport(struct netconfig *nconf)
>  			goto error;
>  		}
>  	}
> +got_socket:
>  
>  #ifdef PORTMAP
>  	/*
> 

Re: [PATCH v2] rpcbind: add support for systemd socket activation

[Cristian Rodriguez]

On 07/12/11 12:18, Tom Gundersen wrote:
> Making rpcbind sockect activated will greatly simplify
> its integration in systemd systems. In essence, other services
> may now assume that rpcbind is always available, even during very
> early boot. This means that we no longer need to worry about any
> ordering dependencies.
>
> This is based on a patch originally posted by Lennart Poettering:
> <http://permalink.gmane.org/gmane.linux.nfs/33774>.
>
> That patch was not merged due to the lack of a shared library and
> as systemd was seen to be too Fedora specific.
>
> Systemd now provides a shared library, and it is shipped by defalt in
> OpenSUSE in addition to Fedora, and it is available in Debain, Gentoo,
> Arch, and others.
>
> This version of the patch has three changes from the original:
>
>   * It uses the shared library.
>   * It comes with unit files.
>   * It is rebased on top of master.
>
> Please review the patch with "git show -b" or otherwise ignoring the
> whitespace changes, or it will be extremely difficult to read.


Thanks for your effort, however, this patch doesnt appear to work..

- There is a missing AC_DEFINE..



   if test "x$with_systemdsystemunitdir" != xno; then

.. (various tests)
 
 
 
 
 

AC_DEFINE([SYSTEMD], [1], [has systemd]) 
 
 

fi

Otherwise SYSTEMD is never defined in CPPFLAGS and the code is skipped.

Also, after fixing this particular nit, I am stuck in an assetion 
failure starting rpcbind

Dec 19 20:00:05 linux-lu80 rpcbind[2013]: rpcbind: svc.c:298: 
svc_register: Assertion `xprt != ((void *)0)' failed.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service: main process 
exited, code=killed, status=6
Dec 19 20:00:05 linux-lu80 systemd[1]: Unit rpcbind.service entered 
failed state.
Dec 19 20:00:05 linux-lu80 rpcbind[2015]: rpcbind: svc.c:298: 
svc_register: Assertion `xprt != ((void *)0)' failed.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service: main process 
exited, code=killed, status=6
Dec 19 20:00:05 linux-lu80 systemd[1]: Unit rpcbind.service entered 
failed state.
Dec 19 20:00:05 linux-lu80 rpcbind[2017]: rpcbind: svc.c:298: 
svc_register: Assertion `xprt != ((void *)0)' failed.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service: main process 
exited, code=killed, status=6
Dec 19 20:00:05 linux-lu80 systemd[1]: Unit rpcbind.service entered 
failed state.
Dec 19 20:00:05 linux-lu80 rpcbind[2019]: rpcbind: svc.c:298: 
svc_register: Assertion `xprt != ((void *)0)' failed.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service: main process 
exited, code=killed, status=6
Dec 19 20:00:05 linux-lu80 systemd[1]: Unit rpcbind.service entered 
failed state.
Dec 19 20:00:05 linux-lu80 rpcbind[2021]: rpcbind: svc.c:298: 
svc_register: Assertion `xprt != ((void *)0)' failed.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service: main process 
exited, code=killed, status=6
Dec 19 20:00:05 linux-lu80 systemd[1]: Unit rpcbind.service entered 
failed state.
Dec 19 20:00:05 linux-lu80 rpcbind[2023]: rpcbind: svc.c:298: 
svc_register: Assertion `xprt != ((void *)0)' failed.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service: main process 
exited, code=killed, status=6
Dec 19 20:00:05 linux-lu80 systemd[1]: Unit rpcbind.service entered 
failed state.
Dec 19 20:00:05 linux-lu80 systemd[1]: rpcbind.service start request 
repeated too quickly, refusing to start.

[PATCH] rpcbind: add support for systemd socket activation

[Tom Gundersen]

Making rpcbind sockect activated will greatly simplify
its integration in systemd systems. In essence, other services
may now assume that rpcbind is always available, even during very
early boot. This means that we no longer need to worry about any
ordering dependencies.

This is based on a patch originally posted by Lennart Poettering:
<http://permalink.gmane.org/gmane.linux.nfs/33774>.

That patch was not merged due to the lack of a shared library and
as systemd was seen to be too Fedora specific.

Systemd now provides a shared library, and it is shipped by defalt in
OpenSUSE in addition to Fedora, and it is available in Debain, Gentoo,
Arch, and others.

This version of the patch has three changes from the original:

 * It uses the shared library.
 * It comes with unit files.
 * It is rebased on top of master.

Please review the patch with "git show -b" or otherwise ignoring the
whitespace changes, or it will be extremely difficult to read.

Comments welcome.

v2: correctly enable systemd code at compile time
    handle the case where not all the required sockets were supplied
    listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
    do not daemonize

Original-patch-by: Lennart Poettering <lennart@poettering.net>
Cc: Steve Dickson <steved@redhat.com>
Cc: systemd-devel@lists.freedesktop.org
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Signed-off-by: Tom Gundersen <teg@jklm.no>
---

Thanks to Cristian for testing. The testcase I had been using was entirely flawed,
the code did in fact not work at all. Sorry about that!

This time around it should work :-)

 Makefile.am                |   15 ++
 configure.in               |   11 +
 src/rpcbind.c              |  467 +++++++++++++++++++++++++-------------------
 systemd/.gitignore         |    1 +
 systemd/rpcbind.service.in |    9 +
 systemd/rpcbind.socket     |   12 ++
 6 files changed, 316 insertions(+), 199 deletions(-)
 create mode 100644 systemd/.gitignore
 create mode 100644 systemd/rpcbind.service.in
 create mode 100644 systemd/rpcbind.socket

diff --git a/Makefile.am b/Makefile.am
index 9fa608e..194b467 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -38,6 +38,21 @@ rpcbind_SOURCES = \
 	src/warmstart.c
 rpcbind_LDADD = $(TIRPC_LIBS)
 
+if SYSTEMD
+AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
+
+rpcbind_LDADD += $(SYSTEMD_LIBS)
+
+systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
+	sed -e 's,@bindir\@,$(bindir),g' \
+		< $< > $@ || rm $@
+
+systemdsystemunit_DATA = \
+	systemd/rpcbind.service \
+	systemd/rpcbind.socket
+
+endif
+
 rpcinfo_SOURCES =       src/rpcinfo.c
 rpcinfo_LDADD   =       $(TIRPC_LIBS)
 
diff --git a/configure.in b/configure.in
index 2b67720..397d52d 100644
--- a/configure.in
+++ b/configure.in
@@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])
  
 PKG_CHECK_MODULES([TIRPC], [libtirpc])
 
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemdsystemunitdir],
+  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
+  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
+  if test "x$with_systemdsystemunitdir" != xno; then
+    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
+  fi
+AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
+
+
 AS_IF([test x$enable_libwrap = xyes], [
 	AC_CHECK_LIB([wrap], [hosts_access], ,
 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
diff --git a/src/rpcbind.c b/src/rpcbind.c
index 24e069b..a87ce05 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -56,6 +56,9 @@
 #include <netinet/in.h>
 #endif
 #include <arpa/inet.h>
+#ifdef SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
 #include <fcntl.h>
 #include <netdb.h>
 #include <stdio.h>
@@ -281,6 +284,7 @@ init_transport(struct netconfig *nconf)
 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
 	struct sockaddr_un sun;
 	mode_t oldmask;
+	int n = 0;
         res = NULL;
 
 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -300,141 +304,285 @@ init_transport(struct netconfig *nconf)
 	}
 #endif
 
-	/*
-	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
-	 * we call this later, for each socket we like to bind.
-	 */
-	if (nconf->nc_semantics != NC_TPI_CLTS) {
-		if ((fd = __rpc_nconf2fd(nconf)) < 0) {
-			syslog(LOG_ERR, "cannot create socket for %s",
-			    nconf->nc_netid);
-			return (1);
-		}
-	}
-
 	if (!__rpc_nconf2sockinfo(nconf, &si)) {
 		syslog(LOG_ERR, "cannot get information for %s",
 		    nconf->nc_netid);
 		return (1);
 	}
 
-	if ((strcmp(nconf->nc_netid, "local") == 0) ||
-	    (strcmp(nconf->nc_netid, "unix") == 0)) {
-		memset(&sun, 0, sizeof sun);
-		sun.sun_family = AF_LOCAL;
-		unlink(_PATH_RPCBINDSOCK);
-		strcpy(sun.sun_path, _PATH_RPCBINDSOCK);
-		addrlen = SUN_LEN(&sun);
-		sa = (struct sockaddr *)&sun;
-	} else {
-		/* Get rpcbind's address on this transport */
-
-		memset(&hints, 0, sizeof hints);
-		hints.ai_flags = AI_PASSIVE;
-		hints.ai_family = si.si_af;
-		hints.ai_socktype = si.si_socktype;
-		hints.ai_protocol = si.si_proto;
+#ifdef SYSTEMD
+	n = sd_listen_fds(0);
+	if (n < 0) {
+		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
+		return 1;
 	}
-	if (nconf->nc_semantics == NC_TPI_CLTS) {
-		/*
-		 * If no hosts were specified, just bind to INADDR_ANY.  Otherwise
-		 * make sure 127.0.0.1 is added to the list.
-		 */
-		nhostsbak = nhosts;
-		nhostsbak++;
-		hosts = realloc(hosts, nhostsbak * sizeof(char *));
-		if (nhostsbak == 1)
-			hosts[0] = "*";
-		else {
-			if (hints.ai_family == AF_INET) {
-				hosts[nhostsbak - 1] = "127.0.0.1";
-			} else if (hints.ai_family == AF_INET6) {
-				hosts[nhostsbak - 1] = "::1";
-			} else
-				return 1;
+
+	/* Try to find if one of the systemd sockets we were given match
+	 * our netconfig structure. */
+
+	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+		struct __rpc_sockinfo si_other;
+		union {
+			struct sockaddr sa;
+			struct sockaddr_un un;
+			struct sockaddr_in in4;
+			struct sockaddr_in6 in6;
+			struct sockaddr_storage storage;
+		} sa;
+		socklen_t addrlen = sizeof(sa);
+
+		if (!__rpc_fd2sockinfo(fd, &si_other)) {
+			syslog(LOG_ERR, "cannot get information for fd %i", fd);
+			return 1;
 		}
 
-	       /*
-		* Bind to specific IPs if asked to
-		*/
-		checkbind = 0;
-		while (nhostsbak > 0) {
-			--nhostsbak;
-			/*
-			 * XXX - using RPC library internal functions.
-			 */
+		if (si.si_af != si_other.si_af ||
+                    si.si_socktype != si_other.si_socktype ||
+                    si.si_proto != si_other.si_proto)
+			continue;
+
+		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+			syslog(LOG_ERR, "failed to query socket name: %s",
+                               strerror(errno));
+			goto error;
+		}
+
+		/* Copy the address */
+		taddr.addr.maxlen = taddr.addr.len = addrlen;
+		taddr.addr.buf = malloc(addrlen);
+		if (taddr.addr.buf == NULL) {
+			syslog(LOG_ERR,
+                               "cannot allocate memory for %s address",
+                               nconf->nc_netid);
+			goto error;
+		}
+		memcpy(taddr.addr.buf, &sa, addrlen);
+
+		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+		if (my_xprt == (SVCXPRT *)NULL) {
+			syslog(LOG_ERR, "%s: could not create service",
+                               nconf->nc_netid);
+			goto error;
+		}
+	}
+
+	/* if none of the systemd sockets matched, we set up the socket in
+	 * the normal way:
+	 */
+#endif
+
+	if(my_xprt == (SVCXPRT *)NULL) {
+
+		/*
+		 * XXX - using RPC library internal functions. For NC_TPI_CLTS
+		 * we call this later, for each socket we like to bind.
+		 */
+		if (nconf->nc_semantics != NC_TPI_CLTS) {
 			if ((fd = __rpc_nconf2fd(nconf)) < 0) {
 				syslog(LOG_ERR, "cannot create socket for %s",
 				    nconf->nc_netid);
 				return (1);
 			}
-			switch (hints.ai_family) {
-			case AF_INET:
-				if (inet_pton(AF_INET, hosts[nhostsbak],
-				    host_addr) == 1) {
-					hints.ai_flags &= AI_NUMERICHOST;
-				} else {
-					/*
-					 * Skip if we have an AF_INET6 adress.
-					 */
-					if (inet_pton(AF_INET6,
-					    hosts[nhostsbak], host_addr) == 1)
-						continue;
+		}
+
+		if ((strcmp(nconf->nc_netid, "local") == 0) ||
+		    (strcmp(nconf->nc_netid, "unix") == 0)) {
+			memset(&sun, 0, sizeof sun);
+			sun.sun_family = AF_LOCAL;
+			unlink(_PATH_RPCBINDSOCK);
+			strcpy(sun.sun_path, _PATH_RPCBINDSOCK);
+			addrlen = SUN_LEN(&sun);
+			sa = (struct sockaddr *)&sun;
+		} else {
+			/* Get rpcbind's address on this transport */
+
+			memset(&hints, 0, sizeof hints);
+			hints.ai_flags = AI_PASSIVE;
+			hints.ai_family = si.si_af;
+			hints.ai_socktype = si.si_socktype;
+			hints.ai_protocol = si.si_proto;
+		}
+		if (nconf->nc_semantics == NC_TPI_CLTS) {
+			/*
+			 * If no hosts were specified, just bind to INADDR_ANY.  Otherwise
+			 * make sure 127.0.0.1 is added to the list.
+			 */
+			nhostsbak = nhosts;
+			nhostsbak++;
+			hosts = realloc(hosts, nhostsbak * sizeof(char *));
+			if (nhostsbak == 1)
+				hosts[0] = "*";
+			else {
+				if (hints.ai_family == AF_INET) {
+					hosts[nhostsbak - 1] = "127.0.0.1";
+				} else if (hints.ai_family == AF_INET6) {
+					hosts[nhostsbak - 1] = "::1";
+				} else
+					return 1;
+			}
+
+		       /*
+			* Bind to specific IPs if asked to
+			*/
+			checkbind = 0;
+			while (nhostsbak > 0) {
+				--nhostsbak;
+				/*
+				 * XXX - using RPC library internal functions.
+				 */
+				if ((fd = __rpc_nconf2fd(nconf)) < 0) {
+					syslog(LOG_ERR, "cannot create socket for %s",
+					    nconf->nc_netid);
+					return (1);
+				}
+				switch (hints.ai_family) {
+				case AF_INET:
+					if (inet_pton(AF_INET, hosts[nhostsbak],
+					    host_addr) == 1) {
+						hints.ai_flags &= AI_NUMERICHOST;
+					} else {
+						/*
+						 * Skip if we have an AF_INET6 adress.
+						 */
+						if (inet_pton(AF_INET6,
+						    hosts[nhostsbak], host_addr) == 1)
+							continue;
+					}
+					break;
+				case AF_INET6:
+					if (inet_pton(AF_INET6, hosts[nhostsbak],
+					    host_addr) == 1) {
+						hints.ai_flags &= AI_NUMERICHOST;
+					} else {
+						/*
+						 * Skip if we have an AF_INET adress.
+						 */
+						if (inet_pton(AF_INET, hosts[nhostsbak],
+						    host_addr) == 1)
+							continue;
+					}
+					break;
+				default:
+					break;
 				}
-				break;
-			case AF_INET6:
-				if (inet_pton(AF_INET6, hosts[nhostsbak],
-				    host_addr) == 1) {
-					hints.ai_flags &= AI_NUMERICHOST;
-				} else {
+
+				/*
+				 * If no hosts were specified, just bind to INADDR_ANY
+				 */
+				if (strcmp("*", hosts[nhostsbak]) == 0)
+					hosts[nhostsbak] = NULL;
+
+				if ((aicode = getaddrinfo(hosts[nhostsbak],
+				    servname, &hints, &res)) != 0) {
+				  if ((aicode = getaddrinfo(hosts[nhostsbak],
+							    "portmapper", &hints, &res)) != 0) {
+					syslog(LOG_ERR,
+					    "cannot get local address for %s: %s",
+					    nconf->nc_netid, gai_strerror(aicode));
+					continue;
+				  }
+				}
+				addrlen = res->ai_addrlen;
+				sa = (struct sockaddr *)res->ai_addr;
+				oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
+				if (bind(fd, sa, addrlen) != 0) {
+					syslog(LOG_ERR, "cannot bind %s on %s: %m",
+						(hosts[nhostsbak] == NULL) ? "*" :
+						hosts[nhostsbak], nconf->nc_netid);
+					if (res != NULL)
+						freeaddrinfo(res);
+					continue;
+				} else
+					checkbind++;
+				(void) umask(oldmask);
+
+				/* Copy the address */
+				taddr.addr.maxlen = taddr.addr.len = addrlen;
+				taddr.addr.buf = malloc(addrlen);
+				if (taddr.addr.buf == NULL) {
+					syslog(LOG_ERR,
+					    "cannot allocate memory for %s address",
+					    nconf->nc_netid);
+					if (res != NULL)
+						freeaddrinfo(res);
+					return 1;
+				}
+				memcpy(taddr.addr.buf, sa, addrlen);
+#ifdef RPCBIND_DEBUG
+				if (debugging) {
 					/*
-					 * Skip if we have an AF_INET adress.
+					 * for debugging print out our universal
+					 * address
 					 */
-					if (inet_pton(AF_INET, hosts[nhostsbak],
-					    host_addr) == 1)
-						continue;
+					char *uaddr;
+					struct netbuf nb;
+					int sa_size = 0;
+
+					nb.buf = sa;
+					switch( sa->sa_family){
+					case AF_INET:
+					  sa_size = sizeof (struct sockaddr_in);
+					  break;
+					case AF_INET6:
+					  sa_size = sizeof (struct sockaddr_in6);
+					  break;
+					}
+					nb.len = nb.maxlen = sa_size;
+					uaddr = taddr2uaddr(nconf, &nb);
+					(void) fprintf(stderr,
+					    "rpcbind : my address is %s\n", uaddr);
+					(void) free(uaddr);
+				}
+#endif
+				my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+				if (my_xprt == (SVCXPRT *)NULL) {
+					syslog(LOG_ERR, "%s: could not create service",
+                                               nconf->nc_netid);
+					goto error;
 				}
-	        		break;
-			default:
-				break;
 			}
-
-			/*
-			 * If no hosts were specified, just bind to INADDR_ANY
-			 */
-			if (strcmp("*", hosts[nhostsbak]) == 0)
-				hosts[nhostsbak] = NULL;
-
-			if ((aicode = getaddrinfo(hosts[nhostsbak],
-			    servname, &hints, &res)) != 0) {
-			  if ((aicode = getaddrinfo(hosts[nhostsbak],
-						    "portmapper", &hints, &res)) != 0) {
-				syslog(LOG_ERR,
-				    "cannot get local address for %s: %s",
-				    nconf->nc_netid, gai_strerror(aicode));
-				continue;
-			  }
+			if (!checkbind)
+				return 1;
+		} else {	/* NC_TPI_COTS */
+			if ((strcmp(nconf->nc_netid, "local") != 0) &&
+			    (strcmp(nconf->nc_netid, "unix") != 0)) {
+				if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
+				  if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
+				  printf("cannot get local address for %s: %s",  nconf->nc_netid, gai_strerror(aicode));
+				  syslog(LOG_ERR,
+					    "cannot get local address for %s: %s",
+					    nconf->nc_netid, gai_strerror(aicode));
+					return 1;
+				  }
+				}
+				addrlen = res->ai_addrlen;
+				sa = (struct sockaddr *)res->ai_addr;
 			}
-			addrlen = res->ai_addrlen;
-			sa = (struct sockaddr *)res->ai_addr;
 			oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
-                        if (bind(fd, sa, addrlen) != 0) {
-				syslog(LOG_ERR, "cannot bind %s on %s: %m",
-					(hosts[nhostsbak] == NULL) ? "*" :
-					hosts[nhostsbak], nconf->nc_netid);
+			__rpc_fd2sockinfo(fd, &si);
+			if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
+					sizeof(on)) != 0) {
+				syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
+					nconf->nc_netid);
 				if (res != NULL)
 					freeaddrinfo(res);
-				continue;
-			} else
-				checkbind++;
+				return 1;
+			}
+			if (bind(fd, sa, addrlen) < 0) {
+				syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
+				if (res != NULL)
+					freeaddrinfo(res);
+				return 1;
+			}
 			(void) umask(oldmask);
 
 			/* Copy the address */
-			taddr.addr.maxlen = taddr.addr.len = addrlen;
+			taddr.addr.len = taddr.addr.maxlen = addrlen;
 			taddr.addr.buf = malloc(addrlen);
 			if (taddr.addr.buf == NULL) {
-				syslog(LOG_ERR,
-				    "cannot allocate memory for %s address",
+				syslog(LOG_ERR, "cannot allocate memory for %s address",
 				    nconf->nc_netid);
 				if (res != NULL)
 					freeaddrinfo(res);
@@ -443,116 +591,37 @@ init_transport(struct netconfig *nconf)
 			memcpy(taddr.addr.buf, sa, addrlen);
 #ifdef RPCBIND_DEBUG
 			if (debugging) {
-				/*
-				 * for debugging print out our universal
-				 * address
-				 */
+				/* for debugging print out our universal address */
 				char *uaddr;
 				struct netbuf nb;
-				int sa_size = 0;
+			        int sa_size2 = 0;
 
 				nb.buf = sa;
 				switch( sa->sa_family){
 				case AF_INET:
-				  sa_size = sizeof (struct sockaddr_in);
+				  sa_size2 = sizeof (struct sockaddr_in);
 				  break;
 				case AF_INET6:
-				  sa_size = sizeof (struct sockaddr_in6);				 
+				  sa_size2 = sizeof (struct sockaddr_in6);
 				  break;
 				}
-				nb.len = nb.maxlen = sa_size;
+				nb.len = nb.maxlen = sa_size2;
 				uaddr = taddr2uaddr(nconf, &nb);
-				(void) fprintf(stderr,
-				    "rpcbind : my address is %s\n", uaddr);
+				(void) fprintf(stderr, "rpcbind : my address is %s\n",
+				    uaddr);
 				(void) free(uaddr);
 			}
 #endif
-			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, 
-                                RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+
+			listen(fd, SOMAXCONN);
+
+			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
 			if (my_xprt == (SVCXPRT *)NULL) {
-				syslog(LOG_ERR, "%s: could not create service", 
-                                        nconf->nc_netid);
+				syslog(LOG_ERR, "%s: could not create service",
+						nconf->nc_netid);
 				goto error;
 			}
 		}
-		if (!checkbind)
-			return 1;
-	} else {	/* NC_TPI_COTS */
-		if ((strcmp(nconf->nc_netid, "local") != 0) &&
-		    (strcmp(nconf->nc_netid, "unix") != 0)) {
-			if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
-			  if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
-			  printf("cannot get local address for %s: %s",  nconf->nc_netid, gai_strerror(aicode));
-			  syslog(LOG_ERR,
-				    "cannot get local address for %s: %s",
-				    nconf->nc_netid, gai_strerror(aicode));
-				return 1;
-			  }
-			}
-			addrlen = res->ai_addrlen;
-			sa = (struct sockaddr *)res->ai_addr;
-		}
-		oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
-		__rpc_fd2sockinfo(fd, &si);
-		if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
-				sizeof(on)) != 0) {
-			syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
-				nconf->nc_netid);
-			if (res != NULL)
-				freeaddrinfo(res);
-			return 1;
-		}
-		if (bind(fd, sa, addrlen) < 0) {
-			syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
-			if (res != NULL)
-				freeaddrinfo(res);
-			return 1;
-		}
-		(void) umask(oldmask);
-
-		/* Copy the address */
-		taddr.addr.len = taddr.addr.maxlen = addrlen;
-		taddr.addr.buf = malloc(addrlen);
-		if (taddr.addr.buf == NULL) {
-			syslog(LOG_ERR, "cannot allocate memory for %s address",
-			    nconf->nc_netid);
-			if (res != NULL)
-				freeaddrinfo(res);
-			return 1;
-		}
-		memcpy(taddr.addr.buf, sa, addrlen);
-#ifdef RPCBIND_DEBUG
-		if (debugging) {
-			/* for debugging print out our universal address */
-			char *uaddr;
-			struct netbuf nb;
-		        int sa_size2 = 0;
-
-			nb.buf = sa;
-			switch( sa->sa_family){
-			case AF_INET:
-			  sa_size2 = sizeof (struct sockaddr_in);
-			  break;
-			case AF_INET6:
-			  sa_size2 = sizeof (struct sockaddr_in6);				 
-			  break;
-			}
-			nb.len = nb.maxlen = sa_size2;
-			uaddr = taddr2uaddr(nconf, &nb);
-			(void) fprintf(stderr, "rpcbind : my address is %s\n",
-			    uaddr);
-			(void) free(uaddr);
-		}
-#endif
-
-		listen(fd, SOMAXCONN);
-
-		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
-		if (my_xprt == (SVCXPRT *)NULL) {
-			syslog(LOG_ERR, "%s: could not create service",
-					nconf->nc_netid);
-			goto error;
-		}
 	}
 
 #ifdef PORTMAP
diff --git a/systemd/.gitignore b/systemd/.gitignore
new file mode 100644
index 0000000..b7b4561
--- /dev/null
+++ b/systemd/.gitignore
@@ -0,0 +1 @@
+rpcbind.service
diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
new file mode 100644
index 0000000..58ae5de
--- /dev/null
+++ b/systemd/rpcbind.service.in
@@ -0,0 +1,9 @@
+[Unit]
+Description=RPC Bind
+
+[Service]
+ExecStart=@bindir@/rpcbind -w -f
+
+[Install]
+WantedBy=multi-user.target
+Also=rpcbind.socket
diff --git a/systemd/rpcbind.socket b/systemd/rpcbind.socket
new file mode 100644
index 0000000..ad5fd62
--- /dev/null
+++ b/systemd/rpcbind.socket
@@ -0,0 +1,12 @@
+[Unit]
+Description=RPCbind Server Activation Socket
+Wants=rpcbind.target
+Before=rpcbind.target
+
+[Socket]
+ListenStream=/var/run/rpcbind.sock
+ListenStream=111
+ListenDatagram=111
+
+[Install]
+WantedBy=sockets.target
-- 
1.7.8

Re: [PATCH] rpcbind: add support for systemd socket activation

[Cristian Rodríguez]

On 22/12/11 22:05, Tom Gundersen wrote:
> Making rpcbind sockect activated will greatly simplify
> its integration in systemd systems. In essence, other services
> may now assume that rpcbind is always available, even during very
> early boot. This means that we no longer need to worry about any
> ordering dependencies.
>
> This is based on a patch originally posted by Lennart Poettering:
> <http://permalink.gmane.org/gmane.linux.nfs/33774>.
>
> That patch was not merged due to the lack of a shared library and
> as systemd was seen to be too Fedora specific.
>
> Systemd now provides a shared library, and it is shipped by defalt in
> OpenSUSE in addition to Fedora, and it is available in Debain, Gentoo,
> Arch, and others.
>
> This version of the patch has three changes from the original:
>
>   * It uses the shared library.
>   * It comes with unit files.
>   * It is rebased on top of master.
>
> Please review the patch with "git show -b" or otherwise ignoring the
> whitespace changes, or it will be extremely difficult to read.
>
> Comments welcome.
>
> v2: correctly enable systemd code at compile time
>      handle the case where not all the required sockets were supplied
>      listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
>      do not daemonize
>
> Original-patch-by: Lennart Poettering<lennart@poettering.net>
> Cc: Steve Dickson<steved@redhat.com>
> Cc: systemd-devel@lists.freedesktop.org
> Cc: Cristian Rodríguez<crrodriguez@opensuse.org>
> Signed-off-by: Tom Gundersen<teg@jklm.no>
> ---
>
> Thanks to Cristian for testing. The testcase I had been using was entirely flawed,
> the code did in fact not work at all. Sorry about that!

ACKed: Cristian Rodríguez<crrodriguez@opensuse.org>

This version works as expected here.

Re: [PATCH] rpcbind: add support for systemd socket activation

[Jim Rees]

Might it be time to break up init_transport?  I see you're already using
some non-standard indentation to keep the AF_INET6 case from dropping off
the right side of the screen.

[PATCH] rpcbind: add support for systemd socket activation

[Tom Gundersen]

Making rpcbind sockect activated will greatly simplify
its integration in systemd systems. In essence, other services
may now assume that rpcbind is always available, even during very
early boot. This means that we no longer need to worry about any
ordering dependencies.

This is based on a patch originally posted by Lennart Poettering:
<http://permalink.gmane.org/gmane.linux.nfs/33774>.

That patch was not merged due to the lack of a shared library and
as systemd was seen to be too Fedora specific.

Systemd now provides a shared library, and it is shipped by defalt in
OpenSUSE in addition to Fedora, and it is available in Debain, Gentoo,
Arch, and others.

This version of the patch has three changes from the original:

 * It uses the shared library.
 * It comes with unit files.
 * It is rebased on top of master.

Please review the patch with "git show -b" or otherwise ignoring the
whitespace changes, or it will be extremely difficult to read.

Comments welcome.

v2: correctly enable systemd code at compile time
    handle the case where not all the required sockets were supplied
    listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
    do not daemonize

Original-patch-by: Lennart Poettering <lennart@poettering.net>
Cc: Michal Schmidt <mschmidt@redhat.com>
Cc: Steve Dickson <steved@redhat.com>
Cc: systemd-devel@lists.freedesktop.org
Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
Signed-off-by: Tom Gundersen <teg@jklm.no>
---

What's the status on this?

Lennart, does your ack still appl after my changes?

Steve, any chance of applying this?

If this is applied I have a couple of follow ups. In particular, I'd like to
do the cleanup of init_transport that Jim suggested as a separate patch,
as leaving the line-wraps alone makes this patch easier to read I think.

Added Michal to cc as this patch should go a long way to sort out
<https://bugzilla.redhat.com/show_bug.cgi?id=747363>.

Cheers,

Tom

 Makefile.am                |   15 ++
 configure.in               |   11 +
 src/rpcbind.c              |  467 +++++++++++++++++++++++++-------------------
 systemd/.gitignore         |    1 +
 systemd/rpcbind.service.in |    9 +
 systemd/rpcbind.socket     |   12 ++
 6 files changed, 316 insertions(+), 199 deletions(-)
 create mode 100644 systemd/.gitignore
 create mode 100644 systemd/rpcbind.service.in
 create mode 100644 systemd/rpcbind.socket

diff --git a/Makefile.am b/Makefile.am
index 9fa608e..194b467 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -38,6 +38,21 @@ rpcbind_SOURCES = \
 	src/warmstart.c
 rpcbind_LDADD = $(TIRPC_LIBS)
 
+if SYSTEMD
+AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
+
+rpcbind_LDADD += $(SYSTEMD_LIBS)
+
+systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
+	sed -e 's,@bindir\@,$(bindir),g' \
+		< $< > $@ || rm $@
+
+systemdsystemunit_DATA = \
+	systemd/rpcbind.service \
+	systemd/rpcbind.socket
+
+endif
+
 rpcinfo_SOURCES =       src/rpcinfo.c
 rpcinfo_LDADD   =       $(TIRPC_LIBS)
 
diff --git a/configure.in b/configure.in
index 2b67720..397d52d 100644
--- a/configure.in
+++ b/configure.in
@@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])
  
 PKG_CHECK_MODULES([TIRPC], [libtirpc])
 
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemdsystemunitdir],
+  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
+  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
+  if test "x$with_systemdsystemunitdir" != xno; then
+    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
+  fi
+AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
+
+
 AS_IF([test x$enable_libwrap = xyes], [
 	AC_CHECK_LIB([wrap], [hosts_access], ,
 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
diff --git a/src/rpcbind.c b/src/rpcbind.c
index 24e069b..a87ce05 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -56,6 +56,9 @@
 #include <netinet/in.h>
 #endif
 #include <arpa/inet.h>
+#ifdef SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
 #include <fcntl.h>
 #include <netdb.h>
 #include <stdio.h>
@@ -281,6 +284,7 @@ init_transport(struct netconfig *nconf)
 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
 	struct sockaddr_un sun;
 	mode_t oldmask;
+	int n = 0;
         res = NULL;
 
 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -300,141 +304,285 @@ init_transport(struct netconfig *nconf)
 	}
 #endif
 
-	/*
-	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
-	 * we call this later, for each socket we like to bind.
-	 */
-	if (nconf->nc_semantics != NC_TPI_CLTS) {
-		if ((fd = __rpc_nconf2fd(nconf)) < 0) {
-			syslog(LOG_ERR, "cannot create socket for %s",
-			    nconf->nc_netid);
-			return (1);
-		}
-	}
-
 	if (!__rpc_nconf2sockinfo(nconf, &si)) {
 		syslog(LOG_ERR, "cannot get information for %s",
 		    nconf->nc_netid);
 		return (1);
 	}
 
-	if ((strcmp(nconf->nc_netid, "local") == 0) ||
-	    (strcmp(nconf->nc_netid, "unix") == 0)) {
-		memset(&sun, 0, sizeof sun);
-		sun.sun_family = AF_LOCAL;
-		unlink(_PATH_RPCBINDSOCK);
-		strcpy(sun.sun_path, _PATH_RPCBINDSOCK);
-		addrlen = SUN_LEN(&sun);
-		sa = (struct sockaddr *)&sun;
-	} else {
-		/* Get rpcbind's address on this transport */
-
-		memset(&hints, 0, sizeof hints);
-		hints.ai_flags = AI_PASSIVE;
-		hints.ai_family = si.si_af;
-		hints.ai_socktype = si.si_socktype;
-		hints.ai_protocol = si.si_proto;
+#ifdef SYSTEMD
+	n = sd_listen_fds(0);
+	if (n < 0) {
+		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
+		return 1;
 	}
-	if (nconf->nc_semantics == NC_TPI_CLTS) {
-		/*
-		 * If no hosts were specified, just bind to INADDR_ANY.  Otherwise
-		 * make sure 127.0.0.1 is added to the list.
-		 */
-		nhostsbak = nhosts;
-		nhostsbak++;
-		hosts = realloc(hosts, nhostsbak * sizeof(char *));
-		if (nhostsbak == 1)
-			hosts[0] = "*";
-		else {
-			if (hints.ai_family == AF_INET) {
-				hosts[nhostsbak - 1] = "127.0.0.1";
-			} else if (hints.ai_family == AF_INET6) {
-				hosts[nhostsbak - 1] = "::1";
-			} else
-				return 1;
+
+	/* Try to find if one of the systemd sockets we were given match
+	 * our netconfig structure. */
+
+	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+		struct __rpc_sockinfo si_other;
+		union {
+			struct sockaddr sa;
+			struct sockaddr_un un;
+			struct sockaddr_in in4;
+			struct sockaddr_in6 in6;
+			struct sockaddr_storage storage;
+		} sa;
+		socklen_t addrlen = sizeof(sa);
+
+		if (!__rpc_fd2sockinfo(fd, &si_other)) {
+			syslog(LOG_ERR, "cannot get information for fd %i", fd);
+			return 1;
 		}
 
-	       /*
-		* Bind to specific IPs if asked to
-		*/
-		checkbind = 0;
-		while (nhostsbak > 0) {
-			--nhostsbak;
-			/*
-			 * XXX - using RPC library internal functions.
-			 */
+		if (si.si_af != si_other.si_af ||
+                    si.si_socktype != si_other.si_socktype ||
+                    si.si_proto != si_other.si_proto)
+			continue;
+
+		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+			syslog(LOG_ERR, "failed to query socket name: %s",
+                               strerror(errno));
+			goto error;
+		}
+
+		/* Copy the address */
+		taddr.addr.maxlen = taddr.addr.len = addrlen;
+		taddr.addr.buf = malloc(addrlen);
+		if (taddr.addr.buf == NULL) {
+			syslog(LOG_ERR,
+                               "cannot allocate memory for %s address",
+                               nconf->nc_netid);
+			goto error;
+		}
+		memcpy(taddr.addr.buf, &sa, addrlen);
+
+		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+		if (my_xprt == (SVCXPRT *)NULL) {
+			syslog(LOG_ERR, "%s: could not create service",
+                               nconf->nc_netid);
+			goto error;
+		}
+	}
+
+	/* if none of the systemd sockets matched, we set up the socket in
+	 * the normal way:
+	 */
+#endif
+
+	if(my_xprt == (SVCXPRT *)NULL) {
+
+		/*
+		 * XXX - using RPC library internal functions. For NC_TPI_CLTS
+		 * we call this later, for each socket we like to bind.
+		 */
+		if (nconf->nc_semantics != NC_TPI_CLTS) {
 			if ((fd = __rpc_nconf2fd(nconf)) < 0) {
 				syslog(LOG_ERR, "cannot create socket for %s",
 				    nconf->nc_netid);
 				return (1);
 			}
-			switch (hints.ai_family) {
-			case AF_INET:
-				if (inet_pton(AF_INET, hosts[nhostsbak],
-				    host_addr) == 1) {
-					hints.ai_flags &= AI_NUMERICHOST;
-				} else {
-					/*
-					 * Skip if we have an AF_INET6 adress.
-					 */
-					if (inet_pton(AF_INET6,
-					    hosts[nhostsbak], host_addr) == 1)
-						continue;
+		}
+
+		if ((strcmp(nconf->nc_netid, "local") == 0) ||
+		    (strcmp(nconf->nc_netid, "unix") == 0)) {
+			memset(&sun, 0, sizeof sun);
+			sun.sun_family = AF_LOCAL;
+			unlink(_PATH_RPCBINDSOCK);
+			strcpy(sun.sun_path, _PATH_RPCBINDSOCK);
+			addrlen = SUN_LEN(&sun);
+			sa = (struct sockaddr *)&sun;
+		} else {
+			/* Get rpcbind's address on this transport */
+
+			memset(&hints, 0, sizeof hints);
+			hints.ai_flags = AI_PASSIVE;
+			hints.ai_family = si.si_af;
+			hints.ai_socktype = si.si_socktype;
+			hints.ai_protocol = si.si_proto;
+		}
+		if (nconf->nc_semantics == NC_TPI_CLTS) {
+			/*
+			 * If no hosts were specified, just bind to INADDR_ANY.  Otherwise
+			 * make sure 127.0.0.1 is added to the list.
+			 */
+			nhostsbak = nhosts;
+			nhostsbak++;
+			hosts = realloc(hosts, nhostsbak * sizeof(char *));
+			if (nhostsbak == 1)
+				hosts[0] = "*";
+			else {
+				if (hints.ai_family == AF_INET) {
+					hosts[nhostsbak - 1] = "127.0.0.1";
+				} else if (hints.ai_family == AF_INET6) {
+					hosts[nhostsbak - 1] = "::1";
+				} else
+					return 1;
+			}
+
+		       /*
+			* Bind to specific IPs if asked to
+			*/
+			checkbind = 0;
+			while (nhostsbak > 0) {
+				--nhostsbak;
+				/*
+				 * XXX - using RPC library internal functions.
+				 */
+				if ((fd = __rpc_nconf2fd(nconf)) < 0) {
+					syslog(LOG_ERR, "cannot create socket for %s",
+					    nconf->nc_netid);
+					return (1);
+				}
+				switch (hints.ai_family) {
+				case AF_INET:
+					if (inet_pton(AF_INET, hosts[nhostsbak],
+					    host_addr) == 1) {
+						hints.ai_flags &= AI_NUMERICHOST;
+					} else {
+						/*
+						 * Skip if we have an AF_INET6 adress.
+						 */
+						if (inet_pton(AF_INET6,
+						    hosts[nhostsbak], host_addr) == 1)
+							continue;
+					}
+					break;
+				case AF_INET6:
+					if (inet_pton(AF_INET6, hosts[nhostsbak],
+					    host_addr) == 1) {
+						hints.ai_flags &= AI_NUMERICHOST;
+					} else {
+						/*
+						 * Skip if we have an AF_INET adress.
+						 */
+						if (inet_pton(AF_INET, hosts[nhostsbak],
+						    host_addr) == 1)
+							continue;
+					}
+					break;
+				default:
+					break;
 				}
-				break;
-			case AF_INET6:
-				if (inet_pton(AF_INET6, hosts[nhostsbak],
-				    host_addr) == 1) {
-					hints.ai_flags &= AI_NUMERICHOST;
-				} else {
+
+				/*
+				 * If no hosts were specified, just bind to INADDR_ANY
+				 */
+				if (strcmp("*", hosts[nhostsbak]) == 0)
+					hosts[nhostsbak] = NULL;
+
+				if ((aicode = getaddrinfo(hosts[nhostsbak],
+				    servname, &hints, &res)) != 0) {
+				  if ((aicode = getaddrinfo(hosts[nhostsbak],
+							    "portmapper", &hints, &res)) != 0) {
+					syslog(LOG_ERR,
+					    "cannot get local address for %s: %s",
+					    nconf->nc_netid, gai_strerror(aicode));
+					continue;
+				  }
+				}
+				addrlen = res->ai_addrlen;
+				sa = (struct sockaddr *)res->ai_addr;
+				oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
+				if (bind(fd, sa, addrlen) != 0) {
+					syslog(LOG_ERR, "cannot bind %s on %s: %m",
+						(hosts[nhostsbak] == NULL) ? "*" :
+						hosts[nhostsbak], nconf->nc_netid);
+					if (res != NULL)
+						freeaddrinfo(res);
+					continue;
+				} else
+					checkbind++;
+				(void) umask(oldmask);
+
+				/* Copy the address */
+				taddr.addr.maxlen = taddr.addr.len = addrlen;
+				taddr.addr.buf = malloc(addrlen);
+				if (taddr.addr.buf == NULL) {
+					syslog(LOG_ERR,
+					    "cannot allocate memory for %s address",
+					    nconf->nc_netid);
+					if (res != NULL)
+						freeaddrinfo(res);
+					return 1;
+				}
+				memcpy(taddr.addr.buf, sa, addrlen);
+#ifdef RPCBIND_DEBUG
+				if (debugging) {
 					/*
-					 * Skip if we have an AF_INET adress.
+					 * for debugging print out our universal
+					 * address
 					 */
-					if (inet_pton(AF_INET, hosts[nhostsbak],
-					    host_addr) == 1)
-						continue;
+					char *uaddr;
+					struct netbuf nb;
+					int sa_size = 0;
+
+					nb.buf = sa;
+					switch( sa->sa_family){
+					case AF_INET:
+					  sa_size = sizeof (struct sockaddr_in);
+					  break;
+					case AF_INET6:
+					  sa_size = sizeof (struct sockaddr_in6);
+					  break;
+					}
+					nb.len = nb.maxlen = sa_size;
+					uaddr = taddr2uaddr(nconf, &nb);
+					(void) fprintf(stderr,
+					    "rpcbind : my address is %s\n", uaddr);
+					(void) free(uaddr);
+				}
+#endif
+				my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+				if (my_xprt == (SVCXPRT *)NULL) {
+					syslog(LOG_ERR, "%s: could not create service",
+                                               nconf->nc_netid);
+					goto error;
 				}
-	        		break;
-			default:
-				break;
 			}
-
-			/*
-			 * If no hosts were specified, just bind to INADDR_ANY
-			 */
-			if (strcmp("*", hosts[nhostsbak]) == 0)
-				hosts[nhostsbak] = NULL;
-
-			if ((aicode = getaddrinfo(hosts[nhostsbak],
-			    servname, &hints, &res)) != 0) {
-			  if ((aicode = getaddrinfo(hosts[nhostsbak],
-						    "portmapper", &hints, &res)) != 0) {
-				syslog(LOG_ERR,
-				    "cannot get local address for %s: %s",
-				    nconf->nc_netid, gai_strerror(aicode));
-				continue;
-			  }
+			if (!checkbind)
+				return 1;
+		} else {	/* NC_TPI_COTS */
+			if ((strcmp(nconf->nc_netid, "local") != 0) &&
+			    (strcmp(nconf->nc_netid, "unix") != 0)) {
+				if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
+				  if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
+				  printf("cannot get local address for %s: %s",  nconf->nc_netid, gai_strerror(aicode));
+				  syslog(LOG_ERR,
+					    "cannot get local address for %s: %s",
+					    nconf->nc_netid, gai_strerror(aicode));
+					return 1;
+				  }
+				}
+				addrlen = res->ai_addrlen;
+				sa = (struct sockaddr *)res->ai_addr;
 			}
-			addrlen = res->ai_addrlen;
-			sa = (struct sockaddr *)res->ai_addr;
 			oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
-                        if (bind(fd, sa, addrlen) != 0) {
-				syslog(LOG_ERR, "cannot bind %s on %s: %m",
-					(hosts[nhostsbak] == NULL) ? "*" :
-					hosts[nhostsbak], nconf->nc_netid);
+			__rpc_fd2sockinfo(fd, &si);
+			if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
+					sizeof(on)) != 0) {
+				syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
+					nconf->nc_netid);
 				if (res != NULL)
 					freeaddrinfo(res);
-				continue;
-			} else
-				checkbind++;
+				return 1;
+			}
+			if (bind(fd, sa, addrlen) < 0) {
+				syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
+				if (res != NULL)
+					freeaddrinfo(res);
+				return 1;
+			}
 			(void) umask(oldmask);
 
 			/* Copy the address */
-			taddr.addr.maxlen = taddr.addr.len = addrlen;
+			taddr.addr.len = taddr.addr.maxlen = addrlen;
 			taddr.addr.buf = malloc(addrlen);
 			if (taddr.addr.buf == NULL) {
-				syslog(LOG_ERR,
-				    "cannot allocate memory for %s address",
+				syslog(LOG_ERR, "cannot allocate memory for %s address",
 				    nconf->nc_netid);
 				if (res != NULL)
 					freeaddrinfo(res);
@@ -443,116 +591,37 @@ init_transport(struct netconfig *nconf)
 			memcpy(taddr.addr.buf, sa, addrlen);
 #ifdef RPCBIND_DEBUG
 			if (debugging) {
-				/*
-				 * for debugging print out our universal
-				 * address
-				 */
+				/* for debugging print out our universal address */
 				char *uaddr;
 				struct netbuf nb;
-				int sa_size = 0;
+			        int sa_size2 = 0;
 
 				nb.buf = sa;
 				switch( sa->sa_family){
 				case AF_INET:
-				  sa_size = sizeof (struct sockaddr_in);
+				  sa_size2 = sizeof (struct sockaddr_in);
 				  break;
 				case AF_INET6:
-				  sa_size = sizeof (struct sockaddr_in6);				 
+				  sa_size2 = sizeof (struct sockaddr_in6);
 				  break;
 				}
-				nb.len = nb.maxlen = sa_size;
+				nb.len = nb.maxlen = sa_size2;
 				uaddr = taddr2uaddr(nconf, &nb);
-				(void) fprintf(stderr,
-				    "rpcbind : my address is %s\n", uaddr);
+				(void) fprintf(stderr, "rpcbind : my address is %s\n",
+				    uaddr);
 				(void) free(uaddr);
 			}
 #endif
-			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, 
-                                RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+
+			listen(fd, SOMAXCONN);
+
+			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
 			if (my_xprt == (SVCXPRT *)NULL) {
-				syslog(LOG_ERR, "%s: could not create service", 
-                                        nconf->nc_netid);
+				syslog(LOG_ERR, "%s: could not create service",
+						nconf->nc_netid);
 				goto error;
 			}
 		}
-		if (!checkbind)
-			return 1;
-	} else {	/* NC_TPI_COTS */
-		if ((strcmp(nconf->nc_netid, "local") != 0) &&
-		    (strcmp(nconf->nc_netid, "unix") != 0)) {
-			if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
-			  if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
-			  printf("cannot get local address for %s: %s",  nconf->nc_netid, gai_strerror(aicode));
-			  syslog(LOG_ERR,
-				    "cannot get local address for %s: %s",
-				    nconf->nc_netid, gai_strerror(aicode));
-				return 1;
-			  }
-			}
-			addrlen = res->ai_addrlen;
-			sa = (struct sockaddr *)res->ai_addr;
-		}
-		oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
-		__rpc_fd2sockinfo(fd, &si);
-		if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
-				sizeof(on)) != 0) {
-			syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
-				nconf->nc_netid);
-			if (res != NULL)
-				freeaddrinfo(res);
-			return 1;
-		}
-		if (bind(fd, sa, addrlen) < 0) {
-			syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
-			if (res != NULL)
-				freeaddrinfo(res);
-			return 1;
-		}
-		(void) umask(oldmask);
-
-		/* Copy the address */
-		taddr.addr.len = taddr.addr.maxlen = addrlen;
-		taddr.addr.buf = malloc(addrlen);
-		if (taddr.addr.buf == NULL) {
-			syslog(LOG_ERR, "cannot allocate memory for %s address",
-			    nconf->nc_netid);
-			if (res != NULL)
-				freeaddrinfo(res);
-			return 1;
-		}
-		memcpy(taddr.addr.buf, sa, addrlen);
-#ifdef RPCBIND_DEBUG
-		if (debugging) {
-			/* for debugging print out our universal address */
-			char *uaddr;
-			struct netbuf nb;
-		        int sa_size2 = 0;
-
-			nb.buf = sa;
-			switch( sa->sa_family){
-			case AF_INET:
-			  sa_size2 = sizeof (struct sockaddr_in);
-			  break;
-			case AF_INET6:
-			  sa_size2 = sizeof (struct sockaddr_in6);				 
-			  break;
-			}
-			nb.len = nb.maxlen = sa_size2;
-			uaddr = taddr2uaddr(nconf, &nb);
-			(void) fprintf(stderr, "rpcbind : my address is %s\n",
-			    uaddr);
-			(void) free(uaddr);
-		}
-#endif
-
-		listen(fd, SOMAXCONN);
-
-		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
-		if (my_xprt == (SVCXPRT *)NULL) {
-			syslog(LOG_ERR, "%s: could not create service",
-					nconf->nc_netid);
-			goto error;
-		}
 	}
 
 #ifdef PORTMAP
diff --git a/systemd/.gitignore b/systemd/.gitignore
new file mode 100644
index 0000000..b7b4561
--- /dev/null
+++ b/systemd/.gitignore
@@ -0,0 +1 @@
+rpcbind.service
diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
new file mode 100644
index 0000000..58ae5de
--- /dev/null
+++ b/systemd/rpcbind.service.in
@@ -0,0 +1,9 @@
+[Unit]
+Description=RPC Bind
+
+[Service]
+ExecStart=@bindir@/rpcbind -w -f
+
+[Install]
+WantedBy=multi-user.target
+Also=rpcbind.socket
diff --git a/systemd/rpcbind.socket b/systemd/rpcbind.socket
new file mode 100644
index 0000000..ad5fd62
--- /dev/null
+++ b/systemd/rpcbind.socket
@@ -0,0 +1,12 @@
+[Unit]
+Description=RPCbind Server Activation Socket
+Wants=rpcbind.target
+Before=rpcbind.target
+
+[Socket]
+ListenStream=/var/run/rpcbind.sock
+ListenStream=111
+ListenDatagram=111
+
+[Install]
+WantedBy=sockets.target
-- 
1.7.9

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

Adding libtirpc developers mailing list...

On Feb 1, 2012, at 6:47 AM, Tom Gundersen wrote:

> Making rpcbind sockect activated will greatly simplify
> its integration in systemd systems. In essence, other services
> may now assume that rpcbind is always available, even during very
> early boot. This means that we no longer need to worry about any
> ordering dependencies.
> 
> This is based on a patch originally posted by Lennart Poettering:
> <http://permalink.gmane.org/gmane.linux.nfs/33774>.
> 
> That patch was not merged due to the lack of a shared library and
> as systemd was seen to be too Fedora specific.
> 
> Systemd now provides a shared library, and it is shipped by defalt in
> OpenSUSE in addition to Fedora, and it is available in Debain, Gentoo,
> Arch, and others.
> 
> This version of the patch has three changes from the original:
> 
> * It uses the shared library.
> * It comes with unit files.
> * It is rebased on top of master.
> 
> Please review the patch with "git show -b" or otherwise ignoring the
> whitespace changes, or it will be extremely difficult to read.
> 
> Comments welcome.
> 
> v2: correctly enable systemd code at compile time
>    handle the case where not all the required sockets were supplied
>    listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
>    do not daemonize
> 
> Original-patch-by: Lennart Poettering <lennart@poettering.net>
> Cc: Michal Schmidt <mschmidt@redhat.com>
> Cc: Steve Dickson <steved@redhat.com>
> Cc: systemd-devel@lists.freedesktop.org
> Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
> Signed-off-by: Tom Gundersen <teg@jklm.no>
> ---
> 
> What's the status on this?
> 
> Lennart, does your ack still appl after my changes?
> 
> Steve, any chance of applying this?
> 
> If this is applied I have a couple of follow ups. In particular, I'd like to
> do the cleanup of init_transport that Jim suggested as a separate patch,
> as leaving the line-wraps alone makes this patch easier to read I think.

Typically this is done by breaking the proposed change into smaller atomic patches.  Is that not possible in this case?

I can play with this more later today or tomorrow.

> Added Michal to cc as this patch should go a long way to sort out
> <https://bugzilla.redhat.com/show_bug.cgi?id=747363>.

Wouldn't comment 3 also work around problems long enough to give us an opportunity to adequately vet the proposed changes?

What additions to our test matrix are needed?

One more immediate comment below.

> Cheers,
> 
> Tom
> 
> Makefile.am                |   15 ++
> configure.in               |   11 +
> src/rpcbind.c              |  467 +++++++++++++++++++++++++-------------------
> systemd/.gitignore         |    1 +
> systemd/rpcbind.service.in |    9 +
> systemd/rpcbind.socket     |   12 ++
> 6 files changed, 316 insertions(+), 199 deletions(-)
> create mode 100644 systemd/.gitignore
> create mode 100644 systemd/rpcbind.service.in
> create mode 100644 systemd/rpcbind.socket
> 
> diff --git a/Makefile.am b/Makefile.am
> index 9fa608e..194b467 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -38,6 +38,21 @@ rpcbind_SOURCES = \
> 	src/warmstart.c
> rpcbind_LDADD = $(TIRPC_LIBS)
> 
> +if SYSTEMD
> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
> +
> +rpcbind_LDADD += $(SYSTEMD_LIBS)
> +
> +systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
> +	sed -e 's,@bindir\@,$(bindir),g' \
> +		< $< > $@ || rm $@
> +
> +systemdsystemunit_DATA = \
> +	systemd/rpcbind.service \
> +	systemd/rpcbind.socket
> +
> +endif
> +
> rpcinfo_SOURCES =       src/rpcinfo.c
> rpcinfo_LDADD   =       $(TIRPC_LIBS)
> 
> diff --git a/configure.in b/configure.in
> index 2b67720..397d52d 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])
> 
> PKG_CHECK_MODULES([TIRPC], [libtirpc])
> 
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemdsystemunitdir],
> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
> +  if test "x$with_systemdsystemunitdir" != xno; then
> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
> +    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> +  fi
> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
> +
> +
> AS_IF([test x$enable_libwrap = xyes], [
> 	AC_CHECK_LIB([wrap], [hosts_access], ,
> 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index 24e069b..a87ce05 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -56,6 +56,9 @@
> #include <netinet/in.h>
> #endif
> #include <arpa/inet.h>
> +#ifdef SYSTEMD
> +#include <systemd/sd-daemon.h>
> +#endif
> #include <fcntl.h>
> #include <netdb.h>
> #include <stdio.h>
> @@ -281,6 +284,7 @@ init_transport(struct netconfig *nconf)
> 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
> 	struct sockaddr_un sun;
> 	mode_t oldmask;
> +	int n = 0;
>         res = NULL;
> 
> 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
> @@ -300,141 +304,285 @@ init_transport(struct netconfig *nconf)
> 	}
> #endif
> 
> -	/*
> -	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
> -	 * we call this later, for each socket we like to bind.
> -	 */
> -	if (nconf->nc_semantics != NC_TPI_CLTS) {
> -		if ((fd = __rpc_nconf2fd(nconf)) < 0) {
> -			syslog(LOG_ERR, "cannot create socket for %s",
> -			    nconf->nc_netid);
> -			return (1);
> -		}
> -	}
> -
> 	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> 		syslog(LOG_ERR, "cannot get information for %s",
> 		    nconf->nc_netid);
> 		return (1);
> 	}
> 
> -	if ((strcmp(nconf->nc_netid, "local") == 0) ||
> -	    (strcmp(nconf->nc_netid, "unix") == 0)) {
> -		memset(&sun, 0, sizeof sun);
> -		sun.sun_family = AF_LOCAL;
> -		unlink(_PATH_RPCBINDSOCK);
> -		strcpy(sun.sun_path, _PATH_RPCBINDSOCK);
> -		addrlen = SUN_LEN(&sun);
> -		sa = (struct sockaddr *)&sun;
> -	} else {
> -		/* Get rpcbind's address on this transport */
> -
> -		memset(&hints, 0, sizeof hints);
> -		hints.ai_flags = AI_PASSIVE;
> -		hints.ai_family = si.si_af;
> -		hints.ai_socktype = si.si_socktype;
> -		hints.ai_protocol = si.si_proto;
> +#ifdef SYSTEMD
> +	n = sd_listen_fds(0);
> +	if (n < 0) {
> +		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
> +		return 1;
> 	}
> -	if (nconf->nc_semantics == NC_TPI_CLTS) {
> -		/*
> -		 * If no hosts were specified, just bind to INADDR_ANY.  Otherwise
> -		 * make sure 127.0.0.1 is added to the list.
> -		 */
> -		nhostsbak = nhosts;
> -		nhostsbak++;
> -		hosts = realloc(hosts, nhostsbak * sizeof(char *));
> -		if (nhostsbak == 1)
> -			hosts[0] = "*";
> -		else {
> -			if (hints.ai_family == AF_INET) {
> -				hosts[nhostsbak - 1] = "127.0.0.1";
> -			} else if (hints.ai_family == AF_INET6) {
> -				hosts[nhostsbak - 1] = "::1";
> -			} else
> -				return 1;
> +
> +	/* Try to find if one of the systemd sockets we were given match
> +	 * our netconfig structure. */
> +
> +	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
> +		struct __rpc_sockinfo si_other;
> +		union {
> +			struct sockaddr sa;
> +			struct sockaddr_un un;
> +			struct sockaddr_in in4;
> +			struct sockaddr_in6 in6;
> +			struct sockaddr_storage storage;
> +		} sa;

Why is sockaddr_storage included in this union?

> +		socklen_t addrlen = sizeof(sa);
> +
> +		if (!__rpc_fd2sockinfo(fd, &si_other)) {
> +			syslog(LOG_ERR, "cannot get information for fd %i", fd);
> +			return 1;
> 		}
> 
> -	       /*
> -		* Bind to specific IPs if asked to
> -		*/
> -		checkbind = 0;
> -		while (nhostsbak > 0) {
> -			--nhostsbak;
> -			/*
> -			 * XXX - using RPC library internal functions.
> -			 */
> +		if (si.si_af != si_other.si_af ||
> +                    si.si_socktype != si_other.si_socktype ||
> +                    si.si_proto != si_other.si_proto)
> +			continue;
> +
> +		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
> +			syslog(LOG_ERR, "failed to query socket name: %s",
> +                               strerror(errno));
> +			goto error;
> +		}
> +
> +		/* Copy the address */
> +		taddr.addr.maxlen = taddr.addr.len = addrlen;
> +		taddr.addr.buf = malloc(addrlen);
> +		if (taddr.addr.buf == NULL) {
> +			syslog(LOG_ERR,
> +                               "cannot allocate memory for %s address",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +		memcpy(taddr.addr.buf, &sa, addrlen);
> +
> +		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> +                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +		if (my_xprt == (SVCXPRT *)NULL) {
> +			syslog(LOG_ERR, "%s: could not create service",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +	}
> +
> +	/* if none of the systemd sockets matched, we set up the socket in
> +	 * the normal way:
> +	 */
> +#endif
> +
> +	if(my_xprt == (SVCXPRT *)NULL) {
> +
> +		/*
> +		 * XXX - using RPC library internal functions. For NC_TPI_CLTS
> +		 * we call this later, for each socket we like to bind.
> +		 */
> +		if (nconf->nc_semantics != NC_TPI_CLTS) {
> 			if ((fd = __rpc_nconf2fd(nconf)) < 0) {
> 				syslog(LOG_ERR, "cannot create socket for %s",
> 				    nconf->nc_netid);
> 				return (1);
> 			}
> -			switch (hints.ai_family) {
> -			case AF_INET:
> -				if (inet_pton(AF_INET, hosts[nhostsbak],
> -				    host_addr) == 1) {
> -					hints.ai_flags &= AI_NUMERICHOST;
> -				} else {
> -					/*
> -					 * Skip if we have an AF_INET6 adress.
> -					 */
> -					if (inet_pton(AF_INET6,
> -					    hosts[nhostsbak], host_addr) == 1)
> -						continue;
> +		}
> +
> +		if ((strcmp(nconf->nc_netid, "local") == 0) ||
> +		    (strcmp(nconf->nc_netid, "unix") == 0)) {
> +			memset(&sun, 0, sizeof sun);
> +			sun.sun_family = AF_LOCAL;
> +			unlink(_PATH_RPCBINDSOCK);
> +			strcpy(sun.sun_path, _PATH_RPCBINDSOCK);
> +			addrlen = SUN_LEN(&sun);
> +			sa = (struct sockaddr *)&sun;
> +		} else {
> +			/* Get rpcbind's address on this transport */
> +
> +			memset(&hints, 0, sizeof hints);
> +			hints.ai_flags = AI_PASSIVE;
> +			hints.ai_family = si.si_af;
> +			hints.ai_socktype = si.si_socktype;
> +			hints.ai_protocol = si.si_proto;
> +		}
> +		if (nconf->nc_semantics == NC_TPI_CLTS) {
> +			/*
> +			 * If no hosts were specified, just bind to INADDR_ANY.  Otherwise
> +			 * make sure 127.0.0.1 is added to the list.
> +			 */
> +			nhostsbak = nhosts;
> +			nhostsbak++;
> +			hosts = realloc(hosts, nhostsbak * sizeof(char *));
> +			if (nhostsbak == 1)
> +				hosts[0] = "*";
> +			else {
> +				if (hints.ai_family == AF_INET) {
> +					hosts[nhostsbak - 1] = "127.0.0.1";
> +				} else if (hints.ai_family == AF_INET6) {
> +					hosts[nhostsbak - 1] = "::1";
> +				} else
> +					return 1;
> +			}
> +
> +		       /*
> +			* Bind to specific IPs if asked to
> +			*/
> +			checkbind = 0;
> +			while (nhostsbak > 0) {
> +				--nhostsbak;
> +				/*
> +				 * XXX - using RPC library internal functions.
> +				 */
> +				if ((fd = __rpc_nconf2fd(nconf)) < 0) {
> +					syslog(LOG_ERR, "cannot create socket for %s",
> +					    nconf->nc_netid);
> +					return (1);
> +				}
> +				switch (hints.ai_family) {
> +				case AF_INET:
> +					if (inet_pton(AF_INET, hosts[nhostsbak],
> +					    host_addr) == 1) {
> +						hints.ai_flags &= AI_NUMERICHOST;
> +					} else {
> +						/*
> +						 * Skip if we have an AF_INET6 adress.
> +						 */
> +						if (inet_pton(AF_INET6,
> +						    hosts[nhostsbak], host_addr) == 1)
> +							continue;
> +					}
> +					break;
> +				case AF_INET6:
> +					if (inet_pton(AF_INET6, hosts[nhostsbak],
> +					    host_addr) == 1) {
> +						hints.ai_flags &= AI_NUMERICHOST;
> +					} else {
> +						/*
> +						 * Skip if we have an AF_INET adress.
> +						 */
> +						if (inet_pton(AF_INET, hosts[nhostsbak],
> +						    host_addr) == 1)
> +							continue;
> +					}
> +					break;
> +				default:
> +					break;
> 				}
> -				break;
> -			case AF_INET6:
> -				if (inet_pton(AF_INET6, hosts[nhostsbak],
> -				    host_addr) == 1) {
> -					hints.ai_flags &= AI_NUMERICHOST;
> -				} else {
> +
> +				/*
> +				 * If no hosts were specified, just bind to INADDR_ANY
> +				 */
> +				if (strcmp("*", hosts[nhostsbak]) == 0)
> +					hosts[nhostsbak] = NULL;
> +
> +				if ((aicode = getaddrinfo(hosts[nhostsbak],
> +				    servname, &hints, &res)) != 0) {
> +				  if ((aicode = getaddrinfo(hosts[nhostsbak],
> +							    "portmapper", &hints, &res)) != 0) {
> +					syslog(LOG_ERR,
> +					    "cannot get local address for %s: %s",
> +					    nconf->nc_netid, gai_strerror(aicode));
> +					continue;
> +				  }
> +				}
> +				addrlen = res->ai_addrlen;
> +				sa = (struct sockaddr *)res->ai_addr;
> +				oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
> +				if (bind(fd, sa, addrlen) != 0) {
> +					syslog(LOG_ERR, "cannot bind %s on %s: %m",
> +						(hosts[nhostsbak] == NULL) ? "*" :
> +						hosts[nhostsbak], nconf->nc_netid);
> +					if (res != NULL)
> +						freeaddrinfo(res);
> +					continue;
> +				} else
> +					checkbind++;
> +				(void) umask(oldmask);
> +
> +				/* Copy the address */
> +				taddr.addr.maxlen = taddr.addr.len = addrlen;
> +				taddr.addr.buf = malloc(addrlen);
> +				if (taddr.addr.buf == NULL) {
> +					syslog(LOG_ERR,
> +					    "cannot allocate memory for %s address",
> +					    nconf->nc_netid);
> +					if (res != NULL)
> +						freeaddrinfo(res);
> +					return 1;
> +				}
> +				memcpy(taddr.addr.buf, sa, addrlen);
> +#ifdef RPCBIND_DEBUG
> +				if (debugging) {
> 					/*
> -					 * Skip if we have an AF_INET adress.
> +					 * for debugging print out our universal
> +					 * address
> 					 */
> -					if (inet_pton(AF_INET, hosts[nhostsbak],
> -					    host_addr) == 1)
> -						continue;
> +					char *uaddr;
> +					struct netbuf nb;
> +					int sa_size = 0;
> +
> +					nb.buf = sa;
> +					switch( sa->sa_family){
> +					case AF_INET:
> +					  sa_size = sizeof (struct sockaddr_in);
> +					  break;
> +					case AF_INET6:
> +					  sa_size = sizeof (struct sockaddr_in6);
> +					  break;
> +					}
> +					nb.len = nb.maxlen = sa_size;
> +					uaddr = taddr2uaddr(nconf, &nb);
> +					(void) fprintf(stderr,
> +					    "rpcbind : my address is %s\n", uaddr);
> +					(void) free(uaddr);
> +				}
> +#endif
> +				my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> +                                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +				if (my_xprt == (SVCXPRT *)NULL) {
> +					syslog(LOG_ERR, "%s: could not create service",
> +                                               nconf->nc_netid);
> +					goto error;
> 				}
> -	        		break;
> -			default:
> -				break;
> 			}
> -
> -			/*
> -			 * If no hosts were specified, just bind to INADDR_ANY
> -			 */
> -			if (strcmp("*", hosts[nhostsbak]) == 0)
> -				hosts[nhostsbak] = NULL;
> -
> -			if ((aicode = getaddrinfo(hosts[nhostsbak],
> -			    servname, &hints, &res)) != 0) {
> -			  if ((aicode = getaddrinfo(hosts[nhostsbak],
> -						    "portmapper", &hints, &res)) != 0) {
> -				syslog(LOG_ERR,
> -				    "cannot get local address for %s: %s",
> -				    nconf->nc_netid, gai_strerror(aicode));
> -				continue;
> -			  }
> +			if (!checkbind)
> +				return 1;
> +		} else {	/* NC_TPI_COTS */
> +			if ((strcmp(nconf->nc_netid, "local") != 0) &&
> +			    (strcmp(nconf->nc_netid, "unix") != 0)) {
> +				if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
> +				  if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
> +				  printf("cannot get local address for %s: %s",  nconf->nc_netid, gai_strerror(aicode));
> +				  syslog(LOG_ERR,
> +					    "cannot get local address for %s: %s",
> +					    nconf->nc_netid, gai_strerror(aicode));
> +					return 1;
> +				  }
> +				}
> +				addrlen = res->ai_addrlen;
> +				sa = (struct sockaddr *)res->ai_addr;
> 			}
> -			addrlen = res->ai_addrlen;
> -			sa = (struct sockaddr *)res->ai_addr;
> 			oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
> -                        if (bind(fd, sa, addrlen) != 0) {
> -				syslog(LOG_ERR, "cannot bind %s on %s: %m",
> -					(hosts[nhostsbak] == NULL) ? "*" :
> -					hosts[nhostsbak], nconf->nc_netid);
> +			__rpc_fd2sockinfo(fd, &si);
> +			if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
> +					sizeof(on)) != 0) {
> +				syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
> +					nconf->nc_netid);
> 				if (res != NULL)
> 					freeaddrinfo(res);
> -				continue;
> -			} else
> -				checkbind++;
> +				return 1;
> +			}
> +			if (bind(fd, sa, addrlen) < 0) {
> +				syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
> +				if (res != NULL)
> +					freeaddrinfo(res);
> +				return 1;
> +			}
> 			(void) umask(oldmask);
> 
> 			/* Copy the address */
> -			taddr.addr.maxlen = taddr.addr.len = addrlen;
> +			taddr.addr.len = taddr.addr.maxlen = addrlen;
> 			taddr.addr.buf = malloc(addrlen);
> 			if (taddr.addr.buf == NULL) {
> -				syslog(LOG_ERR,
> -				    "cannot allocate memory for %s address",
> +				syslog(LOG_ERR, "cannot allocate memory for %s address",
> 				    nconf->nc_netid);
> 				if (res != NULL)
> 					freeaddrinfo(res);
> @@ -443,116 +591,37 @@ init_transport(struct netconfig *nconf)
> 			memcpy(taddr.addr.buf, sa, addrlen);
> #ifdef RPCBIND_DEBUG
> 			if (debugging) {
> -				/*
> -				 * for debugging print out our universal
> -				 * address
> -				 */
> +				/* for debugging print out our universal address */
> 				char *uaddr;
> 				struct netbuf nb;
> -				int sa_size = 0;
> +			        int sa_size2 = 0;
> 
> 				nb.buf = sa;
> 				switch( sa->sa_family){
> 				case AF_INET:
> -				  sa_size = sizeof (struct sockaddr_in);
> +				  sa_size2 = sizeof (struct sockaddr_in);
> 				  break;
> 				case AF_INET6:
> -				  sa_size = sizeof (struct sockaddr_in6);				 
> +				  sa_size2 = sizeof (struct sockaddr_in6);
> 				  break;
> 				}
> -				nb.len = nb.maxlen = sa_size;
> +				nb.len = nb.maxlen = sa_size2;
> 				uaddr = taddr2uaddr(nconf, &nb);
> -				(void) fprintf(stderr,
> -				    "rpcbind : my address is %s\n", uaddr);
> +				(void) fprintf(stderr, "rpcbind : my address is %s\n",
> +				    uaddr);
> 				(void) free(uaddr);
> 			}
> #endif
> -			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, 
> -                                RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +
> +			listen(fd, SOMAXCONN);
> +
> +			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> 			if (my_xprt == (SVCXPRT *)NULL) {
> -				syslog(LOG_ERR, "%s: could not create service", 
> -                                        nconf->nc_netid);
> +				syslog(LOG_ERR, "%s: could not create service",
> +						nconf->nc_netid);
> 				goto error;
> 			}
> 		}
> -		if (!checkbind)
> -			return 1;
> -	} else {	/* NC_TPI_COTS */
> -		if ((strcmp(nconf->nc_netid, "local") != 0) &&
> -		    (strcmp(nconf->nc_netid, "unix") != 0)) {
> -			if ((aicode = getaddrinfo(NULL, servname, &hints, &res))!= 0) {
> -			  if ((aicode = getaddrinfo(NULL, "portmapper", &hints, &res))!= 0) {
> -			  printf("cannot get local address for %s: %s",  nconf->nc_netid, gai_strerror(aicode));
> -			  syslog(LOG_ERR,
> -				    "cannot get local address for %s: %s",
> -				    nconf->nc_netid, gai_strerror(aicode));
> -				return 1;
> -			  }
> -			}
> -			addrlen = res->ai_addrlen;
> -			sa = (struct sockaddr *)res->ai_addr;
> -		}
> -		oldmask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
> -		__rpc_fd2sockinfo(fd, &si);
> -		if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on,
> -				sizeof(on)) != 0) {
> -			syslog(LOG_ERR, "cannot set SO_REUSEADDR on %s",
> -				nconf->nc_netid);
> -			if (res != NULL)
> -				freeaddrinfo(res);
> -			return 1;
> -		}
> -		if (bind(fd, sa, addrlen) < 0) {
> -			syslog(LOG_ERR, "cannot bind %s: %m", nconf->nc_netid);
> -			if (res != NULL)
> -				freeaddrinfo(res);
> -			return 1;
> -		}
> -		(void) umask(oldmask);
> -
> -		/* Copy the address */
> -		taddr.addr.len = taddr.addr.maxlen = addrlen;
> -		taddr.addr.buf = malloc(addrlen);
> -		if (taddr.addr.buf == NULL) {
> -			syslog(LOG_ERR, "cannot allocate memory for %s address",
> -			    nconf->nc_netid);
> -			if (res != NULL)
> -				freeaddrinfo(res);
> -			return 1;
> -		}
> -		memcpy(taddr.addr.buf, sa, addrlen);
> -#ifdef RPCBIND_DEBUG
> -		if (debugging) {
> -			/* for debugging print out our universal address */
> -			char *uaddr;
> -			struct netbuf nb;
> -		        int sa_size2 = 0;
> -
> -			nb.buf = sa;
> -			switch( sa->sa_family){
> -			case AF_INET:
> -			  sa_size2 = sizeof (struct sockaddr_in);
> -			  break;
> -			case AF_INET6:
> -			  sa_size2 = sizeof (struct sockaddr_in6);				 
> -			  break;
> -			}
> -			nb.len = nb.maxlen = sa_size2;
> -			uaddr = taddr2uaddr(nconf, &nb);
> -			(void) fprintf(stderr, "rpcbind : my address is %s\n",
> -			    uaddr);
> -			(void) free(uaddr);
> -		}
> -#endif
> -
> -		listen(fd, SOMAXCONN);
> -
> -		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr, RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> -		if (my_xprt == (SVCXPRT *)NULL) {
> -			syslog(LOG_ERR, "%s: could not create service",
> -					nconf->nc_netid);
> -			goto error;
> -		}
> 	}
> 
> #ifdef PORTMAP
> diff --git a/systemd/.gitignore b/systemd/.gitignore
> new file mode 100644
> index 0000000..b7b4561
> --- /dev/null
> +++ b/systemd/.gitignore
> @@ -0,0 +1 @@
> +rpcbind.service
> diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
> new file mode 100644
> index 0000000..58ae5de
> --- /dev/null
> +++ b/systemd/rpcbind.service.in
> @@ -0,0 +1,9 @@
> +[Unit]
> +Description=RPC Bind
> +
> +[Service]
> +ExecStart=@bindir@/rpcbind -w -f
> +
> +[Install]
> +WantedBy=multi-user.target
> +Also=rpcbind.socket
> diff --git a/systemd/rpcbind.socket b/systemd/rpcbind.socket
> new file mode 100644
> index 0000000..ad5fd62
> --- /dev/null
> +++ b/systemd/rpcbind.socket
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=RPCbind Server Activation Socket
> +Wants=rpcbind.target
> +Before=rpcbind.target
> +
> +[Socket]
> +ListenStream=/var/run/rpcbind.sock
> +ListenStream=111
> +ListenDatagram=111
> +
> +[Install]
> +WantedBy=sockets.target
> -- 
> 1.7.9
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH] rpcbind: add support for systemd socket activation

[Tom Gundersen]

Hi Chuck,

Thanks for the feedback.

On Wed, Feb 1, 2012 at 4:32 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> Typically this is done by breaking the proposed change into smaller atomic patches.  Is that not possible in this case?

Not entirely sure what you have in mind, I don't immediately see how
to split this up. Any suggestions welcome.

>> Added Michal to cc as this patch should go a long way to sort out
>> <https://bugzilla.redhat.com/show_bug.cgi?id=747363>.
>
> Wouldn't comment 3 also work around problems long enough to give us an opportunity to adequately vet the proposed changes?

Sure, that's another option.

> What additions to our test matrix are needed?

I could not find any tests in the rpcbind git repo. Could you point me
at your test matrix?

>> +     /* Try to find if one of the systemd sockets we were given match
>> +      * our netconfig structure. */
>> +
>> +     for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
>> +             struct __rpc_sockinfo si_other;
>> +             union {
>> +                     struct sockaddr sa;
>> +                     struct sockaddr_un un;
>> +                     struct sockaddr_in in4;
>> +                     struct sockaddr_in6 in6;
>> +                     struct sockaddr_storage storage;
>> +             } sa;
>
> Why is sockaddr_storage included in this union?

This is from Lennart's original patch. Lennart, care to comment?

For what it's worth, here is the patch with whitespace ignored, which
should make it simpler to review:


diff --git a/Makefile.am b/Makefile.am
index 9fa608e..194b467 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -38,6 +38,21 @@ rpcbind_SOURCES = \
 	src/warmstart.c
 rpcbind_LDADD = $(TIRPC_LIBS)

+if SYSTEMD
+AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
+
+rpcbind_LDADD += $(SYSTEMD_LIBS)
+
+systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
+	sed -e 's,@bindir\@,$(bindir),g' \
+		< $< > $@ || rm $@
+
+systemdsystemunit_DATA = \
+	systemd/rpcbind.service \
+	systemd/rpcbind.socket
+
+endif
+
 rpcinfo_SOURCES =       src/rpcinfo.c
 rpcinfo_LDADD   =       $(TIRPC_LIBS)

diff --git a/configure.in b/configure.in
index 2b67720..397d52d 100644
--- a/configure.in
+++ b/configure.in
@@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])

 PKG_CHECK_MODULES([TIRPC], [libtirpc])

+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemdsystemunitdir],
+  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for
systemd service files]),
+  [], [with_systemdsystemunitdir=$($PKG_CONFIG
--variable=systemdsystemunitdir systemd)])
+  if test "x$with_systemdsystemunitdir" != xno; then
+    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
+  fi
+AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a
"x$with_systemdsystemunitdir" != xno ])
+
+
 AS_IF([test x$enable_libwrap = xyes], [
 	AC_CHECK_LIB([wrap], [hosts_access], ,
 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
diff --git a/src/rpcbind.c b/src/rpcbind.c
index 24e069b..a87ce05 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -56,6 +56,9 @@
 #include <netinet/in.h>
 #endif
 #include <arpa/inet.h>
+#ifdef SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
 #include <fcntl.h>
 #include <netdb.h>
 #include <stdio.h>
@@ -281,6 +284,7 @@ init_transport(struct netconfig *nconf)
 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
 	struct sockaddr_un sun;
 	mode_t oldmask;
+	int n = 0;
         res = NULL;

 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -300,6 +304,76 @@ init_transport(struct netconfig *nconf)
 	}
 #endif

+	if (!__rpc_nconf2sockinfo(nconf, &si)) {
+		syslog(LOG_ERR, "cannot get information for %s",
+		    nconf->nc_netid);
+		return (1);
+	}
+
+#ifdef SYSTEMD
+	n = sd_listen_fds(0);
+	if (n < 0) {
+		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
+		return 1;
+	}
+
+	/* Try to find if one of the systemd sockets we were given match
+	 * our netconfig structure. */
+
+	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+		struct __rpc_sockinfo si_other;
+		union {
+			struct sockaddr sa;
+			struct sockaddr_un un;
+			struct sockaddr_in in4;
+			struct sockaddr_in6 in6;
+			struct sockaddr_storage storage;
+		} sa;
+		socklen_t addrlen = sizeof(sa);
+
+		if (!__rpc_fd2sockinfo(fd, &si_other)) {
+			syslog(LOG_ERR, "cannot get information for fd %i", fd);
+			return 1;
+		}
+
+		if (si.si_af != si_other.si_af ||
+                    si.si_socktype != si_other.si_socktype ||
+                    si.si_proto != si_other.si_proto)
+			continue;
+
+		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+			syslog(LOG_ERR, "failed to query socket name: %s",
+                               strerror(errno));
+			goto error;
+		}
+
+		/* Copy the address */
+		taddr.addr.maxlen = taddr.addr.len = addrlen;
+		taddr.addr.buf = malloc(addrlen);
+		if (taddr.addr.buf == NULL) {
+			syslog(LOG_ERR,
+                               "cannot allocate memory for %s address",
+                               nconf->nc_netid);
+			goto error;
+		}
+		memcpy(taddr.addr.buf, &sa, addrlen);
+
+		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+		if (my_xprt == (SVCXPRT *)NULL) {
+			syslog(LOG_ERR, "%s: could not create service",
+                               nconf->nc_netid);
+			goto error;
+		}
+	}
+
+	/* if none of the systemd sockets matched, we set up the socket in
+	 * the normal way:
+	 */
+#endif
+
+	if(my_xprt == (SVCXPRT *)NULL) {
+
 	/*
 	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
 	 * we call this later, for each socket we like to bind.
@@ -312,12 +386,6 @@ init_transport(struct netconfig *nconf)
 		}
 	}

-	if (!__rpc_nconf2sockinfo(nconf, &si)) {
-		syslog(LOG_ERR, "cannot get information for %s",
-		    nconf->nc_netid);
-		return (1);
-	}
-
 	if ((strcmp(nconf->nc_netid, "local") == 0) ||
 	    (strcmp(nconf->nc_netid, "unix") == 0)) {
 		memset(&sun, 0, sizeof sun);
@@ -554,6 +622,7 @@ init_transport(struct netconfig *nconf)
 			goto error;
 		}
 	}
+	}

 #ifdef PORTMAP
 	/*
diff --git a/systemd/.gitignore b/systemd/.gitignore
new file mode 100644
index 0000000..b7b4561
--- /dev/null
+++ b/systemd/.gitignore
@@ -0,0 +1 @@
+rpcbind.service
diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
new file mode 100644
index 0000000..58ae5de
--- /dev/null
+++ b/systemd/rpcbind.service.in
@@ -0,0 +1,9 @@
+[Unit]
+Description=RPC Bind
+
+[Service]
+ExecStart=@bindir@/rpcbind -w -f
+
+[Install]
+WantedBy=multi-user.target
+Also=rpcbind.socket
diff --git a/systemd/rpcbind.socket b/systemd/rpcbind.socket
new file mode 100644
index 0000000..ad5fd62
--- /dev/null
+++ b/systemd/rpcbind.socket
@@ -0,0 +1,12 @@
+[Unit]
+Description=RPCbind Server Activation Socket
+Wants=rpcbind.target
+Before=rpcbind.target
+
+[Socket]
+ListenStream=/var/run/rpcbind.sock
+ListenStream=111
+ListenDatagram=111
+
+[Install]
+WantedBy=sockets.target

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

On Feb 1, 2012, at 11:37 AM, Tom Gundersen wrote:

> Hi Chuck,
> 
> Thanks for the feedback.
> 
> On Wed, Feb 1, 2012 at 4:32 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>> Typically this is done by breaking the proposed change into smaller atomic patches.  Is that not possible in this case?
> 
> Not entirely sure what you have in mind, I don't immediately see how
> to split this up. Any suggestions welcome.

Submit the patch you have below, then white space fixes become subsequent clean up patches.  Not only is review easier now, but if we come back to these changes in a year to fix bugs, it's easier for us to re-learn what it does.

> 
>>> Added Michal to cc as this patch should go a long way to sort out
>>> <https://bugzilla.redhat.com/show_bug.cgi?id=747363>.
>> 
>> Wouldn't comment 3 also work around problems long enough to give us an opportunity to adequately vet the proposed changes?
> 
> Sure, that's another option.
> 
>> What additions to our test matrix are needed?
> 
> I could not find any tests in the rpcbind git repo. Could you point me
> at your test matrix?

I'll state the question differently: When the SYSTEMD macro is defined, what kind of tests should we run? In general, how do we confirm this is working as expected?  (There may be no good answer at the moment).

> 
>>> +     /* Try to find if one of the systemd sockets we were given match
>>> +      * our netconfig structure. */
>>> +
>>> +     for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
>>> +             struct __rpc_sockinfo si_other;
>>> +             union {
>>> +                     struct sockaddr sa;
>>> +                     struct sockaddr_un un;
>>> +                     struct sockaddr_in in4;
>>> +                     struct sockaddr_in6 in6;
>>> +                     struct sockaddr_storage storage;
>>> +             } sa;
>> 
>> Why is sockaddr_storage included in this union?
> 
> This is from Lennart's original patch. Lennart, care to comment?
> 
> For what it's worth, here is the patch with whitespace ignored, which
> should make it simpler to review:
> 
> 
> diff --git a/Makefile.am b/Makefile.am
> index 9fa608e..194b467 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -38,6 +38,21 @@ rpcbind_SOURCES = \
> 	src/warmstart.c
> rpcbind_LDADD = $(TIRPC_LIBS)
> 
> +if SYSTEMD
> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
> +
> +rpcbind_LDADD += $(SYSTEMD_LIBS)
> +
> +systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
> +	sed -e 's,@bindir\@,$(bindir),g' \
> +		< $< > $@ || rm $@
> +
> +systemdsystemunit_DATA = \
> +	systemd/rpcbind.service \
> +	systemd/rpcbind.socket
> +
> +endif
> +
> rpcinfo_SOURCES =       src/rpcinfo.c
> rpcinfo_LDADD   =       $(TIRPC_LIBS)
> 
> diff --git a/configure.in b/configure.in
> index 2b67720..397d52d 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])
> 
> PKG_CHECK_MODULES([TIRPC], [libtirpc])
> 
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemdsystemunitdir],
> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for
> systemd service files]),
> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG
> --variable=systemdsystemunitdir systemd)])
> +  if test "x$with_systemdsystemunitdir" != xno; then
> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
> +    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> +  fi
> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a
> "x$with_systemdsystemunitdir" != xno ])
> +
> +
> AS_IF([test x$enable_libwrap = xyes], [
> 	AC_CHECK_LIB([wrap], [hosts_access], ,
> 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index 24e069b..a87ce05 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -56,6 +56,9 @@
> #include <netinet/in.h>
> #endif
> #include <arpa/inet.h>
> +#ifdef SYSTEMD
> +#include <systemd/sd-daemon.h>
> +#endif
> #include <fcntl.h>
> #include <netdb.h>
> #include <stdio.h>
> @@ -281,6 +284,7 @@ init_transport(struct netconfig *nconf)
> 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
> 	struct sockaddr_un sun;
> 	mode_t oldmask;
> +	int n = 0;
>         res = NULL;
> 
> 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
> @@ -300,6 +304,76 @@ init_transport(struct netconfig *nconf)
> 	}
> #endif
> 
> +	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> +		syslog(LOG_ERR, "cannot get information for %s",
> +		    nconf->nc_netid);
> +		return (1);
> +	}
> +
> +#ifdef SYSTEMD
> +	n = sd_listen_fds(0);
> +	if (n < 0) {
> +		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
> +		return 1;
> +	}
> +
> +	/* Try to find if one of the systemd sockets we were given match
> +	 * our netconfig structure. */
> +
> +	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
> +		struct __rpc_sockinfo si_other;
> +		union {
> +			struct sockaddr sa;
> +			struct sockaddr_un un;
> +			struct sockaddr_in in4;
> +			struct sockaddr_in6 in6;
> +			struct sockaddr_storage storage;
> +		} sa;
> +		socklen_t addrlen = sizeof(sa);
> +
> +		if (!__rpc_fd2sockinfo(fd, &si_other)) {
> +			syslog(LOG_ERR, "cannot get information for fd %i", fd);
> +			return 1;
> +		}
> +
> +		if (si.si_af != si_other.si_af ||
> +                    si.si_socktype != si_other.si_socktype ||
> +                    si.si_proto != si_other.si_proto)
> +			continue;
> +
> +		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
> +			syslog(LOG_ERR, "failed to query socket name: %s",
> +                               strerror(errno));
> +			goto error;
> +		}
> +
> +		/* Copy the address */
> +		taddr.addr.maxlen = taddr.addr.len = addrlen;
> +		taddr.addr.buf = malloc(addrlen);
> +		if (taddr.addr.buf == NULL) {
> +			syslog(LOG_ERR,
> +                               "cannot allocate memory for %s address",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +		memcpy(taddr.addr.buf, &sa, addrlen);
> +
> +		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> +                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +		if (my_xprt == (SVCXPRT *)NULL) {
> +			syslog(LOG_ERR, "%s: could not create service",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +	}
> +
> +	/* if none of the systemd sockets matched, we set up the socket in
> +	 * the normal way:
> +	 */
> +#endif
> +
> +	if(my_xprt == (SVCXPRT *)NULL) {
> +
> 	/*
> 	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
> 	 * we call this later, for each socket we like to bind.
> @@ -312,12 +386,6 @@ init_transport(struct netconfig *nconf)
> 		}
> 	}
> 
> -	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> -		syslog(LOG_ERR, "cannot get information for %s",
> -		    nconf->nc_netid);
> -		return (1);
> -	}
> -
> 	if ((strcmp(nconf->nc_netid, "local") == 0) ||
> 	    (strcmp(nconf->nc_netid, "unix") == 0)) {
> 		memset(&sun, 0, sizeof sun);
> @@ -554,6 +622,7 @@ init_transport(struct netconfig *nconf)
> 			goto error;
> 		}
> 	}
> +	}
> 
> #ifdef PORTMAP
> 	/*
> diff --git a/systemd/.gitignore b/systemd/.gitignore
> new file mode 100644
> index 0000000..b7b4561
> --- /dev/null
> +++ b/systemd/.gitignore
> @@ -0,0 +1 @@
> +rpcbind.service
> diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
> new file mode 100644
> index 0000000..58ae5de
> --- /dev/null
> +++ b/systemd/rpcbind.service.in
> @@ -0,0 +1,9 @@
> +[Unit]
> +Description=RPC Bind
> +
> +[Service]
> +ExecStart=@bindir@/rpcbind -w -f
> +
> +[Install]
> +WantedBy=multi-user.target
> +Also=rpcbind.socket
> diff --git a/systemd/rpcbind.socket b/systemd/rpcbind.socket
> new file mode 100644
> index 0000000..ad5fd62
> --- /dev/null
> +++ b/systemd/rpcbind.socket
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=RPCbind Server Activation Socket
> +Wants=rpcbind.target
> +Before=rpcbind.target
> +
> +[Socket]
> +ListenStream=/var/run/rpcbind.sock
> +ListenStream=111
> +ListenDatagram=111
> +
> +[Install]
> +WantedBy=sockets.target
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rpcbind: add support for systemd socket activation

[Tom Gundersen]

On Wed, Feb 1, 2012 at 7:16 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> Submit the patch you have below, then white space fixes become subsequent clean up patches.  Not only is review easier now, but if we come back to these changes in a year to fix bugs, it's easier for us to re-learn what it does.

I'll resubmit as two patches (one only doing indentation). Though,
I'll wait a ittle while in case there is more feedback.

> I'll state the question differently: When the SYSTEMD macro is defined, what kind of tests should we run? In general, how do we confirm this is working as expected?  (There may be no good answer at the moment).

In general the patch is meant to achieve two things:

1) If not using systemd, everything should work as before, even if the
SYSTEMD macro is defined.

2) When using systemd, enabling rpcbind.socket should cause rpcbind to
start on demand and act as if it was running all along. Anything that
used to be "After=rpcbind.service" should simply be
"After=rpcbind.socket" (or more generally: "After=rpcbind.target",
which in turn is "After=rpcbind.socket").

Cheers,

Tom

Re: [PATCH] rpcbind: add support for systemd socket activation

[Lennart Poettering]

On Wed, 01.02.12 17:37, Tom Gundersen (teg@jklm.no) wrote:

> >> +     /* Try to find if one of the systemd sockets we were given match
> >> +      * our netconfig structure. */
> >> +
> >> +     for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
> >> +             struct __rpc_sockinfo si_other;
> >> +             union {
> >> +                     struct sockaddr sa;
> >> +                     struct sockaddr_un un;
> >> +                     struct sockaddr_in in4;
> >> +                     struct sockaddr_in6 in6;
> >> +                     struct sockaddr_storage storage;
> >> +             } sa;
> >
> > Why is sockaddr_storage included in this union?
> 
> This is from Lennart's original patch. Lennart, care to comment?

Well, simply because sockaddr_storage is the actual struct one should
normally use for this kind of thing (where you try to determine a
sockaddr from a socket you don't know at all). With one exception
however, sockaddr_un is actually longer than sockaddr_storage, which is
documented borkedness in the socket API.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

On Feb 1, 2012, at 4:45 PM, Lennart Poettering wrote:

> On Wed, 01.02.12 17:37, Tom Gundersen (teg@jklm.no) wrote:
> 
>>>> +     /* Try to find if one of the systemd sockets we were given match
>>>> +      * our netconfig structure. */
>>>> +
>>>> +     for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
>>>> +             struct __rpc_sockinfo si_other;
>>>> +             union {
>>>> +                     struct sockaddr sa;
>>>> +                     struct sockaddr_un un;
>>>> +                     struct sockaddr_in in4;
>>>> +                     struct sockaddr_in6 in6;
>>>> +                     struct sockaddr_storage storage;
>>>> +             } sa;
>>> 
>>> Why is sockaddr_storage included in this union?
>> 
>> This is from Lennart's original patch. Lennart, care to comment?
> 
> Well, simply because sockaddr_storage is the actual struct one should
> normally use for this kind of thing (where you try to determine a
> sockaddr from a socket you don't know at all). With one exception
> however, sockaddr_un is actually longer than sockaddr_storage, which is
> documented borkedness in the socket API.

You don't reference any of the fields inside this union, except for sa.  It seems unnecessary to include all of these members, and then not use most of them.

The biggest address you're ever going to get out of libtirpc is a sockaddr_storage.  If we must stick with a union to prevent pointer aliasing, can we have just two members: sockaddr and sockaddr_storage?

Otherwise, there's no need for this kind of generality here:  TI-RPC handles only IPv4, IPv6, and AF_LOCAL.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH] rpcbind: add support for systemd socket activation

[Lennart Poettering]

On Wed, 01.02.12 17:03, Chuck Lever (chuck.lever@oracle.com) wrote:

> >>> Why is sockaddr_storage included in this union?
> >> 
> >> This is from Lennart's original patch. Lennart, care to comment?
> > 
> > Well, simply because sockaddr_storage is the actual struct one should
> > normally use for this kind of thing (where you try to determine a
> > sockaddr from a socket you don't know at all). With one exception
> > however, sockaddr_un is actually longer than sockaddr_storage, which is
> > documented borkedness in the socket API.
> 
> You don't reference any of the fields inside this union, except for sa.  It seems unnecessary to include all of these members, and then not use most of them.
> 
> The biggest address you're ever going to get out of libtirpc is a sockaddr_storage.  If we must stick with a union to prevent pointer aliasing, can we have just two members: sockaddr and sockaddr_storage?
> 
> Otherwise, there's no need for this kind of generality here:  TI-RPC handles only IPv4, IPv6, and AF_LOCAL.

Well, as said, sockaddr_un is actually larger than sockaddr_storage, and
hence you definitely do need sockaddr_un in the union. The IP cases are
covered by sockaddr_storage however, and hence can be left out of the
union.

I mean, it's completely up to you what is used here, I just figured the
point of the whole library was to be an exercise in keeping things
independent from the used socket family.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

Hi-

On Feb 1, 2012, at 11:37 AM, Tom Gundersen wrote:

> Hi Chuck,
> 
> Thanks for the feedback.
> 
> On Wed, Feb 1, 2012 at 4:32 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>> Typically this is done by breaking the proposed change into smaller atomic patches.  Is that not possible in this case?
> 
> Not entirely sure what you have in mind, I don't immediately see how
> to split this up. Any suggestions welcome.
> 
>>> Added Michal to cc as this patch should go a long way to sort out
>>> <https://bugzilla.redhat.com/show_bug.cgi?id=747363>.
>> 
>> Wouldn't comment 3 also work around problems long enough to give us an opportunity to adequately vet the proposed changes?
> 
> Sure, that's another option.
> 
>> What additions to our test matrix are needed?
> 
> I could not find any tests in the rpcbind git repo. Could you point me
> at your test matrix?
> 
>>> +     /* Try to find if one of the systemd sockets we were given match
>>> +      * our netconfig structure. */
>>> +
>>> +     for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
>>> +             struct __rpc_sockinfo si_other;
>>> +             union {
>>> +                     struct sockaddr sa;
>>> +                     struct sockaddr_un un;
>>> +                     struct sockaddr_in in4;
>>> +                     struct sockaddr_in6 in6;
>>> +                     struct sockaddr_storage storage;
>>> +             } sa;
>> 
>> Why is sockaddr_storage included in this union?
> 
> This is from Lennart's original patch. Lennart, care to comment?
> 
> For what it's worth, here is the patch with whitespace ignored, which
> should make it simpler to review:
> 
> 
> diff --git a/Makefile.am b/Makefile.am
> index 9fa608e..194b467 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -38,6 +38,21 @@ rpcbind_SOURCES = \
> 	src/warmstart.c
> rpcbind_LDADD = $(TIRPC_LIBS)
> 
> +if SYSTEMD
> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
> +
> +rpcbind_LDADD += $(SYSTEMD_LIBS)
> +
> +systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
> +	sed -e 's,@bindir\@,$(bindir),g' \
> +		< $< > $@ || rm $@
> +
> +systemdsystemunit_DATA = \
> +	systemd/rpcbind.service \
> +	systemd/rpcbind.socket
> +
> +endif
> +
> rpcinfo_SOURCES =       src/rpcinfo.c
> rpcinfo_LDADD   =       $(TIRPC_LIBS)
> 
> diff --git a/configure.in b/configure.in
> index 2b67720..397d52d 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])
> 
> PKG_CHECK_MODULES([TIRPC], [libtirpc])
> 
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemdsystemunitdir],
> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for
> systemd service files]),
> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG
> --variable=systemdsystemunitdir systemd)])
> +  if test "x$with_systemdsystemunitdir" != xno; then
> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
> +    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> +  fi
> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a
> "x$with_systemdsystemunitdir" != xno ])
> +
> +
> AS_IF([test x$enable_libwrap = xyes], [
> 	AC_CHECK_LIB([wrap], [hosts_access], ,
> 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))

It's not clear to me how this works on a system that does not have libsystemd-daemon installed.  It appears to require "--with-systemdsystemunitdir=no" which a) is not documented that I can see, b) is awkward ("no" is not a directory name), and c) violates the principal of least surprise.

Our usual practice is to add features so they are enabled when they find all of the dependencies already on the build system, and they are disabled otherwise.  configure options are used to force the situation, but out of the shrink wrap, configure should just work.

This way, typically applying a patch does not require any changes to RPMs or other automated build infrastructure except in rare circumstances.

Did I miss something?

> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index 24e069b..a87ce05 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -56,6 +56,9 @@
> #include <netinet/in.h>
> #endif
> #include <arpa/inet.h>
> +#ifdef SYSTEMD
> +#include <systemd/sd-daemon.h>
> +#endif
> #include <fcntl.h>
> #include <netdb.h>
> #include <stdio.h>
> @@ -281,6 +284,7 @@ init_transport(struct netconfig *nconf)
> 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
> 	struct sockaddr_un sun;
> 	mode_t oldmask;
> +	int n = 0;
>         res = NULL;
> 
> 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
> @@ -300,6 +304,76 @@ init_transport(struct netconfig *nconf)
> 	}
> #endif
> 
> +	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> +		syslog(LOG_ERR, "cannot get information for %s",
> +		    nconf->nc_netid);
> +		return (1);
> +	}
> +
> +#ifdef SYSTEMD
> +	n = sd_listen_fds(0);
> +	if (n < 0) {
> +		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
> +		return 1;
> +	}
> +
> +	/* Try to find if one of the systemd sockets we were given match
> +	 * our netconfig structure. */
> +
> +	for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
> +		struct __rpc_sockinfo si_other;
> +		union {
> +			struct sockaddr sa;
> +			struct sockaddr_un un;
> +			struct sockaddr_in in4;
> +			struct sockaddr_in6 in6;
> +			struct sockaddr_storage storage;
> +		} sa;
> +		socklen_t addrlen = sizeof(sa);
> +
> +		if (!__rpc_fd2sockinfo(fd, &si_other)) {
> +			syslog(LOG_ERR, "cannot get information for fd %i", fd);
> +			return 1;
> +		}
> +
> +		if (si.si_af != si_other.si_af ||
> +                    si.si_socktype != si_other.si_socktype ||
> +                    si.si_proto != si_other.si_proto)
> +			continue;
> +
> +		if (getsockname(fd, &sa.sa, &addrlen) < 0) {
> +			syslog(LOG_ERR, "failed to query socket name: %s",
> +                               strerror(errno));
> +			goto error;
> +		}
> +
> +		/* Copy the address */
> +		taddr.addr.maxlen = taddr.addr.len = addrlen;
> +		taddr.addr.buf = malloc(addrlen);
> +		if (taddr.addr.buf == NULL) {
> +			syslog(LOG_ERR,
> +                               "cannot allocate memory for %s address",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +		memcpy(taddr.addr.buf, &sa, addrlen);
> +
> +		my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
> +                          RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +		if (my_xprt == (SVCXPRT *)NULL) {
> +			syslog(LOG_ERR, "%s: could not create service",
> +                               nconf->nc_netid);
> +			goto error;
> +		}
> +	}
> +
> +	/* if none of the systemd sockets matched, we set up the socket in
> +	 * the normal way:
> +	 */
> +#endif
> +
> +	if(my_xprt == (SVCXPRT *)NULL) {
> +
> 	/*
> 	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
> 	 * we call this later, for each socket we like to bind.
> @@ -312,12 +386,6 @@ init_transport(struct netconfig *nconf)
> 		}
> 	}
> 
> -	if (!__rpc_nconf2sockinfo(nconf, &si)) {
> -		syslog(LOG_ERR, "cannot get information for %s",
> -		    nconf->nc_netid);
> -		return (1);
> -	}
> -
> 	if ((strcmp(nconf->nc_netid, "local") == 0) ||
> 	    (strcmp(nconf->nc_netid, "unix") == 0)) {
> 		memset(&sun, 0, sizeof sun);
> @@ -554,6 +622,7 @@ init_transport(struct netconfig *nconf)
> 			goto error;
> 		}
> 	}
> +	}
> 
> #ifdef PORTMAP
> 	/*
> diff --git a/systemd/.gitignore b/systemd/.gitignore
> new file mode 100644
> index 0000000..b7b4561
> --- /dev/null
> +++ b/systemd/.gitignore
> @@ -0,0 +1 @@
> +rpcbind.service
> diff --git a/systemd/rpcbind.service.in b/systemd/rpcbind.service.in
> new file mode 100644
> index 0000000..58ae5de
> --- /dev/null
> +++ b/systemd/rpcbind.service.in
> @@ -0,0 +1,9 @@
> +[Unit]
> +Description=RPC Bind
> +
> +[Service]
> +ExecStart=@bindir@/rpcbind -w -f
> +
> +[Install]
> +WantedBy=multi-user.target
> +Also=rpcbind.socket
> diff --git a/systemd/rpcbind.socket b/systemd/rpcbind.socket
> new file mode 100644
> index 0000000..ad5fd62
> --- /dev/null
> +++ b/systemd/rpcbind.socket
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=RPCbind Server Activation Socket
> +Wants=rpcbind.target
> +Before=rpcbind.target
> +
> +[Socket]
> +ListenStream=/var/run/rpcbind.sock
> +ListenStream=111
> +ListenDatagram=111
> +
> +[Install]
> +WantedBy=sockets.target
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

Hi-

On Feb 1, 2012, at 6:47 AM, Tom Gundersen wrote:

> Making rpcbind sockect activated will greatly simplify
> its integration in systemd systems. In essence, other services
> may now assume that rpcbind is always available, even during very
> early boot. This means that we no longer need to worry about any
> ordering dependencies.
> 
> This is based on a patch originally posted by Lennart Poettering:
> <http://permalink.gmane.org/gmane.linux.nfs/33774>.
> 
> That patch was not merged due to the lack of a shared library and
> as systemd was seen to be too Fedora specific.
> 
> Systemd now provides a shared library, and it is shipped by defalt in
> OpenSUSE in addition to Fedora, and it is available in Debain, Gentoo,
> Arch, and others.
> 
> This version of the patch has three changes from the original:
> 
> * It uses the shared library.
> * It comes with unit files.
> * It is rebased on top of master.
> 
> Please review the patch with "git show -b" or otherwise ignoring the
> whitespace changes, or it will be extremely difficult to read.
> 
> Comments welcome.
> 
> v2: correctly enable systemd code at compile time
>    handle the case where not all the required sockets were supplied
>    listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
>    do not daemonize
> 
> Original-patch-by: Lennart Poettering <lennart@poettering.net>
> Cc: Michal Schmidt <mschmidt@redhat.com>
> Cc: Steve Dickson <steved@redhat.com>
> Cc: systemd-devel@lists.freedesktop.org
> Acked-by: Cristian Rodríguez<crrodriguez@opensuse.org>
> Signed-off-by: Tom Gundersen <teg@jklm.no>
> ---
> 
> What's the status on this?
> 
> Lennart, does your ack still appl after my changes?
> 
> Steve, any chance of applying this?
> 
> If this is applied I have a couple of follow ups. In particular, I'd like to
> do the cleanup of init_transport that Jim suggested as a separate patch,
> as leaving the line-wraps alone makes this patch easier to read I think.
> 
> Added Michal to cc as this patch should go a long way to sort out
> <https://bugzilla.redhat.com/show_bug.cgi?id=747363>.
> 
> Cheers,
> 
> Tom
> 
> Makefile.am                |   15 ++
> configure.in               |   11 +
> src/rpcbind.c              |  467 +++++++++++++++++++++++++-------------------
> systemd/.gitignore         |    1 +
> systemd/rpcbind.service.in |    9 +
> systemd/rpcbind.socket     |   12 ++
> 6 files changed, 316 insertions(+), 199 deletions(-)
> create mode 100644 systemd/.gitignore
> create mode 100644 systemd/rpcbind.service.in
> create mode 100644 systemd/rpcbind.socket
> 
> diff --git a/Makefile.am b/Makefile.am
> index 9fa608e..194b467 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -38,6 +38,21 @@ rpcbind_SOURCES = \
> 	src/warmstart.c
> rpcbind_LDADD = $(TIRPC_LIBS)
> 
> +if SYSTEMD
> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
> +
> +rpcbind_LDADD += $(SYSTEMD_LIBS)
> +
> +systemd/rpcbind.service: systemd/rpcbind.service.in Makefile
> +	sed -e 's,@bindir\@,$(bindir),g' \
> +		< $< > $@ || rm $@
> +
> +systemdsystemunit_DATA = \
> +	systemd/rpcbind.service \
> +	systemd/rpcbind.socket
> +
> +endif
> +
> rpcinfo_SOURCES =       src/rpcinfo.c
> rpcinfo_LDADD   =       $(TIRPC_LIBS)
> 
> diff --git a/configure.in b/configure.in
> index 2b67720..397d52d 100644
> --- a/configure.in
> +++ b/configure.in

If I do a "make distclean" I don't have a configure.in.  Should you be patching configure.ac instead?

> @@ -29,6 +29,17 @@ AC_SUBST([rpcuser], [$with_rpcuser])
> 
> PKG_CHECK_MODULES([TIRPC], [libtirpc])
> 
> +PKG_PROG_PKG_CONFIG
> +AC_ARG_WITH([systemdsystemunitdir],
> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]),
> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
> +  if test "x$with_systemdsystemunitdir" != xno; then
> +    AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
> +    PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> +  fi
> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
> +
> +
> AS_IF([test x$enable_libwrap = xyes], [
> 	AC_CHECK_LIB([wrap], [hosts_access], ,
> 		AC_MSG_ERROR([libwrap support requested but unable to find libwrap]))
> 

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

[PATCH] rpcbind: add support for systemd socket activation

[Lennart Poettering]

Heya!

(I sent this patch to Steve Dickson earlier, he asked me to post this
here.)

This patch allows us to spawn rpcbind automatically on first use in a
systemd environment, the same way it is done on MacOS X with launchd.

This makes use of socket-based activation as provided by systemd (in
case you are wondering, systemd is a Fedora 14 feature, described in all
detail here: http://0pointer.de/blog/projects/systemd.html). It uses the
reference implementation of the fd passing code, as supplied by systemd
upstream. I hope that's acceptable. It's liberally licensed, so I hope
this is OK. If not, it isn't difficult to reimplement this without the
reference implementation.

The algorithms implemented are described here:

    http://0pointer.de/public/systemd-man/sd_listen_fds.html
    http://0pointer.de/public/systemd-man/sd-daemon.html
    http://0pointer.de/public/systemd-man/daemon.html

Any comments welcome.

If this patch is OK then I can provide systemd unit files as well.

(This patch is generated with --ignore-space-change, to make it more
readable. A version generated without this switch you find here under
http://0pointer.de/public/0001-systemd-add-support-for-system-bus-activation.patch
That's actually the patch you might want to merge. This patch here in
the mail is the one you want to review. Since the rpcbind sources are a
bit chaotic when it comes to indenting and whitespace, this patch
ended up as messy as it is without the switch applied. Sorry for
that. I'd recommend adopting a stricter whitespace regime for the
package, following how they do it for the kernel. Would make it a lot
ncier to prepare patches, and all the git warning madness about weird
whitespace changes in your commits would go away. But anyway, this is
just me nitpicking.)

Lennart

Signed-off-by: Lennart Poettering <lennart@poettering.net>
---
 src/Makefile.am |    6 +-
 src/rpcbind.c   |   75 +++++++++-
 src/sd-daemon.c |  452 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/sd-daemon.h |  257 +++++++++++++++++++++++++++++++
 4 files changed, 783 insertions(+), 7 deletions(-)
 create mode 100644 src/sd-daemon.c
 create mode 100644 src/sd-daemon.h

diff --git a/src/Makefile.am b/src/Makefile.am
index cc0a85b..6340fe9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -20,7 +20,8 @@ rpcbind_SOURCES =       check_bound.c rpcbind.c \
                         rpcb_svc_4.c rpcb_svc_com.c \
                         util.c pmap_svc.c rpcb_stat.c \
                         rpcb_svc.c security.c warmstart.c \
-                        rpcbind.h
+                        rpcbind.h \
+			sd-daemon.c sd-daemon.h
 
 rpcinfo_SOURCES =       rpcinfo.c
 rpcinfo_LDFLAGS =       -lpthread -ltirpc
@@ -32,3 +33,6 @@ rpcbind_LDADD = $(LIB_TIRPC)
 AM_CPPFLAGS = -I/usr/include/tirpc -DCHECK_LOCAL -DPORTMAP \
                        -DFACILITY=LOG_MAIL -DSEVERITY=LOG_INFO
      
+update-systemd:
+	curl http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.c > sd-daemon.c
+	curl http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.h > sd-daemon.h
diff --git a/src/rpcbind.c b/src/rpcbind.c
index 63023e1..0368eaf 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -70,6 +70,7 @@
 #include <nss.h>
 #include "config.h"
 #include "rpcbind.h"
+#include "sd-daemon.h"
 
 /*#define RPCBIND_DEBUG*/
 
@@ -282,6 +283,7 @@ init_transport(struct netconfig *nconf)
 	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
 	struct sockaddr_un sun;
 	mode_t oldmask;
+	int n;
         res = NULL;
 
 	if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -301,6 +303,72 @@ init_transport(struct netconfig *nconf)
 	}
 #endif
 
+	if (!__rpc_nconf2sockinfo(nconf, &si)) {
+		syslog(LOG_ERR, "cannot get information for %s",
+		    nconf->nc_netid);
+		return (1);
+	}
+
+	n = sd_listen_fds(0);
+	if (n < 0) {
+		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
+		return 1;
+	}
+
+	if (n > 0) {
+		int fd;
+
+		/* We have systemd sockets, now find those that match
+		 * our netconfig structure. */
+
+		for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+			struct __rpc_sockinfo si_other;
+			union {
+				struct sockaddr sa;
+				struct sockaddr_un un;
+				struct sockaddr_in in4;
+				struct sockaddr_in6 in6;
+				struct sockaddr_storage storage;
+			} sa;
+			socklen_t addrlen = sizeof(sa);
+
+			if (!__rpc_fd2sockinfo(fd, &si_other)) {
+				syslog(LOG_ERR, "cannog get information for fd %i", fd);
+				return 1;
+			}
+
+			if (si.si_af != si_other.si_af ||
+			    si.si_socktype != si_other.si_socktype ||
+			    si.si_proto != si_other.si_proto)
+				continue;
+
+			if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+				syslog(LOG_ERR, "failed to query socket name: %s",
+				       strerror(errno));
+				goto error;
+			}
+
+			/* Copy the address */
+			taddr.addr.maxlen = taddr.addr.len = addrlen;
+			taddr.addr.buf = malloc(addrlen);
+			if (taddr.addr.buf == NULL) {
+				syslog(LOG_ERR,
+				       "cannot allocate memory for %s address",
+				       nconf->nc_netid);
+				goto error;
+			}
+			memcpy(taddr.addr.buf, &sa, addrlen);
+
+			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+                                RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+			if (my_xprt == (SVCXPRT *)NULL) {
+				syslog(LOG_ERR, "%s: could not create service",
+                                        nconf->nc_netid);
+				goto error;
+			}
+		}
+	} else {
+
 	/*
 	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
 	 * we call this later, for each socket we like to bind.
@@ -313,12 +381,6 @@ init_transport(struct netconfig *nconf)
 		}
 	}
 
-	if (!__rpc_nconf2sockinfo(nconf, &si)) {
-		syslog(LOG_ERR, "cannot get information for %s",
-		    nconf->nc_netid);
-		return (1);
-	}
-
 	if ((strcmp(nconf->nc_netid, "local") == 0) ||
 	    (strcmp(nconf->nc_netid, "unix") == 0)) {
 		memset(&sun, 0, sizeof sun);
@@ -555,6 +617,7 @@ init_transport(struct netconfig *nconf)
 			goto error;
 		}
 	}
+	}
 
 #ifdef PORTMAP
 	/*
diff --git a/src/sd-daemon.c b/src/sd-daemon.c
new file mode 100644
index 0000000..3bb258d
--- /dev/null
+++ b/src/sd-daemon.c
@@ -0,0 +1,452 @@
+/*-*- Mode: C; c-basic-offset: 8 -*-*/
+
+/***
+  Copyright 2010 Lennart Poettering
+
+  Permission is hereby granted, free of charge, to any person
+  obtaining a copy of this software and associated documentation files
+  (the "Software"), to deal in the Software without restriction,
+  including without limitation the rights to use, copy, modify, merge,
+  publish, distribute, sublicense, and/or sell copies of the Software,
+  and to permit persons to whom the Software is furnished to do so,
+  subject to the following conditions:
+
+  The above copyright notice and this permission notice shall be
+  included in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+  SOFTWARE.
+***/
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <sys/fcntl.h>
+#include <netinet/in.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stdio.h>
+
+#include "sd-daemon.h"
+
+int sd_listen_fds(int unset_environment) {
+
+#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
+        return 0;
+#else
+        int r, fd;
+        const char *e;
+        char *p = NULL;
+        unsigned long l;
+
+        if (!(e = getenv("LISTEN_PID"))) {
+                r = 0;
+                goto finish;
+        }
+
+        errno = 0;
+        l = strtoul(e, &p, 10);
+
+        if (errno != 0) {
+                r = -errno;
+                goto finish;
+        }
+
+        if (!p || *p || l <= 0) {
+                r = -EINVAL;
+                goto finish;
+        }
+
+        /* Is this for us? */
+        if (getpid() != (pid_t) l) {
+                r = 0;
+                goto finish;
+        }
+
+        if (!(e = getenv("LISTEN_FDS"))) {
+                r = 0;
+                goto finish;
+        }
+
+        errno = 0;
+        l = strtoul(e, &p, 10);
+
+        if (errno != 0) {
+                r = -errno;
+                goto finish;
+        }
+
+        if (!p || *p) {
+                r = -EINVAL;
+                goto finish;
+        }
+
+        for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + (int) l; fd ++) {
+                int flags;
+
+                if ((flags = fcntl(fd, F_GETFD)) < 0) {
+                        r = -errno;
+                        goto finish;
+                }
+
+                if (flags & FD_CLOEXEC)
+                        continue;
+
+                if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) {
+                        r = -errno;
+                        goto finish;
+                }
+        }
+
+        r = (int) l;
+
+finish:
+        if (unset_environment) {
+                unsetenv("LISTEN_PID");
+                unsetenv("LISTEN_FDS");
+        }
+
+        return r;
+#endif
+}
+
+int sd_is_fifo(int fd, const char *path) {
+        struct stat st_fd;
+
+        if (fd < 0)
+                return -EINVAL;
+
+        memset(&st_fd, 0, sizeof(st_fd));
+        if (fstat(fd, &st_fd) < 0)
+                return -errno;
+
+        if (!S_ISFIFO(st_fd.st_mode))
+                return 0;
+
+        if (path) {
+                struct stat st_path;
+
+                memset(&st_path, 0, sizeof(st_path));
+                if (stat(path, &st_path) < 0) {
+
+                        if (errno == ENOENT || errno == ENOTDIR)
+                                return 0;
+
+                        return -errno;
+                }
+
+                return
+                        st_path.st_dev == st_fd.st_dev &&
+                        st_path.st_ino == st_fd.st_ino;
+        }
+
+        return 1;
+}
+
+static int sd_is_socket_internal(int fd, int type, int listening) {
+        struct stat st_fd;
+
+        if (fd < 0 || type < 0)
+                return -EINVAL;
+
+        if (fstat(fd, &st_fd) < 0)
+                return -errno;
+
+        if (!S_ISSOCK(st_fd.st_mode))
+                return 0;
+
+        if (type != 0) {
+                int other_type = 0;
+                socklen_t l = sizeof(other_type);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &other_type, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(other_type))
+                        return -EINVAL;
+
+                if (other_type != type)
+                        return 0;
+        }
+
+        if (listening >= 0) {
+                int accepting = 0;
+                socklen_t l = sizeof(accepting);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &accepting, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(accepting))
+                        return -EINVAL;
+
+                if (!accepting != !listening)
+                        return 0;
+        }
+
+        return 1;
+}
+
+union sockaddr_union {
+        struct sockaddr sa;
+        struct sockaddr_in in4;
+        struct sockaddr_in6 in6;
+        struct sockaddr_un un;
+        struct sockaddr_storage storage;
+};
+
+int sd_is_socket(int fd, int family, int type, int listening) {
+        int r;
+
+        if (family < 0)
+                return -EINVAL;
+
+        if ((r = sd_is_socket_internal(fd, type, listening)) <= 0)
+                return r;
+
+        if (family > 0) {
+                union sockaddr_union sockaddr;
+                socklen_t l;
+
+                memset(&sockaddr, 0, sizeof(sockaddr));
+                l = sizeof(sockaddr);
+
+                if (getsockname(fd, &sockaddr.sa, &l) < 0)
+                        return -errno;
+
+                if (l < sizeof(sa_family_t))
+                        return -EINVAL;
+
+                return sockaddr.sa.sa_family == family;
+        }
+
+        return 1;
+}
+
+int sd_is_socket_inet(int fd, int family, int type, int listening, uint16_t port) {
+        union sockaddr_union sockaddr;
+        socklen_t l;
+        int r;
+
+        if (family != 0 && family != AF_INET && family != AF_INET6)
+                return -EINVAL;
+
+        if ((r = sd_is_socket_internal(fd, type, listening)) <= 0)
+                return r;
+
+        memset(&sockaddr, 0, sizeof(sockaddr));
+        l = sizeof(sockaddr);
+
+        if (getsockname(fd, &sockaddr.sa, &l) < 0)
+                return -errno;
+
+        if (l < sizeof(sa_family_t))
+                return -EINVAL;
+
+        if (sockaddr.sa.sa_family != AF_INET &&
+            sockaddr.sa.sa_family != AF_INET6)
+                return 0;
+
+        if (family > 0)
+                if (sockaddr.sa.sa_family != family)
+                        return 0;
+
+        if (port > 0) {
+                if (sockaddr.sa.sa_family == AF_INET) {
+                        if (l < sizeof(struct sockaddr_in))
+                                return -EINVAL;
+
+                        return htons(port) == sockaddr.in4.sin_port;
+                } else {
+                        if (l < sizeof(struct sockaddr_in6))
+                                return -EINVAL;
+
+                        return htons(port) == sockaddr.in6.sin6_port;
+                }
+        }
+
+        return 1;
+}
+
+int sd_is_socket_unix(int fd, int type, int listening, const char *path, size_t length) {
+        union sockaddr_union sockaddr;
+        socklen_t l;
+        int r;
+
+        if ((r = sd_is_socket_internal(fd, type, listening)) <= 0)
+                return r;
+
+        memset(&sockaddr, 0, sizeof(sockaddr));
+        l = sizeof(sockaddr);
+
+        if (getsockname(fd, &sockaddr.sa, &l) < 0)
+                return -errno;
+
+        if (l < sizeof(sa_family_t))
+                return -EINVAL;
+
+        if (sockaddr.sa.sa_family != AF_UNIX)
+                return 0;
+
+        if (path) {
+                if (length <= 0)
+                        length = strlen(path);
+
+                if (length <= 0)
+                        /* Unnamed socket */
+                        return l == sizeof(sa_family_t);
+
+                if (path[0])
+                        /* Normal path socket */
+                        return
+                                (l >= sizeof(sa_family_t) + length + 1) &&
+                                memcmp(path, sockaddr.un.sun_path, length+1) == 0;
+                else
+                        /* Abstract namespace socket */
+                        return
+                                (l == sizeof(sa_family_t) + length) &&
+                                memcmp(path, sockaddr.un.sun_path, length) == 0;
+        }
+
+        return 1;
+}
+
+int sd_notify(int unset_environment, const char *state) {
+#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
+        return 0;
+#else
+        int fd = -1, r;
+        struct msghdr msghdr;
+        struct iovec iovec;
+        union sockaddr_union sockaddr;
+        struct ucred *ucred;
+        union {
+                struct cmsghdr cmsghdr;
+                uint8_t buf[CMSG_SPACE(sizeof(struct ucred))];
+        } control;
+        const char *e;
+
+        if (!state) {
+                r = -EINVAL;
+                goto finish;
+        }
+
+        if (!(e = getenv("NOTIFY_SOCKET")))
+                return 0;
+
+        /* Must be an abstract socket, or an absolute path */
+        if ((e[0] != '@' && e[0] != '/') || e[1] == 0) {
+                r = -EINVAL;
+                goto finish;
+        }
+
+        if ((fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0)) < 0) {
+                r = -errno;
+                goto finish;
+        }
+
+        memset(&sockaddr, 0, sizeof(sockaddr));
+        sockaddr.sa.sa_family = AF_UNIX;
+        strncpy(sockaddr.un.sun_path, e, sizeof(sockaddr.un.sun_path));
+
+        if (sockaddr.un.sun_path[0] == '@')
+                sockaddr.un.sun_path[0] = 0;
+
+        memset(&iovec, 0, sizeof(iovec));
+        iovec.iov_base = (char*) state;
+        iovec.iov_len = strlen(state);
+
+        memset(&control, 0, sizeof(control));
+        control.cmsghdr.cmsg_level = SOL_SOCKET;
+        control.cmsghdr.cmsg_type = SCM_CREDENTIALS;
+        control.cmsghdr.cmsg_len = CMSG_LEN(sizeof(struct ucred));
+
+        ucred = (struct ucred*) CMSG_DATA(&control.cmsghdr);
+        ucred->pid = getpid();
+        ucred->uid = getuid();
+        ucred->gid = getgid();
+
+        memset(&msghdr, 0, sizeof(msghdr));
+        msghdr.msg_name = &sockaddr;
+        msghdr.msg_namelen = sizeof(sa_family_t) + strlen(e);
+
+        if (msghdr.msg_namelen > sizeof(struct sockaddr_un))
+                msghdr.msg_namelen = sizeof(struct sockaddr_un);
+
+        msghdr.msg_iov = &iovec;
+        msghdr.msg_iovlen = 1;
+        msghdr.msg_control = &control;
+        msghdr.msg_controllen = control.cmsghdr.cmsg_len;
+
+        if (sendmsg(fd, &msghdr, MSG_NOSIGNAL) < 0) {
+                r = -errno;
+                goto finish;
+        }
+
+        r = 1;
+
+finish:
+        if (unset_environment)
+                unsetenv("NOTIFY_SOCKET");
+
+        if (fd >= 0)
+                close(fd);
+
+        return r;
+#endif
+}
+
+int sd_notifyf(int unset_environment, const char *format, ...) {
+#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
+        return 0;
+#else
+        va_list ap;
+        char *p = NULL;
+        int r;
+
+        va_start(ap, format);
+        r = vasprintf(&p, format, ap);
+        va_end(ap);
+
+        if (r < 0 || !p)
+                return -ENOMEM;
+
+        r = sd_notify(unset_environment, p);
+        free(p);
+
+        return r;
+#endif
+}
+
+int sd_booted(void) {
+#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
+        return 0;
+#else
+
+        struct stat a, b;
+
+        /* We simply test whether the systemd cgroup hierarchy is
+         * mounted */
+
+        if (lstat("/cgroup", &a) < 0)
+                return 0;
+
+        if (lstat("/cgroup/systemd", &b) < 0)
+                return 0;
+
+        return a.st_dev != b.st_dev;
+#endif
+}
diff --git a/src/sd-daemon.h b/src/sd-daemon.h
new file mode 100644
index 0000000..fd6221f
--- /dev/null
+++ b/src/sd-daemon.h
@@ -0,0 +1,257 @@
+/*-*- Mode: C; c-basic-offset: 8 -*-*/
+
+#ifndef foosddaemonhfoo
+#define foosddaemonhfoo
+
+/***
+  Copyright 2010 Lennart Poettering
+
+  Permission is hereby granted, free of charge, to any person
+  obtaining a copy of this software and associated documentation files
+  (the "Software"), to deal in the Software without restriction,
+  including without limitation the rights to use, copy, modify, merge,
+  publish, distribute, sublicense, and/or sell copies of the Software,
+  and to permit persons to whom the Software is furnished to do so,
+  subject to the following conditions:
+
+  The above copyright notice and this permission notice shall be
+  included in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+  SOFTWARE.
+***/
+
+#include <sys/types.h>
+#include <inttypes.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+  Reference implementation of a few systemd related interfaces for
+  writing daemons. These interfaces are trivial to implement. To
+  simplify porting we provide this reference implementation.
+  Applications are welcome to reimplement the algorithms described
+  here if they do not want to include these two source files.
+
+  The following functionality is provided:
+
+  - Support for logging with log levels on stderr
+  - File descriptor passing for socket-based activation
+  - Daemon startup and status notification
+  - Detection of systemd boots
+
+  You may compile this with -DDISABLE_SYSTEMD to disable systemd
+  support. This makes all those calls NOPs that are directly related to
+  systemd (i.e. only sd_is_xxx() will stay useful).
+
+  Since this is drop-in code we don't want any of our symbols to be
+  exported in any case. Hence we declare hidden visibility for all of
+  them.
+
+  You may find an up-to-date version of these source files online:
+
+  http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.h
+  http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.c
+
+  This should compile on non-Linux systems, too, but with the
+  exception of the sd_is_xxx() calls all functions will become NOPs.
+
+  See sd-daemon(7) for more information.
+*/
+
+#if __GNUC__ >= 4
+#define _sd_printf_attr_(a,b) __attribute__ ((format (printf, a, b)))
+#define _sd_hidden_ __attribute__ ((visibility("hidden")))
+#else
+#define _sd_printf_attr_(a,b)
+#define _sd_hidden_
+#endif
+
+/*
+  Log levels for usage on stderr:
+
+          fprintf(stderr, SD_NOTICE "Hello World!\n");
+
+  This is similar to printk() usage in the kernel.
+*/
+#define SD_EMERG   "<0>"  /* system is unusable */
+#define SD_ALERT   "<1>"  /* action must be taken immediately */
+#define SD_CRIT    "<2>"  /* critical conditions */
+#define SD_ERR     "<3>"  /* error conditions */
+#define SD_WARNING "<4>"  /* warning conditions */
+#define SD_NOTICE  "<5>"  /* normal but significant condition */
+#define SD_INFO    "<6>"  /* informational */
+#define SD_DEBUG   "<7>"  /* debug-level messages */
+
+/* The first passed file descriptor is fd 3 */
+#define SD_LISTEN_FDS_START 3
+
+/*
+  Returns how many file descriptors have been passed, or a negative
+  errno code on failure. Optionally, removes the $LISTEN_FDS and
+  $LISTEN_PID file descriptors from the environment (recommended, but
+  problematic in threaded environments). If r is the return value of
+  this function you'll find the file descriptors passed as fds
+  SD_LISTEN_FDS_START to SD_LISTEN_FDS_START+r-1. Returns a negative
+  errno style error code on failure. This function call ensures that
+  the FD_CLOEXEC flag is set for the passed file descriptors, to make
+  sure they are not passed on to child processes. If FD_CLOEXEC shall
+  not be set, the caller needs to unset it after this call for all file
+  descriptors that are used.
+
+  See sd_listen_fds(3) for more information.
+*/
+int sd_listen_fds(int unset_environment) _sd_hidden_;
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is a FIFO in the file system stored under the
+  specified path, 0 otherwise. If path is NULL a path name check will
+  not be done and the call only verifies if the file descriptor
+  refers to a FIFO. Returns a negative errno style error code on
+  failure.
+
+  See sd_is_fifo(3) for more information.
+*/
+int sd_is_fifo(int fd, const char *path) _sd_hidden_;
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is a socket of the specified family (AF_INET,
+  ...) and type (SOCK_DGRAM, SOCK_STREAM, ...), 0 otherwise. If
+  family is 0 a socket family check will not be done. If type is 0 a
+  socket type check will not be done and the call only verifies if
+  the file descriptor refers to a socket. If listening is > 0 it is
+  verified that the socket is in listening mode. (i.e. listen() has
+  been called) If listening is == 0 it is verified that the socket is
+  not in listening mode. If listening is < 0 no listening mode check
+  is done. Returns a negative errno style error code on failure.
+
+  See sd_is_socket(3) for more information.
+*/
+int sd_is_socket(int fd, int family, int type, int listening) _sd_hidden_;
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is an Internet socket, of the specified family
+  (either AF_INET or AF_INET6) and the specified type (SOCK_DGRAM,
+  SOCK_STREAM, ...), 0 otherwise. If version is 0 a protocol version
+  check is not done. If type is 0 a socket type check will not be
+  done. If port is 0 a socket port check will not be done. The
+  listening flag is used the same way as in sd_is_socket(). Returns a
+  negative errno style error code on failure.
+
+  See sd_is_socket_inet(3) for more information.
+*/
+int sd_is_socket_inet(int fd, int family, int type, int listening, uint16_t port) _sd_hidden_;
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is an AF_UNIX socket of the specified type
+  (SOCK_DGRAM, SOCK_STREAM, ...) and path, 0 otherwise. If type is 0
+  a socket type check will not be done. If path is NULL a socket path
+  check will not be done. For normal AF_UNIX sockets set length to
+  0. For abstract namespace sockets set length to the length of the
+  socket name (including the initial 0 byte), and pass the full
+  socket path in path (including the initial 0 byte). The listening
+  flag is used the same way as in sd_is_socket(). Returns a negative
+  errno style error code on failure.
+
+  See sd_is_socket_unix(3) for more information.
+*/
+int sd_is_socket_unix(int fd, int type, int listening, const char *path, size_t length) _sd_hidden_;
+
+/*
+  Informs systemd about changed daemon state. This takes a number of
+  newline seperated environment-style variable assignments in a
+  string. The following variables are known:
+
+     READY=1      Tells systemd that daemon startup is finished (only
+                  relevant for services of Type=notify). The passed
+                  argument is a boolean "1" or "0". Since there is
+                  little value in signalling non-readiness the only
+                  value daemons should send is "READY=1".
+
+     STATUS=...   Passes a single-line status string back to systemd
+                  that describes the daemon state. This is free-from
+                  and can be used for various purposes: general state
+                  feedback, fsck-like programs could pass completion
+                  percentages and failing programs could pass a human
+                  readable error message. Example: "STATUS=Completed
+                  66% of file system check..."
+
+     ERRNO=...    If a daemon fails, the errno-style error code,
+                  formatted as string. Example: "ERRNO=2" for ENOENT.
+
+     BUSERROR=... If a daemon fails, the D-Bus error-style error
+                  code. Example: "BUSERROR=org.freedesktop.DBus.Error.TimedOut"
+
+     MAINPID=...  The main pid of a daemon, in case systemd did not
+                  fork off the process itself. Example: "MAINPID=4711"
+
+  Daemons can choose to send additional variables. However, it is
+  recommened to prefix variable names not listed above with X_.
+
+  Returns a negative errno-style error code on failure. Returns > 0
+  if systemd could be notified, 0 if it couldn't possibly because
+  systemd is not running.
+
+  Example: When a daemon finished starting up, it could issue this
+  call to notify systemd about it:
+
+     sd_notify(0, "READY=1");
+
+  See sd_notifyf() for more complete examples.
+
+  See sd_notify(3) for more information.
+*/
+int sd_notify(int unset_environment, const char *state) _sd_hidden_;
+
+/*
+  Similar to sd_notify() but takes a format string.
+
+  Example 1: A daemon could send the following after initialization:
+
+     sd_notifyf(0, "READY=1\n"
+                   "STATUS=Processing requests...\n"
+                   "MAINPID=%lu",
+                   (unsigned long) getpid());
+
+  Example 2: A daemon could send the following shortly before
+  exiting, on failure:
+
+     sd_notifyf(0, "STATUS=Failed to start up: %s\n"
+                   "ERRNO=%i",
+                   strerror(errno),
+                   errno);
+
+  See sd_notifyf(3) for more information.
+*/
+int sd_notifyf(int unset_environment, const char *format, ...) _sd_printf_attr_(2,3) _sd_hidden_;
+
+/*
+  Returns > 0 if the system was booted with systemd. Returns < 0 on
+  error. Returns 0 if the system was not booted with systemd. Note
+  that all of the functions above handle non-systemd boots just
+  fine. You should NOT protect them with a call to this function. Also
+  note that this function checks whether the system, not the user
+  session is controlled by systemd. However the functions above work
+  for both session and system services.
+
+  See sd_booted(3) for more information.
+*/
+int sd_booted(void) _sd_hidden_;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
-- 
1.7.0.1



Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

On 07/18/10 10:19 PM, Lennart Poettering wrote:
> Heya!
>
> (I sent this patch to Steve Dickson earlier, he asked me to post this
> here.)
>
> This patch allows us to spawn rpcbind automatically on first use in a
> systemd environment, the same way it is done on MacOS X with launchd.
>
> This makes use of socket-based activation as provided by systemd (in
> case you are wondering, systemd is a Fedora 14 feature, described in all
> detail here: http://0pointer.de/blog/projects/systemd.html). It uses the
> reference implementation of the fd passing code, as supplied by systemd
> upstream. I hope that's acceptable. It's liberally licensed, so I hope
> this is OK. If not, it isn't difficult to reimplement this without the
> reference implementation.
>
> The algorithms implemented are described here:
>
>      http://0pointer.de/public/systemd-man/sd_listen_fds.html
>      http://0pointer.de/public/systemd-man/sd-daemon.html
>      http://0pointer.de/public/systemd-man/daemon.html
>
> Any comments welcome.
>
> If this patch is OK then I can provide systemd unit files as well.
>
> (This patch is generated with --ignore-space-change, to make it more
> readable. A version generated without this switch you find here under
> http://0pointer.de/public/0001-systemd-add-support-for-system-bus-activation.patch
> That's actually the patch you might want to merge. This patch here in
> the mail is the one you want to review. Since the rpcbind sources are a
> bit chaotic when it comes to indenting and whitespace, this patch
> ended up as messy as it is without the switch applied. Sorry for
> that. I'd recommend adopting a stricter whitespace regime for the
> package, following how they do it for the kernel. Would make it a lot
> ncier to prepare patches, and all the git warning madness about weird
> whitespace changes in your commits would go away. But anyway, this is
> just me nitpicking.)
>
> Lennart
>
> Signed-off-by: Lennart Poettering<lennart@poettering.net>
> ---
>   src/Makefile.am |    6 +-
>   src/rpcbind.c   |   75 +++++++++-
>   src/sd-daemon.c |  452 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/sd-daemon.h |  257 +++++++++++++++++++++++++++++++
>   4 files changed, 783 insertions(+), 7 deletions(-)
>   create mode 100644 src/sd-daemon.c
>   create mode 100644 src/sd-daemon.h

Why aren't sd-daemon.[ch] part of a systemd related library?  They seem 
pretty generic to me, and could be useful for other daemons.

Plus, the newly introduced update-systemd Makefile target seems to 
suggest these won't be stable over time.  Wouldn't it be better to have 
a versioned library, external to rpcbind, with this code in it?

> diff --git a/src/Makefile.am b/src/Makefile.am
> index cc0a85b..6340fe9 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -20,7 +20,8 @@ rpcbind_SOURCES =       check_bound.c rpcbind.c \
>                           rpcb_svc_4.c rpcb_svc_com.c \
>                           util.c pmap_svc.c rpcb_stat.c \
>                           rpcb_svc.c security.c warmstart.c \
> -                        rpcbind.h
> +                        rpcbind.h \
> +			sd-daemon.c sd-daemon.h
>
>   rpcinfo_SOURCES =       rpcinfo.c
>   rpcinfo_LDFLAGS =       -lpthread -ltirpc
> @@ -32,3 +33,6 @@ rpcbind_LDADD = $(LIB_TIRPC)
>   AM_CPPFLAGS = -I/usr/include/tirpc -DCHECK_LOCAL -DPORTMAP \
>                          -DFACILITY=LOG_MAIL -DSEVERITY=LOG_INFO
>
> +update-systemd:
> +	curl http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.c>  sd-daemon.c
> +	curl http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.h>  sd-daemon.h
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index 63023e1..0368eaf 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -70,6 +70,7 @@
>   #include<nss.h>
>   #include "config.h"
>   #include "rpcbind.h"
> +#include "sd-daemon.h"
>
>   /*#define RPCBIND_DEBUG*/
>
> @@ -282,6 +283,7 @@ init_transport(struct netconfig *nconf)
>   	u_int32_t host_addr[4];  /* IPv4 or IPv6 */
>   	struct sockaddr_un sun;
>   	mode_t oldmask;
> +	int n;
>           res = NULL;
>
>   	if ((nconf->nc_semantics != NC_TPI_CLTS)&&
> @@ -301,6 +303,72 @@ init_transport(struct netconfig *nconf)
>   	}
>   #endif
>
> +	if (!__rpc_nconf2sockinfo(nconf,&si)) {
> +		syslog(LOG_ERR, "cannot get information for %s",
> +		    nconf->nc_netid);
> +		return (1);
> +	}
> +
> +	n = sd_listen_fds(0);
> +	if (n<  0) {
> +		syslog(LOG_ERR, "failed to acquire systemd scokets: %s", strerror(-n));
> +		return 1;
> +	}
> +
> +	if (n>  0) {
> +		int fd;
> +
> +		/* We have systemd sockets, now find those that match
> +		 * our netconfig structure. */
> +
> +		for (fd = SD_LISTEN_FDS_START; fd<  SD_LISTEN_FDS_START + n; fd++) {
> +			struct __rpc_sockinfo si_other;
> +			union {
> +				struct sockaddr sa;
> +				struct sockaddr_un un;
> +				struct sockaddr_in in4;
> +				struct sockaddr_in6 in6;
> +				struct sockaddr_storage storage;
> +			} sa;
> +			socklen_t addrlen = sizeof(sa);
> +
> +			if (!__rpc_fd2sockinfo(fd,&si_other)) {
> +				syslog(LOG_ERR, "cannog get information for fd %i", fd);
> +				return 1;
> +			}
> +
> +			if (si.si_af != si_other.si_af ||
> +			    si.si_socktype != si_other.si_socktype ||
> +			    si.si_proto != si_other.si_proto)
> +				continue;
> +
> +			if (getsockname(fd,&sa.sa,&addrlen)<  0) {
> +				syslog(LOG_ERR, "failed to query socket name: %s",
> +				       strerror(errno));
> +				goto error;
> +			}
> +
> +			/* Copy the address */
> +			taddr.addr.maxlen = taddr.addr.len = addrlen;
> +			taddr.addr.buf = malloc(addrlen);
> +			if (taddr.addr.buf == NULL) {
> +				syslog(LOG_ERR,
> +				       "cannot allocate memory for %s address",
> +				       nconf->nc_netid);
> +				goto error;
> +			}
> +			memcpy(taddr.addr.buf,&sa, addrlen);
> +
> +			my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf,&taddr,
> +                                RPC_MAXDATASIZE, RPC_MAXDATASIZE);
> +			if (my_xprt == (SVCXPRT *)NULL) {
> +				syslog(LOG_ERR, "%s: could not create service",
> +                                        nconf->nc_netid);
> +				goto error;
> +			}
> +		}
> +	} else {
> +
>   	/*
>   	 * XXX - using RPC library internal functions. For NC_TPI_CLTS
>   	 * we call this later, for each socket we like to bind.
> @@ -313,12 +381,6 @@ init_transport(struct netconfig *nconf)
>   		}
>   	}
>
> -	if (!__rpc_nconf2sockinfo(nconf,&si)) {
> -		syslog(LOG_ERR, "cannot get information for %s",
> -		    nconf->nc_netid);
> -		return (1);
> -	}
> -
>   	if ((strcmp(nconf->nc_netid, "local") == 0) ||
>   	    (strcmp(nconf->nc_netid, "unix") == 0)) {
>   		memset(&sun, 0, sizeof sun);
> @@ -555,6 +617,7 @@ init_transport(struct netconfig *nconf)
>   			goto error;
>   		}
>   	}
> +	}
>
>   #ifdef PORTMAP
>   	/*
> diff --git a/src/sd-daemon.c b/src/sd-daemon.c
> new file mode 100644
> index 0000000..3bb258d
> --- /dev/null
> +++ b/src/sd-daemon.c
> @@ -0,0 +1,452 @@
> +/*-*- Mode: C; c-basic-offset: 8 -*-*/
> +
> +/***
> +  Copyright 2010 Lennart Poettering
> +
> +  Permission is hereby granted, free of charge, to any person
> +  obtaining a copy of this software and associated documentation files
> +  (the "Software"), to deal in the Software without restriction,
> +  including without limitation the rights to use, copy, modify, merge,
> +  publish, distribute, sublicense, and/or sell copies of the Software,
> +  and to permit persons to whom the Software is furnished to do so,
> +  subject to the following conditions:
> +
> +  The above copyright notice and this permission notice shall be
> +  included in all copies or substantial portions of the Software.
> +
> +  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> +  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> +  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> +  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +  SOFTWARE.
> +***/
> +
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
> +#include<sys/types.h>
> +#include<sys/stat.h>
> +#include<sys/socket.h>
> +#include<sys/un.h>
> +#include<sys/fcntl.h>
> +#include<netinet/in.h>
> +#include<stdlib.h>
> +#include<errno.h>
> +#include<unistd.h>
> +#include<string.h>
> +#include<stdarg.h>
> +#include<stdio.h>
> +
> +#include "sd-daemon.h"
> +
> +int sd_listen_fds(int unset_environment) {
> +
> +#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
> +        return 0;
> +#else
> +        int r, fd;
> +        const char *e;
> +        char *p = NULL;
> +        unsigned long l;
> +
> +        if (!(e = getenv("LISTEN_PID"))) {
> +                r = 0;
> +                goto finish;
> +        }
> +
> +        errno = 0;
> +        l = strtoul(e,&p, 10);
> +
> +        if (errno != 0) {
> +                r = -errno;
> +                goto finish;
> +        }
> +
> +        if (!p || *p || l<= 0) {
> +                r = -EINVAL;
> +                goto finish;
> +        }
> +
> +        /* Is this for us? */
> +        if (getpid() != (pid_t) l) {
> +                r = 0;
> +                goto finish;
> +        }
> +
> +        if (!(e = getenv("LISTEN_FDS"))) {
> +                r = 0;
> +                goto finish;
> +        }
> +
> +        errno = 0;
> +        l = strtoul(e,&p, 10);
> +
> +        if (errno != 0) {
> +                r = -errno;
> +                goto finish;
> +        }
> +
> +        if (!p || *p) {
> +                r = -EINVAL;
> +                goto finish;
> +        }
> +
> +        for (fd = SD_LISTEN_FDS_START; fd<  SD_LISTEN_FDS_START + (int) l; fd ++) {
> +                int flags;
> +
> +                if ((flags = fcntl(fd, F_GETFD))<  0) {
> +                        r = -errno;
> +                        goto finish;
> +                }
> +
> +                if (flags&  FD_CLOEXEC)
> +                        continue;
> +
> +                if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC)<  0) {
> +                        r = -errno;
> +                        goto finish;
> +                }
> +        }
> +
> +        r = (int) l;
> +
> +finish:
> +        if (unset_environment) {
> +                unsetenv("LISTEN_PID");
> +                unsetenv("LISTEN_FDS");
> +        }
> +
> +        return r;
> +#endif
> +}
> +
> +int sd_is_fifo(int fd, const char *path) {
> +        struct stat st_fd;
> +
> +        if (fd<  0)
> +                return -EINVAL;
> +
> +        memset(&st_fd, 0, sizeof(st_fd));
> +        if (fstat(fd,&st_fd)<  0)
> +                return -errno;
> +
> +        if (!S_ISFIFO(st_fd.st_mode))
> +                return 0;
> +
> +        if (path) {
> +                struct stat st_path;
> +
> +                memset(&st_path, 0, sizeof(st_path));
> +                if (stat(path,&st_path)<  0) {
> +
> +                        if (errno == ENOENT || errno == ENOTDIR)
> +                                return 0;
> +
> +                        return -errno;
> +                }
> +
> +                return
> +                        st_path.st_dev == st_fd.st_dev&&
> +                        st_path.st_ino == st_fd.st_ino;
> +        }
> +
> +        return 1;
> +}
> +
> +static int sd_is_socket_internal(int fd, int type, int listening) {
> +        struct stat st_fd;
> +
> +        if (fd<  0 || type<  0)
> +                return -EINVAL;
> +
> +        if (fstat(fd,&st_fd)<  0)
> +                return -errno;
> +
> +        if (!S_ISSOCK(st_fd.st_mode))
> +                return 0;
> +
> +        if (type != 0) {
> +                int other_type = 0;
> +                socklen_t l = sizeof(other_type);
> +
> +                if (getsockopt(fd, SOL_SOCKET, SO_TYPE,&other_type,&l)<  0)
> +                        return -errno;
> +
> +                if (l != sizeof(other_type))
> +                        return -EINVAL;
> +
> +                if (other_type != type)
> +                        return 0;
> +        }
> +
> +        if (listening>= 0) {
> +                int accepting = 0;
> +                socklen_t l = sizeof(accepting);
> +
> +                if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN,&accepting,&l)<  0)
> +                        return -errno;
> +
> +                if (l != sizeof(accepting))
> +                        return -EINVAL;
> +
> +                if (!accepting != !listening)
> +                        return 0;
> +        }
> +
> +        return 1;
> +}
> +
> +union sockaddr_union {
> +        struct sockaddr sa;
> +        struct sockaddr_in in4;
> +        struct sockaddr_in6 in6;
> +        struct sockaddr_un un;
> +        struct sockaddr_storage storage;
> +};
> +
> +int sd_is_socket(int fd, int family, int type, int listening) {
> +        int r;
> +
> +        if (family<  0)
> +                return -EINVAL;
> +
> +        if ((r = sd_is_socket_internal(fd, type, listening))<= 0)
> +                return r;
> +
> +        if (family>  0) {
> +                union sockaddr_union sockaddr;
> +                socklen_t l;
> +
> +                memset(&sockaddr, 0, sizeof(sockaddr));
> +                l = sizeof(sockaddr);
> +
> +                if (getsockname(fd,&sockaddr.sa,&l)<  0)
> +                        return -errno;
> +
> +                if (l<  sizeof(sa_family_t))
> +                        return -EINVAL;
> +
> +                return sockaddr.sa.sa_family == family;
> +        }
> +
> +        return 1;
> +}
> +
> +int sd_is_socket_inet(int fd, int family, int type, int listening, uint16_t port) {
> +        union sockaddr_union sockaddr;
> +        socklen_t l;
> +        int r;
> +
> +        if (family != 0&&  family != AF_INET&&  family != AF_INET6)
> +                return -EINVAL;
> +
> +        if ((r = sd_is_socket_internal(fd, type, listening))<= 0)
> +                return r;
> +
> +        memset(&sockaddr, 0, sizeof(sockaddr));
> +        l = sizeof(sockaddr);
> +
> +        if (getsockname(fd,&sockaddr.sa,&l)<  0)
> +                return -errno;
> +
> +        if (l<  sizeof(sa_family_t))
> +                return -EINVAL;
> +
> +        if (sockaddr.sa.sa_family != AF_INET&&
> +            sockaddr.sa.sa_family != AF_INET6)
> +                return 0;
> +
> +        if (family>  0)
> +                if (sockaddr.sa.sa_family != family)
> +                        return 0;
> +
> +        if (port>  0) {
> +                if (sockaddr.sa.sa_family == AF_INET) {
> +                        if (l<  sizeof(struct sockaddr_in))
> +                                return -EINVAL;
> +
> +                        return htons(port) == sockaddr.in4.sin_port;
> +                } else {
> +                        if (l<  sizeof(struct sockaddr_in6))
> +                                return -EINVAL;
> +
> +                        return htons(port) == sockaddr.in6.sin6_port;
> +                }
> +        }
> +
> +        return 1;
> +}
> +
> +int sd_is_socket_unix(int fd, int type, int listening, const char *path, size_t length) {
> +        union sockaddr_union sockaddr;
> +        socklen_t l;
> +        int r;
> +
> +        if ((r = sd_is_socket_internal(fd, type, listening))<= 0)
> +                return r;
> +
> +        memset(&sockaddr, 0, sizeof(sockaddr));
> +        l = sizeof(sockaddr);
> +
> +        if (getsockname(fd,&sockaddr.sa,&l)<  0)
> +                return -errno;
> +
> +        if (l<  sizeof(sa_family_t))
> +                return -EINVAL;
> +
> +        if (sockaddr.sa.sa_family != AF_UNIX)
> +                return 0;
> +
> +        if (path) {
> +                if (length<= 0)
> +                        length = strlen(path);
> +
> +                if (length<= 0)
> +                        /* Unnamed socket */
> +                        return l == sizeof(sa_family_t);
> +
> +                if (path[0])
> +                        /* Normal path socket */
> +                        return
> +                                (l>= sizeof(sa_family_t) + length + 1)&&
> +                                memcmp(path, sockaddr.un.sun_path, length+1) == 0;
> +                else
> +                        /* Abstract namespace socket */
> +                        return
> +                                (l == sizeof(sa_family_t) + length)&&
> +                                memcmp(path, sockaddr.un.sun_path, length) == 0;
> +        }
> +
> +        return 1;
> +}
> +
> +int sd_notify(int unset_environment, const char *state) {
> +#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
> +        return 0;
> +#else
> +        int fd = -1, r;
> +        struct msghdr msghdr;
> +        struct iovec iovec;
> +        union sockaddr_union sockaddr;
> +        struct ucred *ucred;
> +        union {
> +                struct cmsghdr cmsghdr;
> +                uint8_t buf[CMSG_SPACE(sizeof(struct ucred))];
> +        } control;
> +        const char *e;
> +
> +        if (!state) {
> +                r = -EINVAL;
> +                goto finish;
> +        }
> +
> +        if (!(e = getenv("NOTIFY_SOCKET")))
> +                return 0;
> +
> +        /* Must be an abstract socket, or an absolute path */
> +        if ((e[0] != '@'&&  e[0] != '/') || e[1] == 0) {
> +                r = -EINVAL;
> +                goto finish;
> +        }
> +
> +        if ((fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0))<  0) {
> +                r = -errno;
> +                goto finish;
> +        }
> +
> +        memset(&sockaddr, 0, sizeof(sockaddr));
> +        sockaddr.sa.sa_family = AF_UNIX;
> +        strncpy(sockaddr.un.sun_path, e, sizeof(sockaddr.un.sun_path));
> +
> +        if (sockaddr.un.sun_path[0] == '@')
> +                sockaddr.un.sun_path[0] = 0;
> +
> +        memset(&iovec, 0, sizeof(iovec));
> +        iovec.iov_base = (char*) state;
> +        iovec.iov_len = strlen(state);
> +
> +        memset(&control, 0, sizeof(control));
> +        control.cmsghdr.cmsg_level = SOL_SOCKET;
> +        control.cmsghdr.cmsg_type = SCM_CREDENTIALS;
> +        control.cmsghdr.cmsg_len = CMSG_LEN(sizeof(struct ucred));
> +
> +        ucred = (struct ucred*) CMSG_DATA(&control.cmsghdr);
> +        ucred->pid = getpid();
> +        ucred->uid = getuid();
> +        ucred->gid = getgid();
> +
> +        memset(&msghdr, 0, sizeof(msghdr));
> +        msghdr.msg_name =&sockaddr;
> +        msghdr.msg_namelen = sizeof(sa_family_t) + strlen(e);
> +
> +        if (msghdr.msg_namelen>  sizeof(struct sockaddr_un))
> +                msghdr.msg_namelen = sizeof(struct sockaddr_un);
> +
> +        msghdr.msg_iov =&iovec;
> +        msghdr.msg_iovlen = 1;
> +        msghdr.msg_control =&control;
> +        msghdr.msg_controllen = control.cmsghdr.cmsg_len;
> +
> +        if (sendmsg(fd,&msghdr, MSG_NOSIGNAL)<  0) {
> +                r = -errno;
> +                goto finish;
> +        }
> +
> +        r = 1;
> +
> +finish:
> +        if (unset_environment)
> +                unsetenv("NOTIFY_SOCKET");
> +
> +        if (fd>= 0)
> +                close(fd);
> +
> +        return r;
> +#endif
> +}
> +
> +int sd_notifyf(int unset_environment, const char *format, ...) {
> +#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
> +        return 0;
> +#else
> +        va_list ap;
> +        char *p = NULL;
> +        int r;
> +
> +        va_start(ap, format);
> +        r = vasprintf(&p, format, ap);
> +        va_end(ap);
> +
> +        if (r<  0 || !p)
> +                return -ENOMEM;
> +
> +        r = sd_notify(unset_environment, p);
> +        free(p);
> +
> +        return r;
> +#endif
> +}
> +
> +int sd_booted(void) {
> +#if defined(DISABLE_SYSTEMD) || !defined(__linux__)
> +        return 0;
> +#else
> +
> +        struct stat a, b;
> +
> +        /* We simply test whether the systemd cgroup hierarchy is
> +         * mounted */
> +
> +        if (lstat("/cgroup",&a)<  0)
> +                return 0;
> +
> +        if (lstat("/cgroup/systemd",&b)<  0)
> +                return 0;
> +
> +        return a.st_dev != b.st_dev;
> +#endif
> +}
> diff --git a/src/sd-daemon.h b/src/sd-daemon.h
> new file mode 100644
> index 0000000..fd6221f
> --- /dev/null
> +++ b/src/sd-daemon.h
> @@ -0,0 +1,257 @@
> +/*-*- Mode: C; c-basic-offset: 8 -*-*/
> +
> +#ifndef foosddaemonhfoo
> +#define foosddaemonhfoo
> +
> +/***
> +  Copyright 2010 Lennart Poettering
> +
> +  Permission is hereby granted, free of charge, to any person
> +  obtaining a copy of this software and associated documentation files
> +  (the "Software"), to deal in the Software without restriction,
> +  including without limitation the rights to use, copy, modify, merge,
> +  publish, distribute, sublicense, and/or sell copies of the Software,
> +  and to permit persons to whom the Software is furnished to do so,
> +  subject to the following conditions:
> +
> +  The above copyright notice and this permission notice shall be
> +  included in all copies or substantial portions of the Software.
> +
> +  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> +  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> +  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> +  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +  SOFTWARE.
> +***/
> +
> +#include<sys/types.h>
> +#include<inttypes.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/*
> +  Reference implementation of a few systemd related interfaces for
> +  writing daemons. These interfaces are trivial to implement. To
> +  simplify porting we provide this reference implementation.
> +  Applications are welcome to reimplement the algorithms described
> +  here if they do not want to include these two source files.
> +
> +  The following functionality is provided:
> +
> +  - Support for logging with log levels on stderr
> +  - File descriptor passing for socket-based activation
> +  - Daemon startup and status notification
> +  - Detection of systemd boots
> +
> +  You may compile this with -DDISABLE_SYSTEMD to disable systemd
> +  support. This makes all those calls NOPs that are directly related to
> +  systemd (i.e. only sd_is_xxx() will stay useful).
> +
> +  Since this is drop-in code we don't want any of our symbols to be
> +  exported in any case. Hence we declare hidden visibility for all of
> +  them.
> +
> +  You may find an up-to-date version of these source files online:
> +
> +  http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.h
> +  http://cgit.freedesktop.org/systemd/plain/src/sd-daemon.c
> +
> +  This should compile on non-Linux systems, too, but with the
> +  exception of the sd_is_xxx() calls all functions will become NOPs.
> +
> +  See sd-daemon(7) for more information.
> +*/
> +
> +#if __GNUC__>= 4
> +#define _sd_printf_attr_(a,b) __attribute__ ((format (printf, a, b)))
> +#define _sd_hidden_ __attribute__ ((visibility("hidden")))
> +#else
> +#define _sd_printf_attr_(a,b)
> +#define _sd_hidden_
> +#endif
> +
> +/*
> +  Log levels for usage on stderr:
> +
> +          fprintf(stderr, SD_NOTICE "Hello World!\n");
> +
> +  This is similar to printk() usage in the kernel.
> +*/
> +#define SD_EMERG   "<0>"  /* system is unusable */
> +#define SD_ALERT   "<1>"  /* action must be taken immediately */
> +#define SD_CRIT    "<2>"  /* critical conditions */
> +#define SD_ERR     "<3>"  /* error conditions */
> +#define SD_WARNING "<4>"  /* warning conditions */
> +#define SD_NOTICE  "<5>"  /* normal but significant condition */
> +#define SD_INFO    "<6>"  /* informational */
> +#define SD_DEBUG   "<7>"  /* debug-level messages */
> +
> +/* The first passed file descriptor is fd 3 */
> +#define SD_LISTEN_FDS_START 3
> +
> +/*
> +  Returns how many file descriptors have been passed, or a negative
> +  errno code on failure. Optionally, removes the $LISTEN_FDS and
> +  $LISTEN_PID file descriptors from the environment (recommended, but
> +  problematic in threaded environments). If r is the return value of
> +  this function you'll find the file descriptors passed as fds
> +  SD_LISTEN_FDS_START to SD_LISTEN_FDS_START+r-1. Returns a negative
> +  errno style error code on failure. This function call ensures that
> +  the FD_CLOEXEC flag is set for the passed file descriptors, to make
> +  sure they are not passed on to child processes. If FD_CLOEXEC shall
> +  not be set, the caller needs to unset it after this call for all file
> +  descriptors that are used.
> +
> +  See sd_listen_fds(3) for more information.
> +*/
> +int sd_listen_fds(int unset_environment) _sd_hidden_;
> +
> +/*
> +  Helper call for identifying a passed file descriptor. Returns 1 if
> +  the file descriptor is a FIFO in the file system stored under the
> +  specified path, 0 otherwise. If path is NULL a path name check will
> +  not be done and the call only verifies if the file descriptor
> +  refers to a FIFO. Returns a negative errno style error code on
> +  failure.
> +
> +  See sd_is_fifo(3) for more information.
> +*/
> +int sd_is_fifo(int fd, const char *path) _sd_hidden_;
> +
> +/*
> +  Helper call for identifying a passed file descriptor. Returns 1 if
> +  the file descriptor is a socket of the specified family (AF_INET,
> +  ...) and type (SOCK_DGRAM, SOCK_STREAM, ...), 0 otherwise. If
> +  family is 0 a socket family check will not be done. If type is 0 a
> +  socket type check will not be done and the call only verifies if
> +  the file descriptor refers to a socket. If listening is>  0 it is
> +  verified that the socket is in listening mode. (i.e. listen() has
> +  been called) If listening is == 0 it is verified that the socket is
> +  not in listening mode. If listening is<  0 no listening mode check
> +  is done. Returns a negative errno style error code on failure.
> +
> +  See sd_is_socket(3) for more information.
> +*/
> +int sd_is_socket(int fd, int family, int type, int listening) _sd_hidden_;
> +
> +/*
> +  Helper call for identifying a passed file descriptor. Returns 1 if
> +  the file descriptor is an Internet socket, of the specified family
> +  (either AF_INET or AF_INET6) and the specified type (SOCK_DGRAM,
> +  SOCK_STREAM, ...), 0 otherwise. If version is 0 a protocol version
> +  check is not done. If type is 0 a socket type check will not be
> +  done. If port is 0 a socket port check will not be done. The
> +  listening flag is used the same way as in sd_is_socket(). Returns a
> +  negative errno style error code on failure.
> +
> +  See sd_is_socket_inet(3) for more information.
> +*/
> +int sd_is_socket_inet(int fd, int family, int type, int listening, uint16_t port) _sd_hidden_;
> +
> +/*
> +  Helper call for identifying a passed file descriptor. Returns 1 if
> +  the file descriptor is an AF_UNIX socket of the specified type
> +  (SOCK_DGRAM, SOCK_STREAM, ...) and path, 0 otherwise. If type is 0
> +  a socket type check will not be done. If path is NULL a socket path
> +  check will not be done. For normal AF_UNIX sockets set length to
> +  0. For abstract namespace sockets set length to the length of the
> +  socket name (including the initial 0 byte), and pass the full
> +  socket path in path (including the initial 0 byte). The listening
> +  flag is used the same way as in sd_is_socket(). Returns a negative
> +  errno style error code on failure.
> +
> +  See sd_is_socket_unix(3) for more information.
> +*/
> +int sd_is_socket_unix(int fd, int type, int listening, const char *path, size_t length) _sd_hidden_;
> +
> +/*
> +  Informs systemd about changed daemon state. This takes a number of
> +  newline seperated environment-style variable assignments in a
> +  string. The following variables are known:
> +
> +     READY=1      Tells systemd that daemon startup is finished (only
> +                  relevant for services of Type=notify). The passed
> +                  argument is a boolean "1" or "0". Since there is
> +                  little value in signalling non-readiness the only
> +                  value daemons should send is "READY=1".
> +
> +     STATUS=...   Passes a single-line status string back to systemd
> +                  that describes the daemon state. This is free-from
> +                  and can be used for various purposes: general state
> +                  feedback, fsck-like programs could pass completion
> +                  percentages and failing programs could pass a human
> +                  readable error message. Example: "STATUS=Completed
> +                  66% of file system check..."
> +
> +     ERRNO=...    If a daemon fails, the errno-style error code,
> +                  formatted as string. Example: "ERRNO=2" for ENOENT.
> +
> +     BUSERROR=... If a daemon fails, the D-Bus error-style error
> +                  code. Example: "BUSERROR=org.freedesktop.DBus.Error.TimedOut"
> +
> +     MAINPID=...  The main pid of a daemon, in case systemd did not
> +                  fork off the process itself. Example: "MAINPID=4711"
> +
> +  Daemons can choose to send additional variables. However, it is
> +  recommened to prefix variable names not listed above with X_.
> +
> +  Returns a negative errno-style error code on failure. Returns>  0
> +  if systemd could be notified, 0 if it couldn't possibly because
> +  systemd is not running.
> +
> +  Example: When a daemon finished starting up, it could issue this
> +  call to notify systemd about it:
> +
> +     sd_notify(0, "READY=1");
> +
> +  See sd_notifyf() for more complete examples.
> +
> +  See sd_notify(3) for more information.
> +*/
> +int sd_notify(int unset_environment, const char *state) _sd_hidden_;
> +
> +/*
> +  Similar to sd_notify() but takes a format string.
> +
> +  Example 1: A daemon could send the following after initialization:
> +
> +     sd_notifyf(0, "READY=1\n"
> +                   "STATUS=Processing requests...\n"
> +                   "MAINPID=%lu",
> +                   (unsigned long) getpid());
> +
> +  Example 2: A daemon could send the following shortly before
> +  exiting, on failure:
> +
> +     sd_notifyf(0, "STATUS=Failed to start up: %s\n"
> +                   "ERRNO=%i",
> +                   strerror(errno),
> +                   errno);
> +
> +  See sd_notifyf(3) for more information.
> +*/
> +int sd_notifyf(int unset_environment, const char *format, ...) _sd_printf_attr_(2,3) _sd_hidden_;
> +
> +/*
> +  Returns>  0 if the system was booted with systemd. Returns<  0 on
> +  error. Returns 0 if the system was not booted with systemd. Note
> +  that all of the functions above handle non-systemd boots just
> +  fine. You should NOT protect them with a call to this function. Also
> +  note that this function checks whether the system, not the user
> +  session is controlled by systemd. However the functions above work
> +  for both session and system services.
> +
> +  See sd_booted(3) for more information.
> +*/
> +int sd_booted(void) _sd_hidden_;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif

Re: [PATCH] rpcbind: add support for systemd socket activation

[Lennart Poettering]

On Wed, 21.07.10 13:56, Chuck Lever (chuck.lever@oracle.com) wrote:

> Why aren't sd-daemon.[ch] part of a systemd related library?  They
> seem pretty generic to me, and could be useful for other daemons.

Well, these two files are simply a reference implementation. The
algorithms are as trivial as they can get. For now we decided to simply
ask people to include the ref impl in their sources. Either the .c/.h
file or just the function needed. This is usually easier to get accepted
politically since this will then not add a new dependency to the
packages. Maybe one day when the number of APIs we provide are
substantial enough and the algorithms not trivial anymore we will add a
real library for this.

But for now, if people just add this tiny bit of code to their sources
this will be a feature that finds it way silently into the various
packages without requiring any new dependencies, and people will hence
have little reason to disable it.

> Plus, the newly introduced update-systemd Makefile target seems to
> suggest these won't be stable over time.  Wouldn't it be better to
> have a versioned library, external to rpcbind, with this code in it?

Well, the functions are trivial. I see little need to ever update them
unless sombody really wants new functionality.

Or to put this all in other words: the ABI/API that matters for the
socket activation are the execution semantics, and the two env vars
LISTEN_FDS and LISTEN_PID, it is not the C implementation we offer
people -- which does little more than parse those two env vars.

Or to put it in even other words: package maintainers are easily
convinced of merging support for this if we don't ask them to add an
additional dependency (which then would have to be ifdeffed) just for
the sake of parsing two env vars!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] rpcbind: add support for systemd socket activation

[Chuck Lever]

Hi Lennart-

On 07/21/10 05:56 PM, Lennart Poettering wrote:
> On Wed, 21.07.10 13:56, Chuck Lever (chuck.lever@oracle.com) wrote:
>
>> Why aren't sd-daemon.[ch] part of a systemd related library?  They
>> seem pretty generic to me, and could be useful for other daemons.
>
> Well, these two files are simply a reference implementation. The
> algorithms are as trivial as they can get. For now we decided to simply
> ask people to include the ref impl in their sources. Either the .c/.h
> file or just the function needed. This is usually easier to get accepted
> politically since this will then not add a new dependency to the
> packages. Maybe one day when the number of APIs we provide are
> substantial enough and the algorithms not trivial anymore we will add a
> real library for this.
>
> But for now, if people just add this tiny bit of code to their sources
> this will be a feature that finds it way silently into the various
> packages without requiring any new dependencies, and people will hence
> have little reason to disable it.

The complexity or size of the systemd code is not the issue, it is how 
it will be maintained going forward.  Especially since these are all 
system daemons, you know that there will be security bug fixes.  The 
current situation means every daemon will have to be rebuilt and 
redistributed to fix a bug in the systemd code.  It also creates inertia 
to fixing problems, thus inviting the individual snippets in each daemon 
to become hacked up and out of date.

The least you could do is provide both the raw source files and a 
library implementation, and let daemon maintainers choose which they 
prefer.  The best solution would provide a clean versioned C language 
API via a shared library

Do you have some picture of systemd adoption outside of Fedora?

Re: [PATCH] rpcbind: add support for systemd socket activation

[Lennart Poettering]

On Fri, 23.07.10 11:13, Chuck Lever (chuck.lever@oracle.com) wrote:

> >But for now, if people just add this tiny bit of code to their sources
> >this will be a feature that finds it way silently into the various
> >packages without requiring any new dependencies, and people will hence
> >have little reason to disable it.
> 
> The complexity or size of the systemd code is not the issue, it is
> how it will be maintained going forward.  Especially since these are
> all system daemons, you know that there will be security bug fixes.

In code that simply parses two environment variables with strtol(), how
likely is it to encounter a security issue in this?

> The least you could do is provide both the raw source files and a
> library implementation, and let daemon maintainers choose which they
> prefer.  The best solution would provide a clean versioned C
> language API via a shared library

Hmm, providing both is indeed an idea. However, we currently hide the
symbols sd-daemon.[ch] exports when linking for good reasons. Now, if we
supported both an .so and the drop-in code, then we'd have the same
symbols once exported and once not. That's a weird kind of namespace
clash...

But anyway, we'll think about this and most likely add this sooner or
later. However, if possible I'd very muhc prefer to use the drop-in code
for now, if that's ok? I promise I'll prepare the patch that moves
rpcbind to the the .so API as soon as we provide that ;-).

> Do you have some picture of systemd adoption outside of Fedora?

All bigger distributions have it (with one exception, see below). Some
smaller distros made it the default already. Some folks at Suse are
working on making it the default in their next release. Meego has folks
working on something like this too.

So basically, everybody who could adopt it has it at least packaged or
is already on the path to making it the default -- with one exception:
Ubuntu. The Upstart developer works for Canonical, so go figure. He was
vaguely interested in adopting socket-based activation in Upstart too,
but so far nothing has shown up. We have discussed with him whether he
then would be willing to adopt the same interface systemd uses for
socket activation. This petered out without result, but given that the
interface we came up with is really trivial and we tried our best to
keep this independent of systemd and already supported in quite a number
of software our hope is that eventually he'll adopt this scheme too.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.