[CRIU] [PATCH] systemd socket activation support

[shawn at churchofgit.com]

---- Original Message ----
 From: "Pavel Emelyanov" 
 To: "Shawn Landden" 
CC: criu at openvz.org
 Sent: Tue, Dec 10, 2013, 08:56 PM
 Subject: Re: [CRIU] [PATCH] systemd socket activation support
On 12/11/2013 07:34 AM, Shawn Landden wrote:
Makes the criu RPC socket socket-activated with
systemd [1], meaning that systemd will create and listen to
the UNIX socket /var/run/criu-srvice.socket
on behalf of criu until a connection comes in, when it will
then pass control of the socket, along with the first connection
over to a newly spawned criu daemon.

This is similar to inetd, but criu stays around after getting
started, listening itsself on the socket.

Shawn, thank you for the contribution, it's really appreciated.
I have several comments inline.

[1] http://0pointer.de/blog/projects/socket-activation.html (http://0pointer.de/blog/projects/socket-activation.html)

Signed-off-by: Shawn Landden 
---
Makefile         |   3 +
Makefile.crtools |   1 +
Makefile.inc     |   1 +
cr-service.c     |  73 ++++----
criu.service     |   5 +
criu.socket      |   8 +
sd-daemon.c      | 521 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
sd-daemon.h      | 282 ++++++++++++++++++++++++++++++
8 files changed, 862 insertions(+), 32 deletions(-)
create mode 100644 criu.service
create mode 100644 criu.socket
create mode 100644 sd-daemon.c
create mode 100644 sd-daemon.h

diff --git a/cr-service.c b/cr-service.c
index a82ef9e..3b9cb02 100644
--- a/cr-service.c
+++ b/cr-service.c
@@ -332,53 +333,61 @@ static int restore_sigchld_handler()

int cr_service(bool daemon_mode)
{
-   int server_fd = -1;
+   int server_fd = -1, n;
   int child_pid;

-   struct sockaddr_un server_addr;
   struct sockaddr_un client_addr;
-
-   socklen_t server_addr_len;
   socklen_t client_addr_len;

-   server_fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);
-   if (server_fd == -1) {
-      pr_perror("Can't initialize service socket.");
+   n = sd_listen_fds(0);
+   if (n > 1) {
+      pr_perror("Too many file descriptors (%d) recieved.", n);
      goto err;
-   }
+   } else if (n == 1)
+      server_fd = SD_LISTEN_FDS_START + 0;
+   else {

When sd_listen_fds reports negative value, which means some error has
occurred, we get on this branch and start listening on our own socket.
Is that expected behavior?

Yes 
This is only triggered if the passed environment variables are not numbers.
If we are being socket activated we will immediately fail afterwards as the
socket path will be taken.
+      struct sockaddr_un server_addr;
+      socklen_t server_addr_len;
+
+      server_fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);
+      if (server_fd == -1) {
+         pr_perror("Can't initialize service socket.");
+         goto err;
+      }

diff --git a/criu.service b/criu.service
new file mode 100644
index 0000000..803ce46
--- /dev/null
+++ b/criu.service
@@ -0,0 +1,5 @@
+[Unit]
+Description=Checkpoint Restore in Userspace daemon
+
+[Service]
+ExecStart=/usr/sbin/criu service
diff --git a/criu.socket b/criu.socket
new file mode 100644
index 0000000..7ccf7b4
--- /dev/null
+++ b/criu.socket
@@ -0,0 +1,8 @@
+[Unit]
+Description=Checkpoint Restore in Userspace socket
+
+[Socket]
+ListenSequentialPacket=/var/run/criu-service.socket
+
+[Install]
+WantedBy=sockets.target

Can you put these two files into e.g. scripts/sd/ subdirectory?

done 
diff --git a/sd-daemon.c b/sd-daemon.c
new file mode 100644
index 0000000..2bf506e
--- /dev/null
+++ b/sd-daemon.c
@@ -0,0 +1,521 @@

I've noticed that quite a lot of code below (and in sd_daemon.h file) is
unused. Effectively only the sd_listen_fds() is called. Is there a reason 
for keeping the rest? Can you drop unused parts from the patch otherwise?

done
Thanks,
Pavel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20131211/bb9e4d10/attachment.html>