[systemd-devel] [PATCH 1/3] shared: add generic IPC barrier

2014-07-13 Thread David Herrmann
The Barrier object is a simple inter-process barrier implementation. It
allows placing synchronization points and waiting for the other side to
reach it. Additionally, it has an abortion-mechanism as second-layer
synchronization to send abortion-events asynchronously to the other side.

The API is usually used to synchronize processes during fork(). However,
it can be extended to pass state through execve() so you could synchronize
beyond execve().

Usually, it's used like this (error-handling replaced by assert() for
simplicity):

Barrier b;

r = barrier_init(b);
assert_se(r = 0);

pid = fork();
assert_se(pid = 0);
if (pid == 0) {
barrier_set_role(b, BARRIER_CHILD);

...do child post-setup...
if (CHILD_SETUP_FAILED)
   exit(1);
...child setup done...

barrier_place(b);
if (!barrier_sync(b)) {
/* parent setup failed */
exit(1);
}

barrier_destroy(b); /* redundant as execve() and exit() imply this 
*/

/* parent  child setup successful */
execve(...);
}

barrier_set_role(b, BARRIER_PARENT);

...do parent post-setup...
if (PARENT_SETUP_FAILED) {
barrier_abort(b);  /* send abortion event */
barrier_wait_abortion(b);  /* wait for child to abort (exit() 
implies abortion) */
barrier_destroy(b);
   ...bail out...
}
...parent setup done...

barrier_place(b);
if (!barrier_sync(b)) {
...child setup failed... ;
barrier_destroy(b);
...bail out...
}

barrier_destroy(b);

...child setup successfull...

This is the most basic API. Using barrier_place() to place barriers and
barrier_sync() to perform a full synchronization between both processes.
barrier_abort() places an abortion barrier which superceeds any other
barriers, exit() (or barrier_destroy()) places an abortion-barrier that
queues behind existing barriers (thus *not* replacing existing barriers
unlike barrier_abort()).

This example uses hard-synchronization with wait_abortion(), sync() and
friends. These are all optional. Barriers are highly dynamic and can be
used for one-way synchronization or even no synchronization at all
(postponing it for later). The sync() call performs a full two-way
synchronization.

The API is documented and should be fairly self-explanatory. A test-suite
shows some special semantics regarding abortion, wait_next() and exit().

Internally, barriers use two eventfds and a pipe. The pipe is used to
detect exit()s of the remote side as eventfds do not allow that. The
eventfds are used to place barriers, one for each side. Barriers itself
are numbered, but the numbers are reused once both sides reached the same
barrier, thus you cannot address barriers by the index. Moreover, the
numbering is implicit and we only store a counter. This makes the
implementation itself very lightweight, which is probably negligible
considering that we need 3 FDs for a barrier..

Last but not least: This barrier implementation is quite heavy. It's
definitely not meant for fast IPC synchronization. However, it's very easy
to use. And given the *HUGE* overhead of fork(), the barrier-overhead
should be negligible.
---
 .gitignore  |   1 +
 Makefile.am |   9 +
 src/shared/barrier.c| 442 ++
 src/shared/barrier.h|  97 ++
 src/test/test-barrier.c | 460 
 5 files changed, 1009 insertions(+)
 create mode 100644 src/shared/barrier.c
 create mode 100644 src/shared/barrier.h
 create mode 100644 src/test/test-barrier.c

diff --git a/.gitignore b/.gitignore
index 31cd8f8..5289f0e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -123,6 +123,7 @@
 /tags
 /test-architecture
 /test-async
+/test-barrier
 /test-boot-timestamp
 /test-bus-chat
 /test-bus-cleanup
diff --git a/Makefile.am b/Makefile.am
index 2b1484f..039a83e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -828,6 +828,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/login-shared.h \
src/shared/ring.c \
src/shared/ring.h \
+   src/shared/barrier.c \
+   src/shared/barrier.h \
src/shared/async.c \
src/shared/async.h \
src/shared/eventfd-util.c \
@@ -1238,6 +1240,7 @@ tests += \
test-ellipsize \
test-util \
test-ring \
+   test-barrier \
test-tmpfiles \
test-namespace \
test-date \
@@ -1408,6 +1411,12 @@ test_ring_SOURCES = \
 test_ring_LDADD = \
libsystemd-core.la
 
+test_barrier_SOURCES = \
+   src/test/test-barrier.c
+
+test_barrier_LDADD = \
+   libsystemd-core.la
+
 test_tmpfiles_SOURCES = \
src/test/test-tmpfiles.c
 
diff --git a/src/shared/barrier.c b/src/shared/barrier.c
new file mode 100644
index 000..e7b4ead
--- 

[systemd-devel] [PATCH 2/3] nspawn: use Barrier API instead of eventfd-util

2014-07-13 Thread David Herrmann
The Barrier-API simplifies cross-fork() synchronization a lot. Replace the
hard-coded eventfd-util implementation and drop it.

Compared to the old API, Barriers also handle exit() of the remote side as
abortion. This way, segfaults will not cause the parent to deadlock.

EINTR handling is currently ignored for any barrier-waits. This can easily
be added, but it isn't needed so far so I dropped it. EINTR handling in
general is ugly, anyway. You need to deal with pselect/ppoll/... variants
and make sure not to unblock signals at the wrong times. So genrally,
there's little use in adding it.
---
 Makefile.am   |   2 -
 src/nspawn/nspawn.c   | 136 -
 src/shared/eventfd-util.c | 169 --
 src/shared/eventfd-util.h |  43 
 4 files changed, 60 insertions(+), 290 deletions(-)
 delete mode 100644 src/shared/eventfd-util.c
 delete mode 100644 src/shared/eventfd-util.h

diff --git a/Makefile.am b/Makefile.am
index 039a83e..d7b0f90 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -832,8 +832,6 @@ libsystemd_shared_la_SOURCES = \
src/shared/barrier.h \
src/shared/async.c \
src/shared/async.h \
-   src/shared/eventfd-util.c \
-   src/shared/eventfd-util.h \
src/shared/copy.c \
src/shared/copy.h \
src/shared/base-filesystem.c \
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index bad93a5..e75cc28 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -40,7 +40,6 @@
 #include sys/un.h
 #include sys/socket.h
 #include linux/netlink.h
-#include sys/eventfd.h
 #include net/if.h
 #include linux/veth.h
 #include sys/personality.h
@@ -84,12 +83,12 @@
 #include def.h
 #include rtnl-util.h
 #include udev-util.h
-#include eventfd-util.h
 #include blkid-util.h
 #include gpt.h
 #include siphash24.h
 #include copy.h
 #include base-filesystem.h
+#include barrier.h
 
 #ifdef HAVE_SECCOMP
 #include seccomp-util.h
