Re: [systemd-devel] bpfilter blocks root unmount during shutdown
On Mon, 24 Sep 2018 15:20:47 +0200 Lennart Poettering wrote: > On So, 23.09.18 10:38, Andrei Borzenkov (arvidj...@gmail.com) wrote: > > > Dracut /shutdown script first tries to kill all processes still > > running off old root. Unfortunately this fails for special user > > process that runs bpfilter because it does not include reference > > to /oldroot in places where dracut looks for in > > kilall_proc_mountpoint() > > Hmm, when we invoke the /shutdown executable we already executed our > process killing spree as part of systemd-shutdown. How come your > processes even survive that long? What am I missing? I believe it's because the bpfilter helper process is identified as a kernel thread - since it has an empty command line - and therefore not killed. I personally feel this is a bug (in the kernel), but apparently this whole bpfilter thing isn't quite ready yet and shouldn't be used for the moment -- so hopefully it'll improve/be fixed in the mean time. You can see this thread[1] about the issue. Cheers, [1] https://www.spinics.net/lists/netdev/msg520030.html ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Fail to reset-failed as user
On 02/11/15 21:13, Lennart Poettering wrote: On Thu, 05.02.15 19:20, Olivier Brunel (j...@jjacky.com) wrote: On 02/03/15 22:17, Lennart Poettering wrote: On Fri, 12.12.14 16:06, Olivier Brunel (j...@jjacky.com) wrote: Sorry for resurrecting this old thread this late. Is this still an issue? Does this work on current git? Still an issue w/ 218 yes, haven't actually had time to try with current git. I'll try to do that over the weekend. Today I had one unit in failed state, and after taking care of things I wanted to simply reset its state (to inactive) w/out having to start it. Looking up the man page, I see there's a command reset-failed for this exact purpose, awesome. So I go: % systemctl reset-failed backups2.service Failed to reset failed state of unit backups2.service: No such device or address Hmm, did you issue this from some weird environment (su/sudo context, from a system service context or so?) If this is still an issue, could you try to reproduce this after issuing systemd-analyze set-log-level debug? Then please attach the log output this generates! Meanwhile, this is what I get today: http://ix.io/gaR This is not from some weird environment no (or, not that I'm aware of), but an (almost) up-to-date Arch Linux x64, systemd 218. Puzzled. Don't see how this can happen. Also, works fine here... If you can reproduce this on git, it would be good to gdb this. For that: a) start gdb, type attach 1, to attach to PID 1 b) add a breakpoint on method_reset_failed_unit, by issuing b method_reset_failed_unit c) Continue execution of PID 1, by typing in the line c d) trigger the issue, gdb should break at that instant. e) now, check which call fails by stepping through the function with n. As soon as the function is left, use c to continue execution. Not that the function will be executed twice, one after the other. The first invocation will be without PolicyKit privs, the second one with PolicyKit privs. The second invocation is the interesting one. Check why it exits non-zero, and whether unit_reset_failed() is invoked at all (which actually does the inetersting work). f) post your findings here Alright so I did some testing, here's what I found: - on that second invocation, method_reset_failed_unit() fails from its call to bus_unit_method_reset_failed(), and that comes down to bus_message_enter_struct() returning -ENXIO. - I don't know how this whole thing is supposed to work, but what I noticed is that bus_message_enter_struct() is called twice from method_reset_failed_unit(), once from bus_verify_manage_unit_async() and then from bus_unit_method_reset_failed(). Details as follow: First, when bus_verify_manage_unit_async() is called: #0 bus_message_enter_struct (m=0x7f5fb0cb88b0, c=0x7f5fb0cb8aa0, contents=0x7f5faef0d152 bba{ss}, item_size=0x7fffcebd4928, offsets=0x7fffcebd4918, n_offsets=0x7fffcebd4920) at src/libsystemd/sd-bus/bus-message.c:3865 #1 0x7f5faee80136 in sd_bus_message_enter_container (m=0x7f5fb0cb88b0, type=114 'r', contents=0x7f5faef0d152 bba{ss}) at src/libsystemd/sd-bus/bus-message.c:4012 #2 0x7f5faee8e00d in bus_verify_polkit_async (call=0x7f5fb0ca59a0, capability=21, action=0x7f5faeef05f8 org.freedesktop.systemd1.manage-units, interactive=false, registry=0x7f5fb0c0a890, error=0x7fffcebd4ad0) at src/libsystemd/sd-bus/bus-util.c:374 #3 0x7f5faee0aa00 in bus_verify_manage_unit_async (m=0x7f5fb0c0a460, call=0x7f5fb0ca59a0, error=0x7fffcebd4ad0) at src/core/dbus.c:1196 #4 0x7f5faee0c801 in method_reset_failed_unit (bus=0x7f5fb0ca32f0, message=0x7f5fb0ca59a0, userdata=0x7f5fb0c0a460, error=0x7fffcebd4ad0) at src/core/dbus-manager.c:574 (gdb) p *c $38 = {enclosing = 0 '\000', need_offsets = false, index = 0, saved_index = 0, signature = 0x7f5fb0c09110 (bba{ss}), before = 0, begin = 0, end = 133, array_size = 0x0, offsets = 0x0, n_offsets = 0, offsets_allocated = 0, offset_index = 0, item_size = 133, peeked_signature = 0x0} (gdb) p contents $39 = 0x7f5faef0d152 bba{ss} It eventually returns 1. Then it gets to called from bus_unit_method_reset_failed(): #0 bus_message_enter_struct (m=0x7f5fb0cb88b0, c=0x7f5fb0cb8250, contents=0x7f5faef0d152 bba{ss}, item_size=0x7fffcebd48e8, offsets=0x7fffcebd48d8, n_offsets=0x7fffcebd48e0) at src/libsystemd/sd-bus/bus-message.c:3865 #1 0x7f5faee80136 in sd_bus_message_enter_container (m=0x7f5fb0cb88b0, type=114 'r', contents=0x7f5faef0d152 bba{ss}) at src/libsystemd/sd-bus/bus-message.c:4012 #2 0x7f5faee8e00d in bus_verify_polkit_async (call=0x7f5fb0ca59a0, capability=21, action=0x7f5faeef05f8 org.freedesktop.systemd1.manage-units, interactive=false, registry=0x7f5fb0c0a890, error=0x7fffcebd4ad0) at src/libsystemd/sd-bus/bus-util.c:374 #3 0x7f5faee0aa00 in bus_verify_manage_unit_async (m=0x7f5fb0c0a460, call=0x7f5fb0ca59a0, error=0x7fffcebd4ad0) at src/core/dbus.c
Re: [systemd-devel] Fail to reset-failed as user
On 02/03/15 22:17, Lennart Poettering wrote: On Fri, 12.12.14 16:06, Olivier Brunel (j...@jjacky.com) wrote: Sorry for resurrecting this old thread this late. Is this still an issue? Does this work on current git? Still an issue w/ 218 yes, haven't actually had time to try with current git. I'll try to do that over the weekend. Today I had one unit in failed state, and after taking care of things I wanted to simply reset its state (to inactive) w/out having to start it. Looking up the man page, I see there's a command reset-failed for this exact purpose, awesome. So I go: % systemctl reset-failed backups2.service Failed to reset failed state of unit backups2.service: No such device or address Hmm, did you issue this from some weird environment (su/sudo context, from a system service context or so?) If this is still an issue, could you try to reproduce this after issuing systemd-analyze set-log-level debug? Then please attach the log output this generates! Meanwhile, this is what I get today: http://ix.io/gaR This is not from some weird environment no (or, not that I'm aware of), but an (almost) up-to-date Arch Linux x64, systemd 218. -j Thanks, Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Fail to reset-failed as user
Hi, Today I had one unit in failed state, and after taking care of things I wanted to simply reset its state (to inactive) w/out having to start it. Looking up the man page, I see there's a command reset-failed for this exact purpose, awesome. So I go: % systemctl reset-failed backups2.service Failed to reset failed state of unit backups2.service: No such device or address I was nicely asked to authenticate, but then it failed stating the unit doesn't exist or something (not sure what the error message refers to)? Now of course said unit does exist: % systemctl is-failed backups2.service failed And I could eventually do it, as root: % sudo systemctl reset-failed backups2.service This worked fine and is probably what I would have done had my fingers not slipped (sc instead of ssc, aliases for `systemctl` and `sudo systemctl` resp.), but I'm not sure what I missed: since I was properly authenticated, shouldn't the systemctl call also have worked? FYI here's what shows up in the journal, confirming the auth: Dec 12 15:40:00 arch.local polkitd[670]: Operator of unix-session:c3 successfully authenticated as unix-user:jjacky to gain TEMPORARY authorization for action org.freedesktop.systemd1.manage-units for system-bus-name::1.259 [systemctl reset-failed backups2.service] (owned by unix-user:jjacky) What am I missing/misunderstanding? (or is this a bug?) Thanks, -j ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] journal: Fix navigating backwards missing entries
With DIRECTION_UP (i.e. navigating backwards) in generic_array_bisect() when the needle was found as the last item in the array, it wasn't actually processed as match, resulting in entries being missed. https://bugs.freedesktop.org/show_bug.cgi?id=86855 --- This was a good excuse for me to dive in and learn about journal's internals, but someone with actual knowledge of said internals should review this, in case I missed something. src/journal/journal-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 7858435..c5d2d19 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -1657,7 +1657,7 @@ static int generic_array_bisect( } } -if (k n) { +if (k = n) { if (direction == DIRECTION_UP) { i = n; subtract_one = true; -- 2.1.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt
On 08/11/14 16:25, Lennart Poettering wrote: On Fri, 08.08.14 20:45, Olivier Brunel (j...@jjacky.com) wrote: In session_prepare_vt() we set owner of /dev/ttyX to the user, as that is needed for things to work. However, we shouldn't reset it to root on session_restore_vt() since it could have in fact already been set to the user. I don't follow here, can't parse this. Could you please elaborate? I meant, before the call to session_prepare_vt() the owner of /dev/ttyX might not be root, but already set to the user. In which case setting it back to root might not be expected/best. E.g. if a log in on tty3 right now, /dev/tty3 will be owned by my user, then if a process try a TakeControl() and it failed (or after it's done) the ownership would be set to root, even though it wasn't actually root to begin with. --- src/login/logind-session.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index fdeacb1..905e73f 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1070,8 +1070,6 @@ void session_restore_vt(Session *s) { mode.mode = VT_AUTO; ioctl(vt, VT_SETMODE, mode); -fchown(vt, 0, -1); - s-vtfd = safe_close(s-vtfd); } Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 3/3] logind: session: Fix not allowing more than one controller
On 08/11/14 16:34, Lennart Poettering wrote: On Fri, 08.08.14 20:45, Olivier Brunel (j...@jjacky.com) wrote: While a session can only ever have one controller, there can be more than one session with a controller at a time. However, because of the handling of SIGUSR1 for handling VT switch, trying to set a controller on a session while another session had a controller would fail. I really don't feel comfortable with using SIGUSR1 for this, anyway I must say... SIGUSR1 and SIGUSR2 I think should be left for admins to communicate with the daemon for some simple operations, but using this internally sounds wrong. Now, the VT is so stupid to require a signa handler here, but I think using SIGRTMIN+1 or so might be the better choice here? Now, what makes we wonder here, shouldn't we just install a single signal event handler for this when logind initializes, and leaving it on until the very end? Just to note: the problem is that when the signal is called, there's no way of telling which VT it is about. I think it was only intended for one process to handle one VT, so there was no question. But if logind wants to handle more than one VT, to know on which VT to operate then I'm not sure it would actually be doable unless using a different signal per VT... David, could you comment on this, please? Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt
On 08/11/14 16:54, Lennart Poettering wrote: On Mon, 11.08.14 16:39, Olivier Brunel (j...@jjacky.com) wrote: On 08/11/14 16:25, Lennart Poettering wrote: On Fri, 08.08.14 20:45, Olivier Brunel (j...@jjacky.com) wrote: In session_prepare_vt() we set owner of /dev/ttyX to the user, as that is needed for things to work. However, we shouldn't reset it to root on session_restore_vt() since it could have in fact already been set to the user. I don't follow here, can't parse this. Could you please elaborate? I meant, before the call to session_prepare_vt() the owner of /dev/ttyX might not be root, but already set to the user. In which case setting it back to root might not be expected/best. But that sounds more as if session_restore_vt() should not be used as-is as cleanup path for session_prepare_vt(), no? E.g. if a log in on tty3 right now, /dev/tty3 will be owned by my user, then if a process try a TakeControl() and it failed (or after it's done) the ownership would be set to root, even though it wasn't actually root to begin with. Isn't this very theoretic? I mean, when does TakeControl() actually really fail for you IRL? Right, this was noticed when trying to start a second rootless X, since with systemd-215 things currently fail, and the ownership of /dev/ttyX would then be switched/changed to root. session_prepare_vt() could also check the owner first, and note what to reset to on session_restore_vt(), to effectively restore things as they were. Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt
On 08/11/14 17:12, David Herrmann wrote: Hi On Mon, Aug 11, 2014 at 5:05 PM, Olivier Brunel j...@jjacky.com wrote: On 08/11/14 16:54, Lennart Poettering wrote: On Mon, 11.08.14 16:39, Olivier Brunel (j...@jjacky.com) wrote: On 08/11/14 16:25, Lennart Poettering wrote: On Fri, 08.08.14 20:45, Olivier Brunel (j...@jjacky.com) wrote: In session_prepare_vt() we set owner of /dev/ttyX to the user, as that is needed for things to work. However, we shouldn't reset it to root on session_restore_vt() since it could have in fact already been set to the user. I don't follow here, can't parse this. Could you please elaborate? I meant, before the call to session_prepare_vt() the owner of /dev/ttyX might not be root, but already set to the user. In which case setting it back to root might not be expected/best. But that sounds more as if session_restore_vt() should not be used as-is as cleanup path for session_prepare_vt(), no? E.g. if a log in on tty3 right now, /dev/tty3 will be owned by my user, then if a process try a TakeControl() and it failed (or after it's done) the ownership would be set to root, even though it wasn't actually root to begin with. Isn't this very theoretic? I mean, when does TakeControl() actually really fail for you IRL? Right, this was noticed when trying to start a second rootless X, since with systemd-215 things currently fail, and the ownership of /dev/ttyX would then be switched/changed to root. Wait, what? Can you please elaborate. Currently, only one process can Sorry, I meant e.g. having one rootless X on tt1 and starting another one on tty2. Currently this fails (see other patch/mail), and is how this was observed. be controller at a time, and session_prepare_vt() is called *after* the controller is set. Therefore, it is not called when you try starting a second controller. The only race I see is this: * start legacy Xserver (which doesn't use TakeControl()) which sets user-id on the TTY * start new Xserver which uses TakeControl() (and thus calls session_prepare_vt()) * stop new Xserver (thus drop Control and reset the TTY) In this case, the new xserver will try to start up, but fail as the old one is still running. Therefore, it *might* call ReleaseControl() and thus reset the TTY. However, you're not supposed to mix both and this is not a legitimate use-case. I mean, the old server is root and modifies the TTY by itself (without using systemd). Obviously, this is racy. From a systemd-logind perspective, this is the same as if you run sudo chown xyz /dev/tty manually. session_prepare_vt() could also check the owner first, and note what to reset to on session_restore_vt(), to effectively restore things as they were. Even though I don't see any problem here, I'd be fine with such a patch. Care to send one? Note that you probably need to store that information in the session-file (session_save() / session_load()) too. If you think the current behavior (setting to root on session_restore_vt()) is fine, I'm fine leaving it as is. Else, I could work on a patch as described. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] login: share VT-signal handler between sessions
On 08/11/14 18:21, David Herrmann wrote: sd-event does not allow multiple handlers for a single signal. However, logind sets up signal handlers for each session with VT_PROCESS set (that is, it has an active controller). Therefore, registering multiple such controllers will fail. Lets make the VT-handler global, as it's mostly trivial, anyway. This way, the sessions don't have to take care of that and we can simply acknowledge all VT-switch requests as we always did. --- Hi Olivier Can you give this a try? It should solve your issues. Yes, just tried it -- it does solve the issue. Thanks guys, -j BTW there's a bug report opened about this issue: https://bugs.freedesktop.org/show_bug.cgi?id=81932 Thanks David src/login/logind-session.c | 26 ++- src/login/logind-session.h | 1 - src/login/logind.c | 62 ++ 3 files changed, 64 insertions(+), 25 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index fdeacb1..26b6a90 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -153,8 +153,6 @@ void session_free(Session *s) { hashmap_remove(s-manager-sessions, s-id); -s-vt_source = sd_event_source_unref(s-vt_source); - free(s-state_file); free(s); } @@ -994,19 +992,9 @@ static int session_open_vt(Session *s) { return s-vtfd; } -static int session_vt_fn(sd_event_source *source, const struct signalfd_siginfo *si, void *data) { -Session *s = data; - -if (s-vtfd = 0) -ioctl(s-vtfd, VT_RELDISP, 1); - -return 0; -} - void session_prepare_vt(Session *s) { int vt, r; struct vt_mode mode = { 0 }; -sigset_t mask; vt = session_open_vt(s); if (vt 0) @@ -1024,20 +1012,12 @@ void session_prepare_vt(Session *s) { if (r 0) goto error; -sigemptyset(mask); -sigaddset(mask, SIGUSR1); -sigprocmask(SIG_BLOCK, mask, NULL); - -r = sd_event_add_signal(s-manager-event, s-vt_source, SIGUSR1, session_vt_fn, s); -if (r 0) -goto error; - /* Oh, thanks to the VT layer, VT_AUTO does not work with KD_GRAPHICS. * So we need a dummy handler here which just acknowledges *all* VT * switch requests. */ mode.mode = VT_PROCESS; -mode.relsig = SIGUSR1; -mode.acqsig = SIGUSR1; +mode.relsig = SIGRTMIN; +mode.acqsig = SIGRTMIN + 1; r = ioctl(vt, VT_SETMODE, mode); if (r 0) goto error; @@ -1058,8 +1038,6 @@ void session_restore_vt(Session *s) { if (vt 0) return; -s-vt_source = sd_event_source_unref(s-vt_source); - ioctl(vt, KDSETMODE, KD_TEXT); if (read_one_line_file(/sys/module/vt/parameters/default_utf8, utf8) = 0 *utf8 == '1') diff --git a/src/login/logind-session.h b/src/login/logind-session.h index e62b76d..562332c 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -98,7 +98,6 @@ struct Session { Seat *seat; unsigned int vtnr; int vtfd; -sd_event_source *vt_source; pid_t leader; uint32_t audit_id; diff --git a/src/login/logind.c b/src/login/logind.c index ec5529d..b470916 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -720,6 +720,46 @@ static int manager_connect_bus(Manager *m) { return 0; } +static int manager_vt_switch(sd_event_source *src, const struct signalfd_siginfo *si, void *data) { +Manager *m = data; +Session *active, *iter; + +/* + * We got a VT-switch signal and we have to acknowledge it immediately. + * Preferably, we'd just use m-seat0-active-vtfd, but unfortunately, + * old user-space might run multiple sessions on a single VT, *sigh*. + * Therefore, we have to iterate all sessions and find one with a vtfd + * on the requested VT. + * As only VTs with active controllers have VT_PROCESS set, our current + * notion of the active VT might be wrong (for instance if the switch + * happens while we setup VT_PROCESS). Therefore, read the current VT + * first and then use s-active-vtnr as reference. Note that this is + * not racy, as no further VT-switch can happen as long as we're in + * synchronous VT_PROCESS mode. + */ + +seat_read_active_vt(m-seat0); + +active = m-seat0-active; +if (!active || active-vtnr 1) { +log_warning(Received VT_PROCESS signal without a registered session on that VT.); +return 0; +} + +if (active-vtfd = 0) { +ioctl(active-vtfd, VT_RELDISP, 1); +
[systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt
In session_prepare_vt() we set owner of /dev/ttyX to the user, as that is needed for things to work. However, we shouldn't reset it to root on session_restore_vt() since it could have in fact already been set to the user. --- src/login/logind-session.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index fdeacb1..905e73f 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1070,8 +1070,6 @@ void session_restore_vt(Session *s) { mode.mode = VT_AUTO; ioctl(vt, VT_SETMODE, mode); -fchown(vt, 0, -1); - s-vtfd = safe_close(s-vtfd); } -- 2.0.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/3] logind: session: Fix not allowing more than one controller
While a session can only ever have one controller, there can be more than one session with a controller at a time. However, because of the handling of SIGUSR1 for handling VT switch, trying to set a controller on a session while another session had a controller would fail. E.g. trying to start a rootless X while there's already one running on another session would fail, as its request to TakeControl() would fail, because we couldn't set a signal handler for SIGUSR1 (since we've already one set up for the other session). When a session goes inactive, we remove the handler (since it cannot be deactivated anymore), thus allowing to set one up for another session if needed. Of course, when the session is activated again, we reset the handler. (Should be noted that our handler might also never be called, if e.g. the controller did a VT_SETMODE as well, thus handling/receiving the SIGUSR1) Alas, if there can be two sessions active at the same time (in different seats) then the second TakeControl() request will fail, for the same reason. This could probably be fixed by making sd-event support multiple signal handlers per signal. https://bugs.freedesktop.org/show_bug.cgi?id=81932 --- Note that when I say trying to start another rootless X would have its TakeControl() to fail, that's only true with the previous patch. Today it actually succeeds, but then X fails with a permission denied error when it tries to access /dev/ttyX, as session_restore_vt() was called and, amongst other things, the owner was reset to root. src/login/logind-seat.c| 9 + src/login/logind-session.c | 39 ++- src/login/logind-session.h | 2 ++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 9992195..1fb0d68 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -239,6 +239,8 @@ int seat_set_active(Seat *s, Session *session) { s-active = session; if (old_active) { +if (old_active-controller) +session_vt_deactivate(old_active); session_device_pause_all(old_active); session_send_changed(old_active, Active, NULL); } @@ -246,6 +248,13 @@ int seat_set_active(Seat *s, Session *session) { seat_apply_acls(s, old_active); if (session session-started) { +if (session-controller) { +int r; + +r = session_vt_activate(session); +if (r 0) +log_error(failed to activate VT %u for session %s (%d/%d), session-vtnr, session-id, r, errno); +} session_send_changed(session, Active, NULL); session_device_resume_all(session); } diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 3f4e177..7428e6c 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1003,10 +1003,35 @@ static int session_vt_fn(sd_event_source *source, const struct signalfd_siginfo return 0; } +int session_vt_activate(Session *s) { +int r; +sigset_t mask; + +sigemptyset(mask); +sigaddset(mask, SIGUSR1); +sigprocmask(SIG_BLOCK, mask, NULL); + +r = sd_event_add_signal(s-manager-event, s-vt_source, SIGUSR1, session_vt_fn, s); +if (r 0) +return r; + +return 0; +} + +void session_vt_deactivate(Session *s) { +assert(s); + +/* This is called once the session is inactive, and therefore the + * handler isn't needed anymore, and will be reset when the session is + * activated again. + * Meanwhile, it allows to set up another handler for another session if + * needed. */ +s-vt_source = sd_event_source_unref(s-vt_source); +} + int session_prepare_vt(Session *s) { int vt, r; struct vt_mode mode = { 0 }; -sigset_t mask; vt = session_open_vt(s); if (vt 0) @@ -1024,17 +1049,13 @@ int session_prepare_vt(Session *s) { if (r 0) goto error; -sigemptyset(mask); -sigaddset(mask, SIGUSR1); -sigprocmask(SIG_BLOCK, mask, NULL); - -r = sd_event_add_signal(s-manager-event, s-vt_source, SIGUSR1, session_vt_fn, s); -if (r 0) -goto error; - /* Oh, thanks to the VT layer, VT_AUTO does not work with KD_GRAPHICS. * So we need a dummy handler here which just acknowledges *all* VT * switch requests. */ +r = session_vt_activate(s); +if (r 0) +goto error; + mode.mode = VT_PROCESS; mode.relsig = SIGUSR1; mode.acqsig = SIGUSR1; diff --git a/src/login/logind-session.h b/src/login/logind-session.h index 2ab3182..5ad2f26 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h
[systemd-devel] [PATCH 2/3] logind: session: set_controller should fail if prepare_vt fails
If controllers can expect logind to have prepared the VT (e.g. set it to graphics mode, etc) then TakeControl() should fail if said preparation failed (and session_restore_vt() was called). --- src/login/logind-session.c | 15 +-- src/login/logind-session.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 905e73f..3f4e177 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1003,14 +1003,14 @@ static int session_vt_fn(sd_event_source *source, const struct signalfd_siginfo return 0; } -void session_prepare_vt(Session *s) { +int session_prepare_vt(Session *s) { int vt, r; struct vt_mode mode = { 0 }; sigset_t mask; vt = session_open_vt(s); if (vt 0) -return; +return vt; r = fchown(vt, s-user-uid, -1); if (r 0) @@ -1042,11 +1042,12 @@ void session_prepare_vt(Session *s) { if (r 0) goto error; -return; +return 0; error: log_error(cannot mute VT %u for session %s (%d/%d), s-vtnr, s-id, r, errno); session_restore_vt(s); +return r; } void session_restore_vt(Session *s) { @@ -1123,8 +1124,6 @@ int session_set_controller(Session *s, const char *sender, bool force) { return r; } -session_swap_controller(s, t); - /* When setting a session controller, we forcibly mute the VT and set * it into graphics-mode. Applications can override that by changing * VT state after calling TakeControl(). However, this serves as a good @@ -1133,7 +1132,11 @@ int session_set_controller(Session *s, const char *sender, bool force) { * exits. * If logind crashes/restarts, we restore the controller during restart * or reset the VT in case it crashed/exited, too. */ -session_prepare_vt(s); +r = session_prepare_vt(s); +if (r 0) +return r; + +session_swap_controller(s, t); return 0; } diff --git a/src/login/logind-session.h b/src/login/logind-session.h index e62b76d..2ab3182 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -172,7 +172,7 @@ SessionClass session_class_from_string(const char *s) _pure_; const char *kill_who_to_string(KillWho k) _const_; KillWho kill_who_from_string(const char *s) _pure_; -void session_prepare_vt(Session *s); +int session_prepare_vt(Session *s); void session_restore_vt(Session *s); bool session_is_controller(Session *s, const char *sender); -- 2.0.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/3] Fix issues re: visibility of status messages
On 11/14/13 05:40, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Sep 20, 2013 at 10:18:27PM +0200, Olivier Brunel wrote: Hi, I'm running Arch Linux, have been using systemd-204, and recently tried the new 207 release, and I have been having some issues with it. One was that status messages would just stop at some point near the end of the boot process, and also that I wouldn't get any during a shutdown/reboot. It might be useful to note that I don't start a getty on tty1, which is why I expect to see all status messages until default target is reached, even after the getty/login has been started (which happens on tty2). After looking into it, I came up with the following patches to fix the issue. The reason status messages would stop was that the getty was started, and systemd then stopped using the console to avoid collisions w/ gettys. However, as I said I don't have a getty started on tty1 so for me that is a bug, as there's no reason not to keep printing status messages on tty1. The lack of messages on shutdown/reboot was also linked to this, because if no_console_output was set to true during boot, it'd stay there and prevent messages to show up on shutdown. To fix this (in the event it was set to true on boot) a patch simply resets it to false on job_shutdown_magic(), but I'm not exactly sure if that's the right way to do this. All 3 patches applied. I *think* they are all correct, but this code has so many corner cases that it's hard to be sure. I made some tweaks, please check that it still works. Sorry for the delay. In the future, if you don't get an answer within a week or two, please holler :) Patches do sometimes slip through, especially when there are a lot of changes like recently, and a ping to the ml will help to bring the thread to the bottom. Noted, thanks. Tried the latest git, it all works as expected. FYI I should add that in a similar setup as the one I described, this will not be enough to keep messages on tty1, since fsck's units are now RemainAfterExit (see https://bugs.freedesktop.org/show_bug.cgi?id=66784), which means they're seen by systemd as owning the console (as far as outputing messages there is concerned I mean), and it will therefore stop printing status messages. I'm not sure you want to fix this, as it might be only a cosmetic issue for a small usecase hence not worth the trouble, so I've simply undone it using a .conf file on my end, figured I should mention it though. Hm, we could detect this case by looking at services in the SERVICE_EXITED substate. It might actually be worth fixing, since almost everything now is RemainAfterExit=true. Alright, I've looked into this a bit, I'll send a patch that should handle it as well. -j Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Fix RemainAfterExit services keeping a hold on console
When a service exits succesfully and has RemainAfterExit set, its hold on the console (in m-n_on_console) wasn't released since the unit state didn't change. --- TODO | 2 -- src/core/service.c | 16 src/core/unit.c| 3 +++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 57e1122..efc7e2a 100644 --- a/TODO +++ b/TODO @@ -21,8 +21,6 @@ Bugfixes: Cannot add dependency job for unit display-manager.service, ignoring: Unit display-manager.service failed to load: No such file or directory. See system logs and 'systemctl status display-manager.service' for details. -* Substract units in SERVICE_EXITED substate from n_on_console. - Fedora 20: * external: ps should gain colums for slice and machine diff --git a/src/core/service.c b/src/core/service.c index 3da32a1..c0ee114 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1519,6 +1519,22 @@ static void service_set_state(Service *s, ServiceState state) { if (state == SERVICE_EXITED UNIT(s)-manager-n_reloading = 0) unit_destroy_cgroup(UNIT(s)); +/* For remain_after_exit services, let's see if we can release the + * hold on the console, since unit_notify() only does that in case of + * change of state */ +if (state == SERVICE_EXITED s-remain_after_exit +UNIT(s)-manager-n_on_console 0) { +ExecContext *ec = unit_get_exec_context(UNIT(s)); +if (ec exec_context_may_touch_console(ec)) { +Manager *m = UNIT(s)-manager; + +m-n_on_console --; +if (m-n_on_console == 0) +/* unset no_console_output flag, since the console is free */ +m-no_console_output = false; +} +} + if (old_state != state) log_debug_unit(UNIT(s)-id, %s changed %s - %s, UNIT(s)-id, diff --git a/src/core/unit.c b/src/core/unit.c index 15e0a82..d41bc90 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1484,6 +1484,9 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su if (UNIT_IS_INACTIVE_OR_FAILED(ns)) unit_destroy_cgroup(u); +/* Note that this doesn't apply to RemainAfterExit services exiting + * sucessfully, since there's no change of state in that case. Which is + * why it is handled in service_set_state() */ if (UNIT_IS_INACTIVE_OR_FAILED(os) != UNIT_IS_INACTIVE_OR_FAILED(ns)) { ExecContext *ec = unit_get_exec_context(u); if (ec exec_context_may_touch_console(ec)) { -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus: fix duplicate comparisons
On 10/10/13 12:38, Carlos Silva wrote: On Thu, Oct 10, 2013 at 5:14 AM, Tero Roponen tero.ropo...@gmail.com wrote: Testing for y x is the same as testing for x y. -if (y x) +if (x y) snip I thing you forgot to change the signs ;) No, I believe that was the point of the patch. The two tests were the same, first testing (x y), and then (y x). Now it then properly tests for (x y) -j ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Fix timeout when stopping Type=notify service
On 10/01/13 05:08, Lennart Poettering wrote: On Fri, 20.09.13 22:53, Olivier Brunel (j...@jjacky.com) wrote: Since 41efeaec a call to service_unwatch_main_pid() is done from service_set_main_pid(), which is called upon receiving message MAINPID= This had the side effect of not watching pid anymore, and would result in a useless timeout when stopping the service, as the unit wouldn't be identified from the pid, so not marked stopped which would result in systemd thinking this was a timeout. I commited a different fix now in 7400b9d2e99938d17b281d7df43680eade18666e, but it's untested. Could you check if this makes things work? Yes, fixes the issue. Thanks, -j --- I'm not exactly familiar with systemd's internals, so this might not be the correct way to fix this, please correct me if it isn't. src/core/service.c | 8 1 file changed, 8 insertions(+) diff --git a/src/core/service.c b/src/core/service.c index 246a86e..1dccdff 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3388,9 +3388,17 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags) { log_warning_unit(u-id, Failed to parse notification message %s, e); else { +int r; + log_debug_unit(u-id, %s: got %s, u-id, e); service_set_main_pid(s, pid); +r = unit_watch_pid(u, pid); +if (r 0) +/* FIXME: we need to do something here */ +log_warning_unit(u-id, + Failed to watch PID %lu from service %s, + (unsigned long) pid, u-id); } } Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] Resolve /dev/console to the active tty instead of just tty0
When resolving /dev/console one would often get tty0 meaning the active VT. Resolving to the actual tty (e.g. tty1) will notably help on boot when determining whether or not PID1 can output to the console. --- src/shared/util.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/shared/util.c b/src/shared/util.c index 9a075fa..391bc1d 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -3621,6 +3621,16 @@ char *resolve_dev_console(char **active) { else tty = *active; +if (streq(tty, tty0)) { +free(*active); + +/* Get the active VC (e.g. tty1) */ +if (read_one_line_file(/sys/class/tty/tty0/active, active) 0) +*active = strdup(tty0); + +tty = *active; +} + return tty; } -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Fix timeout when stopping Type=notify service
Since 41efeaec a call to service_unwatch_main_pid() is done from service_set_main_pid(), which is called upon receiving message MAINPID= This had the side effect of not watching pid anymore, and would result in a useless timeout when stopping the service, as the unit wouldn't be identified from the pid, so not marked stopped which would result in systemd thinking this was a timeout. --- I'm not exactly familiar with systemd's internals, so this might not be the correct way to fix this, please correct me if it isn't. src/core/service.c | 8 1 file changed, 8 insertions(+) diff --git a/src/core/service.c b/src/core/service.c index 246a86e..1dccdff 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3388,9 +3388,17 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags) { log_warning_unit(u-id, Failed to parse notification message %s, e); else { +int r; + log_debug_unit(u-id, %s: got %s, u-id, e); service_set_main_pid(s, pid); +r = unit_watch_pid(u, pid); +if (r 0) +/* FIXME: we need to do something here */ +log_warning_unit(u-id, + Failed to watch PID %lu from service %s, + (unsigned long) pid, u-id); } } -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 0/3] Fix issues re: visibility of status messages
Hi, I'm running Arch Linux, have been using systemd-204, and recently tried the new 207 release, and I have been having some issues with it. One was that status messages would just stop at some point near the end of the boot process, and also that I wouldn't get any during a shutdown/reboot. It might be useful to note that I don't start a getty on tty1, which is why I expect to see all status messages until default target is reached, even after the getty/login has been started (which happens on tty2). After looking into it, I came up with the following patches to fix the issue. The reason status messages would stop was that the getty was started, and systemd then stopped using the console to avoid collisions w/ gettys. However, as I said I don't have a getty started on tty1 so for me that is a bug, as there's no reason not to keep printing status messages on tty1. The lack of messages on shutdown/reboot was also linked to this, because if no_console_output was set to true during boot, it'd stay there and prevent messages to show up on shutdown. To fix this (in the event it was set to true on boot) a patch simply resets it to false on job_shutdown_magic(), but I'm not exactly sure if that's the right way to do this. FYI I should add that in a similar setup as the one I described, this will not be enough to keep messages on tty1, since fsck's units are now RemainAfterExit (see https://bugs.freedesktop.org/show_bug.cgi?id=66784), which means they're seen by systemd as owning the console (as far as outputing messages there is concerned I mean), and it will therefore stop printing status messages. I'm not sure you want to fix this, as it might be only a cosmetic issue for a small usecase hence not worth the trouble, so I've simply undone it using a .conf file on my end, figured I should mention it though. -jjacky Olivier Brunel (3): Resolve /dev/console to the active tty instead of just tty0 Only disable output on console during boot if needed Fix possible lack of status messages on shutdown/reboot src/core/job.c | 3 +++ src/core/manager.c | 2 +- src/shared/util.c | 10 ++ 3 files changed, 14 insertions(+), 1 deletion(-) -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/3] Only disable output on console during boot if needed
If there are no more jobs on console, no need/we shouldn't disable output. --- src/core/manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/manager.c b/src/core/manager.c index 669af15..a48d711 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1757,7 +1757,7 @@ static int process_event(Manager *m, struct epoll_event *ev) { } case WATCH_IDLE_PIPE: { -m-no_console_output = true; +m-no_console_output = m-n_on_console 0; manager_unwatch_idle_pipe(m); close_idle_pipe(m); -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/3] Fix possible lack of status messages on shutdown/reboot
Since 31a7eb86 the output on console can be disabled to avoid colliding with gettys. However, it could also lead to a lack of messages during shutdown/reboot. --- src/core/job.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/job.c b/src/core/job.c index 85f77e8..fd22184 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1100,6 +1100,9 @@ void job_shutdown_magic(Job *j) { if (detect_container(NULL) 0) return; +/* In case messages on console has been disabled on boot */ +j-unit-manager-no_console_output = false; + asynchronous_sync(); } -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] journald: Log error when failed to get machine-id on start
Can help since the journal requires /etc/machine-id to exists in order to start, and will simply silently exit when it does not. --- Not sure if the behavior is known/expected or a bug, but when e.g. booting a system with a read-only rootfs where /etc/machine-id doesn't exist, the journal would just silently fail (over over) with no indication of why (even at debug log_level), and regardless of the Storage option (i.e. even with Storage=none). Again, this might be expected, so this just adds a log message to clue you in on why it doesn't start. (Might also be a good idea to mention this requirement in systemd-journald(8) ?) src/journal/journald-server.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 9daeb6e..ba211b3 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -897,8 +897,10 @@ static int system_journal_open(Server *s) { char ids[33]; r = sd_id128_get_machine(machine); -if (r 0) +if (r 0) { +log_error(Failed to get machine id: %s, strerror(-r)); return r; +} sd_id128_to_string(machine, ids); @@ -1000,10 +1002,8 @@ int server_flush_to_var(Server *s) { log_debug(Flushing to /var...); r = sd_id128_get_machine(machine); -if (r 0) { -log_error(Failed to get machine id: %s, strerror(-r)); +if (r 0) return r; -} r = sd_journal_open(j, SD_JOURNAL_RUNTIME_ONLY); if (r 0) { -- 1.8.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Highlight ordering cycle deletions
Having unit(s) removed/not started, even if it solved the issue and allowed to boot successfully, should still be considered an error, as something clearly isn't right. This patch elevates the log message from warning to error, and adds a status message to make things more obvious. Signed-off-by: Olivier Brunel i.am.jack.m...@gmail.com --- src/core/transaction.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/transaction.c b/src/core/transaction.c index 4bce942..ee6992a 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -374,7 +374,8 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi if (delete) { -log_warning(Breaking ordering cycle by deleting job %s/%s, delete-unit-id, job_type_to_string(delete-type)); +log_error(Breaking ordering cycle by deleting job %s/%s, delete-unit-id, job_type_to_string(delete-type)); +status_printf(ANSI_HIGHLIGHT_RED_ON SKIP ANSI_HIGHLIGHT_OFF, true, Ordering cycle found, skip %s, unit_description(delete-unit)); transaction_delete_unit(tr, delete-unit); return -EAGAIN; } -- 1.8.0 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Fix starting swap unit on symlink made it unstoppable
Starting a swap unit pointing to (What) a symlink (e.g. /dev/mapper/swap or /dev/disk/by-uuid/...) would have said unit be marked active, follow the one using the actual device (/dev/{dm-1,sda3}), but that new unit would be seen as inactive. Since all requests to stop swap units would follow/redirect to it, and it is seen inactive, nothing would be done (swapoff never called). This is because this unit would be treated twice in swap_process_new_swap, the second call to swap_add_one causing it to eventually be marked inactive. Signed-off-by: Olivier Brunel i.am.jack.m...@gmail.com --- The patch removes the call to udev_device_get_devnode, assuming that device nodes (and not symlinks) are used under /proc/swaps, which seems to be the case. If that's not always true though, a strcmp(device, dn) would then fix the issue, as well as a second one on the symlinks loop (to avoid double calls to swap_add_one on the symink this time). src/core/swap.c | 5 - 1 file changed, 5 deletions(-) diff --git a/src/core/swap.c b/src/core/swap.c index b4f53b7..9394a07 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -424,7 +424,6 @@ static int swap_process_new_swap(Manager *m, const char *device, int prio, bool if (stat(device, st) = 0 S_ISBLK(st.st_mode)) { struct udev_device *d; -const char *dn; struct udev_list_entry *item = NULL, *first = NULL; /* So this is a proper swap device. Create swap units @@ -433,10 +432,6 @@ static int swap_process_new_swap(Manager *m, const char *device, int prio, bool if (!(d = udev_device_new_from_devnum(m-udev, 'b', st.st_rdev))) return -ENOMEM; -dn = udev_device_get_devnode(d); -if (dn) -r = swap_add_one(m, dn, device, prio, false, false, set_flags); - /* Add additional units for all symlinks */ first = udev_device_get_devlinks_list_entry(d); udev_list_entry_foreach(item, first) { -- 1.7.12.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel