<!DOCTYPE html><html dir="ltr"><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8" /><head><body><div><div><br><blockquote>---- Original Message ----<br>
From: "Pavel Emelyanov" <xemul@parallels.com><br>
To: "Shawn Landden" <shawn@churchofgit.com><br>CC: criu@openvz.org<br>
Sent: Tue, Dec 10, 2013, 08:56 PM<br>
Subject: Re: [CRIU] [PATCH] systemd socket activation support<br><br><br>On 12/11/2013 07:34 AM, Shawn Landden wrote:<br><blockquote>Makes the criu RPC socket socket-activated with<br>systemd [1], meaning that systemd will create and listen to<br>the UNIX socket /var/run/criu-srvice.socket<br>on behalf of criu until a connection comes in, when it will<br>then pass control of the socket, along with the first connection<br>over to a newly spawned criu daemon.<br><br>This is similar to inetd, but criu stays around after getting<br>started, listening itsself on the socket.</blockquote><br><br>Shawn, thank you for the contribution, it's really appreciated.<br>I have several comments inline.<br><br><blockquote>[1] <a target="_blank" href="http://0pointer.de/blog/projects/socket-activation.html">http://0pointer.de/blog/projects/socket-activation.html</a><br><br>Signed-off-by: Shawn Landden <<a target="_blank" href="mailto:shawn@churchofgit.com">shawn@churchofgit.com</a>><br>---<br>Makefile | 3 +<br>Makefile.crtools | 1 +<br>Makefile.inc | 1 +<br>cr-service.c | 73 ++++----<br>criu.service | 5 +<br>criu.socket | 8 +<br>sd-daemon.c | 521 +++++++++++++++++++++++++++++++++++++++++++++++++++++++<br>sd-daemon.h | 282 ++++++++++++++++++++++++++++++<br>8 files changed, 862 insertions(+), 32 deletions(-)<br>create mode 100644 criu.service<br>create mode 100644 criu.socket<br>create mode 100644 sd-daemon.c<br>create mode 100644 sd-daemon.h</blockquote><br><br><blockquote>diff --git a/cr-service.c b/cr-service.c<br>index a82ef9e..3b9cb02 100644<br>--- a/cr-service.c<br>+++ b/cr-service.c<br>@@ -332,53 +333,61 @@ static int restore_sigchld_handler()<br><br>int cr_service(bool daemon_mode)<br>{<br>- int server_fd = -1;<br>+ int server_fd = -1, n;<br> int child_pid;<br><br>- struct sockaddr_un server_addr;<br> struct sockaddr_un client_addr;<br>-<br>- socklen_t server_addr_len;<br> socklen_t client_addr_len;<br><br>- server_fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);<br>- if (server_fd == -1) {<br>- pr_perror("Can't initialize service socket.");<br>+ n = sd_listen_fds(0);<br>+ if (n > 1) {<br>+ pr_perror("Too many file descriptors (%d) recieved.", n);<br> goto err;<br>- }<br>+ } else if (n == 1)<br>+ server_fd = SD_LISTEN_FDS_START + 0;<br>+ else {</blockquote><br><br>When sd_listen_fds reports negative value, which means some error has<br>occurred, we get on this branch and start listening on our own socket.<br>Is that expected behavior?<br><br></blockquote><blockquote></blockquote><span>Yes </span><br><blockquote></blockquote><span>This is only triggered if the passed environment variables are not numbers.</span><div>If we are being socket activated we will immediately fail afterwards as the</div><div>socket path will be taken.</div><div><blockquote><blockquote>+ struct sockaddr_un server_addr;<br>+ socklen_t server_addr_len;<br>+<br>+ server_fd = socket(AF_LOCAL, SOCK_SEQPACKET, 0);<br>+ if (server_fd == -1) {<br>+ pr_perror("Can't initialize service socket.");<br>+ goto err;<br>+ }</blockquote><br><br><blockquote>diff --git a/criu.service b/criu.service<br>new file mode 100644<br>index 0000000..803ce46<br>--- /dev/null<br>+++ b/criu.service<br>@@ -0,0 +1,5 @@<br>+[Unit]<br>+Description=Checkpoint Restore in Userspace daemon<br>+<br>+[Service]<br>+ExecStart=/usr/sbin/criu service<br>diff --git a/criu.socket b/criu.socket<br>new file mode 100644<br>index 0000000..7ccf7b4<br>--- /dev/null<br>+++ b/criu.socket<br>@@ -0,0 +1,8 @@<br>+[Unit]<br>+Description=Checkpoint Restore in Userspace socket<br>+<br>+[Socket]<br>+ListenSequentialPacket=/var/run/criu-service.socket<br>+<br>+[Install]<br>+WantedBy=sockets.target</blockquote><br><br>Can you put these two files into e.g. scripts/sd/ subdirectory?<br><br></blockquote><span>done </span><br><blockquote><blockquote>diff --git a/sd-daemon.c b/sd-daemon.c<br>new file mode 100644<br>index 0000000..2bf506e<br>--- /dev/null<br>+++ b/sd-daemon.c<br>@@ -0,0 +1,521 @@</blockquote><br><br>I've noticed that quite a lot of code below (and in sd_daemon.h file) is<br>unused. Effectively only the sd_listen_fds() is called. Is there a reason <br>for keeping the rest? Can you drop unused parts from the patch otherwise?<br><br></blockquote><span>done</span></div><div><blockquote>Thanks,<br>Pavel</blockquote></div></div></div></body></html>