@@ -3074,12 +3073,18 @@ int main(int argc, char *argv[]) {
 
 for (;;) {
 ContainerStatus container_status;
-int eventfds[2] = { -1, -1 };
+_barrier_destroy_ Barrier barrier = { };
 struct sigaction sa = {
 .sa_handler = nop_handler,
 .sa_flags = SA_NOCLDSTOP,
 };
 
+r = barrier_init(barrier);
+if (r  0) {
+log_error(Cannot initialize IPC barrier: %s, 
strerror(-r));
+goto finish;
+}
+
 /* Child can be killed before execv(), so handle SIGCHLD
  * in order to interrupt parent's blocking calls and
  * give it a chance to call wait() and terminate. */
@@ -3095,9 +3100,9 @@ int main(int argc, char *argv[]) {
 goto finish;
 }
 
-pid = clone_with_eventfd(SIGCHLD|CLONE_NEWNS|
- (arg_share_system ? 0 : 
CLONE_NEWIPC|CLONE_NEWPID|CLONE_NEWUTS)|
- (arg_private_network ? CLONE_NEWNET : 
0), eventfds);
+pid = syscall(__NR_clone, SIGCHLD|CLONE_NEWNS|
+  (arg_share_system ? 0 : 
CLONE_NEWIPC|CLONE_NEWPID|CLONE_NEWUTS)|
+  (arg_private_network ? CLONE_NEWNET 
: 0), NULL);
 if (pid  0) {
 if (errno == EINVAL)
 log_error(clone() failed, do you have 
namespace support enabled in your kernel? (You need UTS, IPC, PID and NET 
namespacing built in): %m);
@@ -3126,6 +3131,8 @@ int main(int argc, char *argv[]) {
 };
 char **env_use;
 
+barrier_set_role(barrier, BARRIER_CHILD);
+
 envp[n_env] = strv_find_prefix(environ, TERM=);
 if (envp[n_env])
 n_env ++;
@@ -3151,26 +3158,26 @@ int main(int argc, char *argv[]) {
 }
 
 log_error(Failed to open console: %s, 
strerror(-k));
-goto child_fail;
+_exit(EXIT_FAILURE);
 }
 
 if (dup2(STDIN_FILENO, STDOUT_FILENO) != STDOUT_FILENO 
||
 dup2(STDIN_FILENO, STDERR_FILENO) != 
STDERR_FILENO) {
 log_error(Failed to duplicate console: %m);
-goto child_fail;
+_exit(EXIT_FAILURE);
 }
 
 if (setsid()  0) {
 log_error(setsid() failed: %m);
-goto child_fail;
+_exit(EXIT_FAILURE);

[systemd-devel] [PATCH 0/3] Cross fork() synchronization via Barrier API

2014-07-13 Thread David Herrmann
Hi

While working on a PTY helper I needed a synchronized fork() where parent and
child can both perform their setup before either proceeds.
src/shared/eventfd-util.c already provides a coarse API for that, but lacks many
features like implicit exit() handling. Therefore, I went ahead and wrote
barrier.[ch] based on eventfd-util.

Patch #1 adds the Barrier API with extensive tests, #2 converts nspawn to use
it and #3 adds a PTY helper as another example.

As I'm one of these old guys (cough) who never used containers, testing is
welcome! But I'll try to setup nspawn as soon as possible.
Code is also available at fdo in my @ui branch:
  http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=ui

Comments welcome!
David

David Herrmann (3):
  shared: add generic IPC barrier
  nspawn: use Barrier API instead of eventfd-util
  shared: add PTY helper

 .gitignore|   2 +
 Makefile.am   |  20 +-
 src/nspawn/nspawn.c   | 136 +---
 src/shared/barrier.c  | 442 +
 src/shared/barrier.h  |  97 +
 src/shared/eventfd-util.c | 169 ---
 src/shared/eventfd-util.h |  43 
 src/shared/pty.c  | 542 ++
 src/shared/pty.h  |  61 ++
 src/test/test-barrier.c   | 460 +++
 src/test/test-pty.c   | 143 
 11 files changed, 1825 insertions(+), 290 deletions(-)
 create mode 100644 src/shared/barrier.c
 create mode 100644 src/shared/barrier.h
 delete mode 100644 src/shared/eventfd-util.c
 delete mode 100644 src/shared/eventfd-util.h
 create mode 100644 src/shared/pty.c
 create mode 100644 src/shared/pty.h
 create mode 100644 src/test/test-barrier.c
 create mode 100644 src/test/test-pty.c

-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 3/3] shared: add PTY helper

2014-07-13 Thread David Herrmann
This Pty API wraps the ugliness that is POSIX PTY. It takes care of:
  - edge-triggered HUP handling (avoid heavy CPU-usage on vhangup)
  - HUP vs. input-queue draining (handle HUP _after_ draining the whole
input queue)
  - SIGCHLD vs. HUP (HUP is no reliable way to catch PTY deaths, always
use SIGCHLD. Otherwise, vhangup() and friends will break.)
  - Output queue buffering (async EPOLLOUT handling)
  - synchronous setup (via Barrier API)

At the same time, the PTY API does not execve(). It simply fork()s and
leaves everything else to the caller. Usually, they execve() but we
support other setups, too.

This will be needed by multiple UI binaries (systemd-console, systemd-er,
...) so it's placed in src/shared/. It's not strictly related to
libsystemd-ui, so it's not included there.
---
 .gitignore  |   1 +
 Makefile.am |   9 +
 src/shared/pty.c| 542 
 src/shared/pty.h|  61 ++
 src/test/test-pty.c | 143 ++
 5 files changed, 756 insertions(+)
 create mode 100644 src/shared/pty.c
 create mode 100644 src/shared/pty.h
 create mode 100644 src/test/test-pty.c

diff --git a/.gitignore b/.gitignore
index 5289f0e..d942691 100644
--- a/.gitignore
+++ b/.gitignore
@@ -201,6 +201,7 @@
 /test-path-util
 /test-prioq
 /test-ratelimit
+/test-pty
 /test-replace-var
 /test-resolve
 /test-ring
diff --git a/Makefile.am b/Makefile.am
index d7b0f90..2344087 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -830,6 +830,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/ring.h \
src/shared/barrier.c \
src/shared/barrier.h \
+   src/shared/pty.c \
+   src/shared/pty.h \
src/shared/async.c \
src/shared/async.h \
src/shared/copy.c \
@@ -1239,6 +1241,7 @@ tests += \
test-util \
test-ring \
test-barrier \
+   test-pty \
test-tmpfiles \
test-namespace \
test-date \
@@ -1415,6 +1418,12 @@ test_barrier_SOURCES = \
 test_barrier_LDADD = \
libsystemd-core.la
 
+test_pty_SOURCES = \
+   src/test/test-pty.c
+
+test_pty_LDADD = \
+   libsystemd-core.la
+
 test_tmpfiles_SOURCES = \
src/test/test-tmpfiles.c
 
diff --git a/src/shared/pty.c b/src/shared/pty.c
new file mode 100644
index 000..c82c277
--- /dev/null
+++ b/src/shared/pty.c
@@ -0,0 +1,542 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+/*
+ * PTY
+ * A PTY object represents a single PTY connection between a master and a
+ * child. The child process is fork()ed so the caller controls what program
+ * will be run.
+ *
+ * Programs like /bin/login tend to perform a vhangup() on their TTY
+ * before running the login procedure. This also causes the pty master
+ * to get a EPOLLHUP event as long as no client has the TTY opened.
+ * This means, we cannot use the TTY connection as reliable way to track
+ * the client. Instead, we _must_ rely on the PID of the client to track
+ * them.
+ * However, this has the side effect that if the client forks and the
+ * parent exits, we loose them and restart the client. But this seems to
+ * be the expected behavior so we implement it here.
+ *
+ * Unfortunately, epoll always polls for EPOLLHUP so as long as the
+ * vhangup() is ongoing, we will _always_ get EPOLLHUP and cannot sleep.
+ * This gets worse if the client closes the TTY but doesn't exit.
+ * Therefore, the fd must be edge-triggered in the epoll-set so we
+ * only get the events once they change.
+ */
+
+#include errno.h
+#include fcntl.h
+#include limits.h
+#include pty.h
+#include signal.h
+#include stdbool.h
+#include stdint.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include sys/epoll.h
+#include sys/eventfd.h
+#include sys/ioctl.h
+#include sys/types.h
+#include sys/uio.h
+#include sys/wait.h
+#include termios.h
+#include unistd.h
+
+#include barrier.h
+#include macro.h
+#include pty.h
+#include ring.h
+#include util.h
+
+#define PTY_BUFSIZE 16384
+
+struct Pty {
+unsigned long ref;
+Barrier barrier;
+int fd;
+pid_t child;
+sd_event_source *fd_source;
+sd_event_source *child_source;
+
+char in_buf[PTY_BUFSIZE];
+Ring 

[systemd-devel] [PATCH] fileio: quote more shell characters in envfiles

2014-07-13 Thread Mantas Mikulėnas
Turns out, making strings shell-proof is harder than expected:

# machinectl set-hostname foo|poweroff  . /etc/machine-info

(This could be simplified by quoting *and* escaping all characters,
which is harmless in shell but unnecessary.)
---
 src/shared/fileio.c | 4 ++--
 src/shared/util.h   | 6 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index b0ab780..cbb40c2 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -738,11 +738,11 @@ static void write_env_var(FILE *f, const char *v) {
 p++;
 fwrite(v, 1, p-v, f);
 
-if (string_has_cc(p, NULL) || chars_intersect(p, WHITESPACE 
\'\\\`$)) {
+if (string_has_cc(p, NULL) || chars_intersect(p, WHITESPACE 
SHELL_NEED_QUOTES)) {
 fputc('\', f);
 
 for (; *p; p++) {
-if (strchr(\'\\\`$, *p))
+if (strchr(SHELL_NEED_ESCAPE, *p))
 fputc('\\', f);
 
 fputc(*p, f);
diff --git a/src/shared/util.h b/src/shared/util.h
index c5eadc9..b3187a9 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -93,6 +93,12 @@
 #define COMMENTS   #;
 #define GLOB_CHARS *?[
 
+/* What characters are special in the shell? */
+/* must be escaped outside and inside double-quotes */
+#define SHELL_NEED_ESCAPE \\\`$
+/* can be escaped or double-quoted */
+#define SHELL_NEED_QUOTES SHELL_NEED_ESCAPE GLOB_CHARS '()|;
+
 #define FORMAT_BYTES_MAX 8
 
 #define ANSI_HIGHLIGHT_ON \x1B[1;39m
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Setting bonding parameters

2014-07-13 Thread poma

On 26.06.2014 15:30, Lukáš Nykrýn wrote:

poma píše v Pá 20. 06. 2014 v 13:36 +0200:

On 20.06.2014 13:31, Tom Gundersen wrote:

On Thu, Jun 19, 2014 at 1:37 PM, Vladimir Elisseev vo...@vovan.nl wrote:

Simple question: is there a way to set bridge parameters (bridge forward delay, 
bridge hello time, etc) using systemd networking units?


This is still on the TODO. Last I heard Lukas was looking into this,
so maybe we'll get that soon :)

Cheers,

Tom


Ping.
Simple question: was Lukas looking into bonding params also? :)


poma


I have started with bonding params, but it's not looking promising.
Except mode all calls end with something like Could not append
IFLA_BOND_PRIMARY_RESELEC attribute: Operation not supported, even on
latest kernel which is in rawhide.
And setting mode does not show any error, but does not work either.
If anybody wants to look at it, here is my patch for mode
http://pastebin.com/qz1XQ9DA

and here is a earlier version which tried to setup everything.
http://pastebin.com/dzsDQqYF

Lukas





Yep, ain't workin,
http://cgit.freedesktop.org/systemd/systemd/commit/?id=fe8ac65

systemd-215-4.git3864c28.20140711.fc21.x86_64

man 5 systemd.netdev
...
[BOND] SECTION OPTIONS
The [Bond] section accepts the following key:

Mode=
Specifies one of the bonding policies. The default is balance-rr 
(round robin).

balance-rractive-backupbalance-xorbroadcast802.3adbalance-tlbbalance-alb


/etc/systemd/network/bond0mode.netdev
[NetDev]
Name=bond0
Kind=bond

[Bond]
Mode=active-backup


bondctl detail bond0
Bonding Master: bond0
 Oper State: up
 Slaves: enp3s0 enp1s9
 Active Slave:
 Mode:   balance-rr 0
 Monitor:No monitoring enabled


poma


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] SIT tunnel does not work

2014-07-13 Thread Tomasz Torcz
On Sun, Jun 29, 2014 at 01:05:22PM +0200, Tomasz Torcz wrote:
 he.netdev: 
 --
 [NetDev]
 Name=he
 Kind=sit
 
 [Tunnel]
 Local=109.107.25.67
 Remote=216.66.80.162
 
 
 he.network:
 ---
 [Match]
 Name=enp5s0
 
 [Network]
 Tunnel=he
 Address=2001:470:70:68d::2/64
 
 $ SYSTEMD_LOG_LEVEL=debug /lib/systemd/systemd-networkd  
 timestamp of '/etc/systemd/network' changed
  lan: creating netdev
  lan: loaded bridge
   he: loaded sit
 sd-rtnl: discarding 20 bytes of incoming message


  Can anyone share examples of _working_ SIT tunnel configuration?

-- 
Tomasz TorczOnly gods can safely risk perfection,
xmpp: zdzich...@chrome.pl it's a dangerous thing for a man.  -- Alia

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] man: sysusers.d correct default user shell

2014-07-13 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Jul 13, 2014 at 04:56:16PM +0200, Sjoerd Simons wrote:
 For the non-root user sysusers uses nologin as the default shell, not
 login. Correct the documentation to match the code.
 ---
  man/sysusers.d.xml | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
Applied.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] man: mention XDG_DATA_HOME in systemd.unit

2014-07-13 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Jul 12, 2014 at 07:36:16PM +0300, Tanu Kaskinen wrote:
 ---
  man/systemd.unit.xml | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
Applied.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] path-lookup: don't make ~/.local/share/systemd/user a symlink

2014-07-13 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Jul 12, 2014 at 06:59:19PM +0300, Tanu Kaskinen wrote:
 We already encourage upstreams to keep the default configuration
 separate from user customizations for software that is installed in
 the system location. Let's allow that separation also for software
 that is installed in the home directory.
Applied.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/3] nspawn: use Barrier API instead of eventfd-util

2014-07-13 Thread Djalal Harouni
Hi,

Yes, sending abortion events asynchronously is really nice :-)

Just small quick comments for nspawn here, but I'll comment on patch #1

On Sun, Jul 13, 2014 at 12:37:07PM +0200, David Herrmann wrote:
 The Barrier-API simplifies cross-fork() synchronization a lot. Replace the
 hard-coded eventfd-util implementation and drop it.
Ok

 Compared to the old API, Barriers also handle exit() of the remote side as
 abortion. This way, segfaults will not cause the parent to deadlock.
Indeed, eventfd only sends POLLHUP wakeups for in kernel clients, userspace
do not see it, and we can't complain since the doc states that :-/

Having POLLHUP for userspace will make eventfd replace pipe. I hacked a
quick kernel patch to test it, but it would be hard to send it via poll,
since the only available mechanism is eventfd counter and it is already
used to generate POLLERR...

 EINTR handling is currently ignored for any barrier-waits. This can easily
 be added, but it isn't needed so far so I dropped it. EINTR handling in
 general is ugly, anyway. You need to deal with pselect/ppoll/... variants
 and make sure not to unblock signals at the wrong times. So genrally,
 there's little use in adding it.

ppoll is atomic and it is handled by the kernel, so perhaps
setting/restoring sigmask can be done easily! and for nspawn: IMO we need
to receive SIGCHLD which implies EINTR.

I say EINTR since not only for blocking read or infinite poll, but
perhaps for all the other functions that the parent may do to setup the
environment of the container, currently nspawn will set network
interfaces before moving them into the container, it will also register
the machine, and perhaps other operations...

So having EINTR errors is useful here not only for direct reads, but for
all the other calls that might block! IOW I think that nspawn should
have an empty sig handler for SIGCHLD.

Barrier reads already use poll and pipe to handle remote abortion since
it can *not* be done by eventfd, yes this is perfect but for nspawn we
can also achieve the same by combining eventfd and SICCHLD!

What do you think if we make Barrier use:
eventfd+pipe and/or eventfd+SIGCHLD ?

Most complex fork/clone code should receive SIGCHLD, and think about
nspawn! we do want it to be as lightweigh as possible, having 4 fds by
default (2 eventfd + heavy pipe) may hit some resource limits quickly!

compared to: 2 eventfd + empty sig handler!


And it seems from the patch you are not checking barrier_place() return
code, if the remote aborted ?

Thanks for the patches, sure the API is really nice, I'll try to comment
on #1


 ---
  Makefile.am   |   2 -
  src/nspawn/nspawn.c   | 136 -
  src/shared/eventfd-util.c | 169 
 --
  src/shared/eventfd-util.h |  43 
  4 files changed, 60 insertions(+), 290 deletions(-)
  delete mode 100644 src/shared/eventfd-util.c
  delete mode 100644 src/shared/eventfd-util.h
 
 diff --git a/Makefile.am b/Makefile.am
 index 039a83e..d7b0f90 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -832,8 +832,6 @@ libsystemd_shared_la_SOURCES = \
   src/shared/barrier.h \
   src/shared/async.c \
   src/shared/async.h \
 - src/shared/eventfd-util.c \
 - src/shared/eventfd-util.h \
   src/shared/copy.c \
   src/shared/copy.h \
   src/shared/base-filesystem.c \
 diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
 index bad93a5..e75cc28 100644
 --- a/src/nspawn/nspawn.c
 +++ b/src/nspawn/nspawn.c
 @@ -40,7 +40,6 @@
  #include sys/un.h
  #include sys/socket.h
  #include linux/netlink.h
 -#include sys/eventfd.h
  #include net/if.h
  #include linux/veth.h
  #include sys/personality.h
 @@ -84,12 +83,12 @@
  #include def.h
  #include rtnl-util.h
  #include udev-util.h
 -#include eventfd-util.h
  #include blkid-util.h
  #include gpt.h
  #include siphash24.h
  #include copy.h
  #include base-filesystem.h
 +#include barrier.h
  
  #ifdef HAVE_SECCOMP
  #include seccomp-util.h
 @@ -3074,12 +3073,18 @@ int main(int argc, char *argv[]) {
  
  for (;;) {
  ContainerStatus container_status;
 -int eventfds[2] = { -1, -1 };
 +_barrier_destroy_ Barrier barrier = { };
  struct sigaction sa = {
  .sa_handler = nop_handler,
  .sa_flags = SA_NOCLDSTOP,
  };
  
 +r = barrier_init(barrier);
 +if (r  0) {
 +log_error(Cannot initialize IPC barrier: %s, 
 strerror(-r));
 +goto finish;
 +}
 +
  /* Child can be killed before execv(), so handle SIGCHLD
   * in order to interrupt parent's blocking calls and
   * give it a chance to call wait() and terminate. */
 @@ -3095,9 +3100,9 @@ int main(int argc, char *argv[]) {
  goto finish;
  

Re: [systemd-devel] [PATCH] sysusers: Preserve label of /etc/{passwd, group}

2014-07-13 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Jul 13, 2014 at 01:35:33PM -0700, Colin Walters wrote:
 From b1113ed91ebdcf7ac2546a2618fe83751edfbaa0 Mon Sep 17 00:00:00 2001
 From: Colin Walters walt...@verbum.org
 Date: Fri, 11 Jul 2014 15:03:29 -0400
 Subject: [PATCH] sysusers: Ensure /etc/{passwd,group} are labeled correctly
 
 These files are specially labeled on SELinux systems, and we need to
 preserve that label.
Applied. I also made a follow up to move this funtionality to fileio-label.[ch].

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] fileio: quote more shell characters in envfiles

2014-07-13 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Jul 13, 2014 at 06:49:00PM +0300, Mantas Mikulėnas wrote:
 Turns out, making strings shell-proof is harder than expected:
 
 # machinectl set-hostname foo|poweroff  . /etc/machine-info
 
 (This could be simplified by quoting *and* escaping all characters,
 which is harmless in shell but unnecessary.)
Lovely :)

Applied.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel