[systemd-devel] Why does nspawn check if in a user session?
Hi all,
I have another question about `systemd-nspawn` internals.
When sanity-checking argv, it does:
if (arg_keep_unit && arg_register && cg_pid_get_owner_uid(0, NULL) >=
0) {
log_error("--keep-unit --register=yes may not be used when
invoked from a user session.");
return -EINVAL;
}
(the `&& arg_register` bit was added in 234)
Why does nspawn care if it is in a user session?
My best guess is that it doesn't want to share its cgroup with any
other processes, and it is using user session membership as a sloppy
proxy for that. If that's the case, wouldn't it be more correct and
robust to check for other processes in
"/sys/fs/cgroup/.../cgroup.procs"?
--
Happy hacking,
~ Luke Shumaker
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Why does nspawn need two child processes?
Hi all, I have a question about `systemd-nspawn` internals. When creating the child process, it does something like: parent | clone(MOUNT) | `, | outer_child() | | | clone(rest) | | `, |returninner_child() | ,---'| wait()| | exec() | ||| | exit() | ,' wait() where in the first `clone()` it unshares the mount namespace, and in the second `clone()` it unshares all of the other namespaces (except for the cgroup namespace). Initially, I was confused by the awkward dance with having two children; I couldn't imagine a reason why it is necessary to do this with a separate `inner_child` and `outer_child`; why can't everything be done in a single child process?: parent | clone(MOUNT) | `, |child() | | | unshare(rest) | | | exec() | ||| | exit() | ,' wait() It has used the current two-child approach since user-namespace support was first completed in 03cfe0d5, which only has the brief commit message "nspawn: finish user namespace support"; so there aren't too many clues to be found in the commit log. Part of the answer lies in the behavior of `unshare(CLONE_NEWPID)`. Unlike all of the other namespaces that may be unshared, calling `unshare(CLONE_NEWPID)` doesn't actually unshare the PID namespace in *this* process, it says to unshare the PID namespace at the next `fork()`/`clone()` call. So even if we changed `systemd-nspawn` to the `clone(MOUNT)/unshare(rest)` model, it would still have to `clone()` (or plain `fork()` at that point) a second, inner, child process. So then, I'm left wondering why unsharing the PID namespace can't be moved up to the initial `clone()`, allowing everything else to be `unshare`(2)ed in the initial child process: parent | clone(MOUNT|PID) | `, |child() | | | unshare(rest) | | | exec() | ||| | exit() | ,' wait() So my question becomes: what has to be done *after* unsharing the mount namespace, but *before* unsharing the PID namespace? -- Happy hacking, ~ Luke Shumaker ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] time-sync.target
On Wed, 31 May 2017 04:33:09 -0400,
Pietro Paolini wrote:
> Thanks! Unfortunately it is not an option to install the Go runtime
> system on the board I am working with, is there a place where I can
> find docs about writing my own wrapper in C ?
Go is a compiled language, there is no separate runtime to install.
It's also easy to cross-compile Go programs. However, Go binaries
tend to have a minimum size of about 1.5MB, so I can appreciate not
wanting to add another ~4MB for two tiny programs.
Each of the programs is pretty simple. If you know C, the Go should
be pretty simple to read; I think the weirdest bit to a C programmer
is that "go fn()" runs "fn()" in a separate thread.
The systemd-timesyncd-wait program is super-simple. It recieves a
socket from systemd, waits for some data to come in on it, then exits.
Most of the Go version is re-implementing libsystemd's
sd_listen_fds(). Without actually testing it, I think a C
implementation is as simple as:
#include
#include
#include
#include
int main() {
int r;
char dat[4096];
r = sd_listen_fds(0);
if (r != 1)
error(1, 0, "expected 1 fd, got %d\n", r);
for (;;) {
r = read(SD_LISTEN_FDS_START, dat, sizeof(dat));
if (r < 0)
error(127, errno, "read");
if (r > 0)
return 0;
}
}
The systemd-timesyncd-wrap program is a bit more complicated. It
looks the socket used by sd_notify(), and intercepts it. So look at
the socket pointed to by $NOTIFY_SOCKET, create an identical socket,
and then run the real systemd-timesyncd with $NOTIFY_SOCKET set to the
copy (also, you should set $WATCHDOG_PID to the PID of the real
systemd-timesyncd, but that turned out to be tricky in Go, and wasn't
too important, so I just unset it). We listen on the copy, and simply
forward all messages to the original socket; with the exception that
if the message has a line that starts with "STATUS=Synchronized", then
we *also* send a message to systemd-timesyncd-wait via
"/run/timesyncd/time-sync.sock". The tricky part is that messages
sent on $NOTIFY_SOCKET may have out-of-band control message (cmsg)
data associated with them, and we have to forward this too. Control
messages are poorly documented in both C and Go. To receive them,
look manager.c:manager_dispatch_notify_fd(), and to send them, look at
libsystemd's sd_pid_notify_with_fds().
If you're writing C anyway, it may be easier to patch
systemd-timesyncd to send the additional message when it sends the
"STATUS=Syncronized..." message, rather than wrapping it.
--
Happy hacking,
~ Luke Shumaker
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] time-sync.target
On Tue, 30 May 2017 11:57:00 -0400, Pietro Paolini wrote: > Hi everybody, > > I am trying to configure my systemd to run my service *after* the > timesyncd service is synchronized with the NTP server, unfortunately > adding the "After=time-sycd.target" does not work as I can't see any > change in the behavior - my service is started immediately anyway. > > Reading here https://github.com/systemd/systemd/issues/5097 it looks > like there is a problem with that target but I am not sure where to > start from, is it feasible to patch the systemd-timesyncd daemon to > correct the behavior ? Yes, this is a problem with timesyncd, and it should eventually be patched. The problem with patching it is that without re-thinking something, it is impossible to have it block time-sync.target without also risking systemd-timesyncd.service from timing out. The only solution that I can think of is to have a second helper process that can't time out that waits for a signal from timesyncd; and have that process block time-sync.target. But, the systemd team hasn't yet decided if this solution is acceptable to them. So, anyway, I went ahead wrote such a helper program, and a wrapper for timesyncd that handles sending the signal. > As alternative I could write my wrapper around the daemon as done in > the link reporting the issue, both ways I will need to read > something enlightening on the matter, would you be able to point me > into the right direction ? You should be able to install and use the wrapper that I wrote. There's nothing specific to my use-case about it. If you install it, then the "time-sync.target" should "just work" correctly with timesyncd. Of course, in the long-term it would be preferable to patch timesyncd, instead of having to install separate wrapper to fix the behavior. But for today, it works. -- Happy hacking, ~ Luke Shumaker ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Using systemd --user to manage graphical sessions?
On Wed, 11 May 2016 12:13:45 -0400,
Martin Pitt wrote:
> Or is someone actually using systemd --user for graphical sessions
> already and found a trick that I missed?
I am! For several months now (and before that, I was still using
systemd --user for graphical stuff, but not consistently/coherently).
My configuration: https://lukeshu.com/git/dotfiles/tree/.config
Crappy write-up: https://lukeshu.com/blog/x11-systemd.html
One thing to note is that I don't use a DE, and have minimal
bus-activated services.
The big difference between what I do and what you wrote is that I
don't tie the DISPLAY name to the XDG_SESSION_ID (actually, the
session ID isn't even set in the graphical session).
The short version of how I have it work:
My ~/.xinitrc (AKA: script that starts the initial X11 clients)
contains:
_DISPLAY="$(systemd-escape -- "$DISPLAY")"
mkfifo "${XDG_RUNTIME_DIR}/x11-wm@${_DISPLAY}"
cat < "${XDG_RUNTIME_DIR}/x11-wm@${_DISPLAY}" &
systemctl --user start "X11@${_DISPLAY}.target" &
wait
systemctl --user stop "X11@${_DISPLAY}.target"
Which basically says: start X11@:0.target, wait for something to open
"${XDG_RUNTIME_DIR}/x11-wm@${_DISPLAY}" for writing and then close it,
then stop X11@:0.target. Then I have my window manager configured to
open/close the file when I want to quit X11/log out (really, I have it
open at start, then just exit on quit; implicitly closing it).
Then, each of the whatever@${DISPLAY}.service files contains:
[Unit]
After=X11@%i.target
Requisite=X11@%i.target
[Service]
Environment=DISPLAY=%I
Now, when I launch a program from the window manager, I have it launch
with `systemd-run --user --scope -- sh -c "COMMAND HERE"`, so that
systemd can tell the difference between it and the window manager. I
imagine that this would be problematic with less configurable window
managers.
As I type this, I have two graphical logins to the same user. One on
a real screen, and the other with a fake screen via `vncserver` (of
course, managed as a systemd user unit :-) ).
The only problem I have with this setup is that dunst (my desktop
notification daemon) isn't happy running multiple instances on
different displays. I think it's because it isn't happy sharing the
dbus, but I haven't spent even 1 minute debugging it yet.
--
Happy hacking,
~ Luke Shumaker
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] cg_path_get_user_unit(): Did not correctly parse user-unit templates.
It ran either skip_session() or skip_user_manager(), then ran skip_slices()
iff skip_session() ran. It needs to run skip_slices() in either case.
Included is a test case demonstrating why.
---
src/shared/cgroup-util.c| 19 ---
src/test/test-cgroup-util.c | 1 +
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 0d3cc53..1c5af69 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1251,17 +1251,14 @@ int cg_path_get_user_unit(const char *path, char
**unit) {
/* Skip slices, if there are any */
e = skip_slices(path);
-/* Skip the session scope... */
-t = skip_session(e);
-if (t)
-/* ... and skip more slices if there's one */
-e = skip_slices(t);
-else {
-/* ... or require a user manager unit to be there */
-e = skip_user_manager(e);
-if (!e)
-return -ENOENT;
-}
+/* Skip the session scope or user manager... */
+(t = skip_session(e)) || (t = skip_user_manager(e));
+
+if (!t)
+return -ENOENT;
+
+/* ... and skip more slices if there are any */
+e = skip_slices(t);
return cg_path_decode_unit(e, unit);
}
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index 58eb744..67eeeb5 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -93,6 +93,7 @@ static void test_path_get_user_unit(void) {
check_p_g_u_u(/meh.service, -ENOENT, NULL);
check_p_g_u_u(/session-3.scope/_cpu.service, 0, cpu.service);
check_p_g_u_u(/user.slice/user-1000.slice/user@1000.service/server.service,
0, server.service);
+
check_p_g_u_u(/user.slice/user-1000.slice/user@1000.service/foobar.slice/foobar@pie.service,
0, foobar@pie.service);
check_p_g_u_u(/user.slice/user-1000.slice/user@.service/server.service,
-ENOENT, NULL);
}
--
2.2.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] cg_path_get_user_unit(): Did not correctly parse user-unit templates.
At Wed, 4 Feb 2015 01:52:36 +0100, Lennart Poettering wrote: +/* Skip the session scope or user manager... */ +(t = skip_session(e)) || (t = skip_user_manager(e)); Hmpf, I really don't like this ()||() magic I must say. Any chance you can rework this to just use normal t = skip_session(e); if (!t) t = skip_user_manager(...) THis is really an excercise in making code easily readable, not an excercsie to show superior C skills ;-) Sure. I thought the || version was easier to read; that it showed the intent better. Though it may be that it's similar to a Ruby idiom, and I've been working with some Ruby recently. -- Happy hacking, ~ Luke Shumaker ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCHv2] cg_path_get_user_unit(): Did not correctly parse user-unit templates.
It ran either skip_session() or skip_user_manager(), then ran skip_slices()
iff skip_session() ran. It needs to run skip_slices() in either case.
Included is a test case demonstrating why.
---
src/shared/cgroup-util.c| 18 --
src/test/test-cgroup-util.c | 1 +
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 0d3cc53..d34e1fa 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1251,17 +1251,15 @@ int cg_path_get_user_unit(const char *path, char
**unit) {
/* Skip slices, if there are any */
e = skip_slices(path);
-/* Skip the session scope... */
+/* Skip the session scope or user manager... */
t = skip_session(e);
-if (t)
-/* ... and skip more slices if there's one */
-e = skip_slices(t);
-else {
-/* ... or require a user manager unit to be there */
-e = skip_user_manager(e);
-if (!e)
-return -ENOENT;
-}
+if (!t)
+t = skip_user_manager(e);
+if (!t)
+return -ENOENT;
+
+/* ... and skip more slices if there are any */
+e = skip_slices(t);
return cg_path_decode_unit(e, unit);
}
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index 58eb744..67eeeb5 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -93,6 +93,7 @@ static void test_path_get_user_unit(void) {
check_p_g_u_u(/meh.service, -ENOENT, NULL);
check_p_g_u_u(/session-3.scope/_cpu.service, 0, cpu.service);
check_p_g_u_u(/user.slice/user-1000.slice/user@1000.service/server.service,
0, server.service);
+
check_p_g_u_u(/user.slice/user-1000.slice/user@1000.service/foobar.slice/foobar@pie.service,
0, foobar@pie.service);
check_p_g_u_u(/user.slice/user-1000.slice/user@.service/server.service,
-ENOENT, NULL);
}
--
2.2.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] nspawn: When exiting with an error, make the error code meaningful.
At Sun, 29 Jun 2014 12:31:13 +0100, Djalal Harouni wrote: On Sat, Jun 28, 2014 at 12:09:56PM -0400, Luke Shumaker wrote: This is accomplished by having wait_for_container() return a positive error code when we would like that error code to make its way to the user. This is at odds with the CODING_STYLE rule that error codes should be returned as negative values. This is not odd, we already do that! When I did convert to wait_for_container(), I remember I checked all calls to wait_for_terminate() to see what others do, and there was the: src/shared/util.c:wait_for_terminate_and_warn() Which also returns the positive 'status.si_status' code to caller, and it is used in all places... I missed that wait_for_terminate_and_warn() also did that! With that in consideration, shouldn't the check of the return value from wait_for_terminate_and_warn() at src/quotacheck/quotacheck.c:110 be '== 0' instead of '= 0'? So if you are able to send a v2 of this patch, here are some comments: I'm putting one together now; thanks for the feedback! + * Return values: + * 0 : The container was terminated by a signal, or failed for an + * unknown reason. No change is made to the container argument. wait_for_container() failed to get the state of the container, the container was terminated by a signal or failed for an unknown reason. You mean wait_for_terminate()? A minor thing: in wait_for_container() could you add please a log_warning() if wait_for_terminate() fails, as it is done in wait_for_terminate_and_warn(). Yeah, I'll add it as a separate commit. Thank you for the report and the patch! Thank you for taking the time to review it! Happy hacking, ~ Luke Shumaker ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 2/3] nspawn: Fix regression with exit status
Commit 113cea8 introduced a bug that caused the exit code of systemd-nspawn
to not reflect the exit code of the program executed in the container.
---
src/nspawn/nspawn.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0a8dc0c..be0e6b5 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2645,12 +2645,21 @@ static int change_uid_gid(char **_home) {
}
/*
- * Return 0 in case the container is being rebooted, has been shut
- * down or exited successfully. On failures a negative value is
- * returned.
+ * Return values:
+ * 0 : wait_for_terminate() failed to get the state of the
+ * container, the container was terminated by a signal, or
+ * failed for an unknown reason. No change is made to the
+ * container argument.
+ * 0 : The program executed in the container terminated with an
+ * error. The exit code of the program executed in the
+ * container is returned. No change is made to the container
+ * argument.
+ * 0 : The container is being rebooted, has been shut down or exited
+ * successfully. The container argument has been set to either
+ * CONTAINER_TERMINATED or CONTAINER_REBOOTED.
*
- * The status of the container CONTAINER_TERMINATED or
- * CONTAINER_REBOOTED will be saved in the container argument
+ * That is, success is indicated by a return value of zero, and an
+ * error is indicated by a non-zero value.
*/
static int wait_for_container(pid_t pid, ContainerStatus *container) {
int r;
@@ -2672,7 +2681,6 @@ static int wait_for_container(pid_t pid, ContainerStatus
*container) {
} else {
log_error(Container %s failed with error code %i.,
arg_machine, status.si_status);
-r = -1;
}
break;
@@ -3299,8 +3307,12 @@ check_container_status:
r = wait_for_container(pid, container_status);
pid = 0;
-if (r 0) {
-r = EXIT_FAILURE;
+if (r != 0) {
+/* If r 0, explicitly set to EXIT_FAILURE,
+ * otherwise return the exit code of the
+ * containered process. */
+if (r 0)
+r = EXIT_FAILURE;
break;
} else if (container_status == CONTAINER_TERMINATED)
break;
--
2.0.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 3/3] nspawn: log a warning on failure from wait_for_terminate()
This is at the suggestion of Djalal Harouni on the mailing list, and
reflects the behavior of shared/util.c:wait_for_terminate_and_warn().
---
src/nspawn/nspawn.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index be0e6b5..8fb72d6 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2666,8 +2666,10 @@ static int wait_for_container(pid_t pid, ContainerStatus
*container) {
siginfo_t status;
r = wait_for_terminate(pid, status);
-if (r 0)
+if (r 0) {
+log_warning(Failed to wait for container: %s, strerror(-r));
return r;
+}
switch (status.si_code) {
case CLD_EXITED:
--
2.0.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] nspawn: When exiting with an error, make the error code meaningful.
This is accomplished by having the return value of wait_for_container be
interpreted as a negative version of the exit status code.
---
src/nspawn/nspawn.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0a8dc0c..0c89c40 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2662,7 +2662,7 @@ static int wait_for_container(pid_t pid, ContainerStatus
*container) {
switch (status.si_code) {
case CLD_EXITED:
-r = status.si_status;
+r = -status.si_status;
if (r == 0) {
if (!arg_quiet)
log_debug(Container %s exited successfully.,
@@ -2672,7 +2672,6 @@ static int wait_for_container(pid_t pid, ContainerStatus
*container) {
} else {
log_error(Container %s failed with error code %i.,
arg_machine, status.si_status);
-r = -1;
}
break;
@@ -2699,13 +2698,13 @@ static int wait_for_container(pid_t pid,
ContainerStatus *container) {
case CLD_DUMPED:
log_error(Container %s terminated by signal %s.,
arg_machine, signal_to_string(status.si_status));
-r = -1;
+r = -EXIT_FAILURE;
break;
default:
log_error(Container %s failed due to unknown reason.,
arg_machine);
-r = -1;
+r = -EXIT_FAILURE;
break;
}
@@ -3296,11 +3295,10 @@ check_container_status:
/* Redundant, but better safe than sorry */
kill(pid, SIGKILL);
-r = wait_for_container(pid, container_status);
+r = -wait_for_container(pid, container_status);
pid = 0;
-if (r 0) {
-r = EXIT_FAILURE;
+if (r 0) {
break;
} else if (container_status == CONTAINER_TERMINATED)
break;
--
2.0.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] nspawn: When exiting with an error, make the exit code meaningful.
These two patches both do the same thing, in slightly different ways. They fix a regression intruduced in commit 113cea8 (present in v213 and 214) where the exit status of nspawn does not reflect the exit status of the containered process. Happy hacking, ~ Luke Shumaker ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] nspawn: When exiting with an error, make the error code meaningful.
This is accomplished by having wait_for_container() return a positive error
code when we would like that error code to make its way to the user. This
is at odds with the CODING_STYLE rule that error codes should be returned
as negative values.
---
src/nspawn/nspawn.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0a8dc0c..42f939b 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2645,12 +2645,17 @@ static int change_uid_gid(char **_home) {
}
/*
- * Return 0 in case the container is being rebooted, has been shut
- * down or exited successfully. On failures a negative value is
- * returned.
+ * Return values:
+ * 0 : The container was terminated by a signal, or failed for an
+ * unknown reason. No change is made to the container argument.
+ * 0 : The container terminated with an error code, which is the
+ * return value. No change is made to the container argument.
+ * 0 : The container is being rebooted, has been shut down or exited
+ * successfully. The container argument has been set to either
+ * CONTAINER_TERMINATED or CONTAINER_REBOOTED.
*
- * The status of the container CONTAINER_TERMINATED or
- * CONTAINER_REBOOTED will be saved in the container argument
+ * That is, success is indicated by a return value of zero, and an
+ * error is indicated by a non-zero value.
*/
static int wait_for_container(pid_t pid, ContainerStatus *container) {
int r;
@@ -2672,7 +2677,6 @@ static int wait_for_container(pid_t pid, ContainerStatus
*container) {
} else {
log_error(Container %s failed with error code %i.,
arg_machine, status.si_status);
-r = -1;
}
break;
@@ -3299,8 +3303,9 @@ check_container_status:
r = wait_for_container(pid, container_status);
pid = 0;
-if (r 0) {
-r = EXIT_FAILURE;
+if (r != 0) {
+if (r 0)
+r = EXIT_FAILURE;
break;
} else if (container_status == CONTAINER_TERMINATED)
break;
--
2.0.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] ptyfwd: Set the size of the PTY base on the size of stdout, not stdin.
---
src/shared/ptyfwd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index 7225b93..85a0ddc 100644
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -305,7 +305,7 @@ static int process_pty_loop(int master, sigset_t *mask,
pid_t kill_pid, int sign
struct winsize ws;
/* The window size changed, let's
forward that. */
-if (ioctl(STDIN_FILENO, TIOCGWINSZ,
ws) = 0)
+if (ioctl(STDOUT_FILENO, TIOCGWINSZ,
ws) = 0)
ioctl(master, TIOCSWINSZ, ws);
} else if (sfsi.ssi_signo == SIGTERM
kill_pid 0 signo 0 !tried_orderly_shutdown) {
@@ -346,7 +346,7 @@ int process_pty(int master, sigset_t *mask, pid_t kill_pid,
int signo) {
struct winsize ws;
int r;
-if (ioctl(STDIN_FILENO, TIOCGWINSZ, ws) = 0)
+if (ioctl(STDOUT_FILENO, TIOCGWINSZ, ws) = 0)
ioctl(master, TIOCSWINSZ, ws);
if (tcgetattr(STDIN_FILENO, saved_attr) = 0) {
--
1.8.4.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] nspawn (ptyfwd): Correctly handle output redirection
At Tue, 5 Nov 2013 19:59:46 +0100, Lennart Poettering wrote: On Mon, 04.11.13 11:05, Luke T. Shumaker (luke...@sbcglobal.net) wrote: A couple of weeks ago, I reported a bug that systemd-nspawn does not correctly handle I/O redirection[1]. I described in detail the several smaller bugs that lead up to both stdin and stdout redirection being broken, and uploaded a patch that fixes stdout redirection. That patch is attached here. Stdin redirection is more involved to fix. Hmm, can you rebase on current git please? Sorry it took so long, I've been unable to devote much time to this. Happy hacking, ~ Luke Shumaker ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] ptyfwd: Don't set the output prop of stdin, nor the input props of stdout.
It was calling cfmakeraw(3) on the properties for STDIN_FILENO; cfmakeraw
sets both input and output properties. If (and only if) stdin and stdout
are the same device is this correct. Otherwise, we must change only the
input properties of stdin, and only the output properties of stdout.
---
src/shared/ptyfwd.c | 35 +++
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index 85a0ddc..72aa59e 100644
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -341,29 +341,40 @@ static int process_pty_loop(int master, sigset_t *mask,
pid_t kill_pid, int sign
}
int process_pty(int master, sigset_t *mask, pid_t kill_pid, int signo) {
-struct termios saved_attr;
-bool saved = false;
+struct termios saved_stdin_attr, raw_stdin_attr;
+struct termios saved_stdout_attr, raw_stdout_attr;
+bool saved_stdin = false;
+bool saved_stdout = false;
struct winsize ws;
int r;
if (ioctl(STDOUT_FILENO, TIOCGWINSZ, ws) = 0)
ioctl(master, TIOCSWINSZ, ws);
-if (tcgetattr(STDIN_FILENO, saved_attr) = 0) {
-struct termios raw_attr;
-saved = true;
+if (tcgetattr(STDIN_FILENO, saved_stdin_attr) = 0) {
+saved_stdin = true;
-raw_attr = saved_attr;
-cfmakeraw(raw_attr);
-raw_attr.c_lflag = ~ECHO;
-
-tcsetattr(STDIN_FILENO, TCSANOW, raw_attr);
+raw_stdin_attr = saved_stdin_attr;
+cfmakeraw(raw_stdin_attr);
+raw_stdin_attr.c_oflag = saved_stdin_attr.c_oflag;
+tcsetattr(STDIN_FILENO, TCSANOW, raw_stdin_attr);
+}
+if (tcgetattr(STDOUT_FILENO, saved_stdout_attr) = 0) {
+saved_stdout = true;
+
+raw_stdout_attr = saved_stdout_attr;
+cfmakeraw(raw_stdout_attr);
+raw_stdout_attr.c_iflag = saved_stdout_attr.c_iflag;
+raw_stdout_attr.c_lflag = saved_stdout_attr.c_lflag;
+tcsetattr(STDOUT_FILENO, TCSANOW, raw_stdout_attr);
}
r = process_pty_loop(master, mask, kill_pid, signo);
-if (saved)
-tcsetattr(STDIN_FILENO, TCSANOW, saved_attr);
+if (saved_stdout)
+tcsetattr(STDOUT_FILENO, TCSANOW, saved_stdout_attr);
+if (saved_stdin)
+tcsetattr(STDIN_FILENO, TCSANOW, saved_stdin_attr);
return r;
--
1.8.4.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
