Re: [systemd-devel] /usr vs /etc for default distro units enablement
Le 21/11/2014 12:08, Colin Guthrie a écrit : Hello again! Hey, trying to revive the topic :) Didier Roche wrote on 18/11/14 15:40: Le 18/11/2014 15:59, Colin Guthrie a écrit : Hiya, Hey, Didier Roche wrote on 18/11/14 13:58: This would be maybe a nice way for the admin to know what's coming from a distribution default or not. However, let's say I want to ensure that ssh will always be available on my server, I would (even if it's in my server preset) then systemctl enable openssh, no matter whatever future preset updates does (like disable it in the next batch upgrade). For the avoidance of doubt, I believe that running systemctl preset should only ever happen on *first* install, never on upgrade or such like. This also avoids any problems here. (Of course if /etc is nuked, then reapplying the defaults from *.preset should be done!) See my Michael's answer (and my previous one) on the fact that maybe the preset files would be part of multiple packages (to disable certain units), and some being only part of packages not installed by default. Michael's case as well of a unit changing its target on a package upgrade (as a packaging fix, maybe) is valid as well. I think the distro-wide preset usage would be in a very core package that is likely installed very early on (perhaps even the package is required by the one that ships systemctl and thus has to be installed first). If you end up shipping random .preset files with the actual packages containing the units they affect (which isn't recommend as previously noted, but perhaps you still want to do it that way for some special cases) then this too will be fine. I can see some complications here but nothing that isn't manageable. This is a good news. With a shared distro/admin /etc, we have no way to take that fact into account and the next one would follow distro policy, right? Yeah, but that's assuming there *is* a next one. Once things are installed, the user should *not* be surprised by any action being taken without their consent on upgrade. FWIW, it's probably worth considering how you'd handle not changing users current preferences with regards to enabling/disabling things on upgrade. I can see this being quite hard for you roll out with your current suggestions, but you may have this covered already. Actually, this reminds me some issues we had with gconf in the past in changing distribution's default and deciphering what was users current preference or what was distro default behavior. Gnome-panel was copying its whole distro defaults in ~/.gconf. Adding/removing some default layouts and settings from the panel was then unnecessary difficult. Some changes was part of new default behaviors we wanted that the user never interacted with and was desired to be changed (because of some applets getting low maintenance, incompatible with newer technology or duplicates…) As everything was at the same place, it was difficult to know if the current value was set because of the previous default, or if the user explicitly tweaked and then selected it. I see the same issue with the shared /etc: is this unit enabled for that target because of one the preset config, or did the admin run systemctl enable unit explicitly and want to keep it? I think it's ok to change distro defaults on upgrade (will be potentially in major version upgrade of course), not user (admin here) preferences, of course. I would personally disagree with this statement that it's OK to change the distro defaults on upgrade. As an admin, whether I observe some behaviour but do not actively reinforce it (e.g. I see that installing httpd enables it by default and thus my server is working fine, so I don't need to do systemctl enable httpd manually) I now rely on the distro default. If that was to be changed on upgrade (whether package or whole distro), I'd personally be really annoyed! With the goal of being able to reset things (i.e. trashing /etc) if desired, the admin has a pretty good choice to start again with a clean slate after a distro upgrade if they so wish. Otherwise I'd very much expect my system to retain as much state as possible (whether that may have come from a preset or an active choice is, IMO, irrelevant - it's how the system was ultimately configured and what the admin now relies on) over the distro upgrade process. That's what we did in multiple cases in ubuntu in particular. I gave in previous emails some desktop-related email. Another one I didn't mention is when we changed from gdm to lightdm as the default dm, or gnome-panel - unity. If the user selected another dm or another desktop session, we keep user's preference, if they switched and then switched back, we keep user's preference as well. We migrate to our new defaults if there was trace of new user settings. I guess different settings and policy from different distro, but it's clearly something that was always not easy to managed and having a clean /etc will go into that
Re: [systemd-devel] udevd: increase maximum number of children
Hi Robert, On Fri, Nov 28, 2014 at 8:57 AM, Robert Milasan rmila...@suse.com wrote: Hello, since a while back, there was a commit (haven't found it) commit 8cc3f8c0bcd23bb68166cb197a4c541d7621b19c Author: Harald Hoyer har...@redhat.com Date: Mon Mar 25 13:02:05 2013 +0100 udevd.c: set udev children_max according to CPU count Setting children_max according to RAM leads to too much concurrent I/O. which limits the number of children/workers to 8 + num_cpu * 2, which in a normal case like a 4 core/cpu machine is 16 children/workers. This limit is way too low even for a single core VM, that actually means 10 children/workers. I've attached the patch which increased this number to 8 + num_cpu * 256, which is 1032 for a 4 core/cpu machine and 264 for a single core/cpu machine. The reason we have to limit the number of workers is IO, but we used to limit based on RAM, and now we limit based on number of CPUs. Could we not use some scheduler/cgroup tweaks to achieve the correct result instead? Harald, do you have some input on this? Also the patch changes the logging level of 'maximum number of children reached' to an error, this should be visible as an error when the number has been reached. I don't think an error is appropriate here (as nothing actually fails). At most a warning I would say. Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udevd: increase maximum number of children
On Fri, 28 Nov 2014 13:51:04 +0100 Tom Gundersen t...@jklm.no wrote: Also the patch changes the logging level of 'maximum number of children reached' to an error, this should be visible as an error when the number has been reached. I don't think an error is appropriate here (as nothing actually fails). At most a warning I would say. Cheers, Tom This is an error, because the behavior makes the system not work correctly. I don't care about a warning that much, but in this case and reference bug, we see a bug and/or an error which is causes by the small amount of children, or the impossibility of udev daemon to create new children/workers, stopping the queue processing until the number of children is lower the children_max. Anyway, please do as you wish as long as it gets fixed. -- Robert Milasan L3 Support Engineer SUSE Linux (http://www.suse.com) email: rmila...@suse.com GPG fingerprint: B6FE F4A8 0FA3 3040 3402 6FE7 2F64 167C 1909 6D1A ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udevd: increase maximum number of children
On 28.11.2014 13:59, Robert Milasan wrote: On Fri, 28 Nov 2014 13:51:04 +0100 Tom Gundersen t...@jklm.no wrote: Also the patch changes the logging level of 'maximum number of children reached' to an error, this should be visible as an error when the number has been reached. I don't think an error is appropriate here (as nothing actually fails). At most a warning I would say. Cheers, Tom This is an error, because the behavior makes the system not work correctly. I don't care about a warning that much, but in this case and reference bug, we see a bug and/or an error which is causes by the small amount of children, or the impossibility of udev daemon to create new children/workers, stopping the queue processing until the number of children is lower the children_max. Anyway, please do as you wish as long as it gets fixed. This is not true. It only defers the uevent until a worker is available. So logging it as an error is incorrect. It's a debug message. We don't have unlimited resources. You don't do make -j with unlimited make jobs either for a kernel build to get the minimum build time. Having 1024 concurrent blkid's running will slow down a machine significantly, because concurrent I/O over a single path is never good. So, what we see is a lot of I/O errors because of timeouts on huge systems, if the bottleneck is saturated. Ideally we would limit the concurrent I/O, but we can't know (in a simple way) where the bottleneck is. It might be a SAN behind FCoE, or iscsi and the network is the bottleneck. That being said, I don't have a sane solution to satisfy everybody. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udevd: increase maximum number of children
On 28.11.2014 08:57, Robert Milasan wrote: Hello, since a while back, there was a commit (haven't found it) which limits the number of children/workers to 8 + num_cpu * 2, which in a normal case like a 4 core/cpu machine is 16 children/workers. This limit is way too low even for a single core VM, that actually means 10 children/workers. I've attached the patch which increased this number to 8 + num_cpu * 256, which is 1032 for a 4 core/cpu machine and 264 for a single core/cpu machine. Also the patch changes the logging level of 'maximum number of children reached' to an error, this should be visible as an error when the number has been reached. Reference bug: http://bugzilla.opensuse.org/show_bug.cgi?id=907393 I think what we are seeing here is, that module loading saturates the udev workers here. So there are at least 16 modprobes (kmod) running and this hinders further processing of the uevents. In theory we could increase arg_children_max before builtin_kmod() and decrease it again afterwards. CC'ing Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udevd: increase maximum number of children
On Fri, 28 Nov 2014 14:28:31 +0100 Harald Hoyer harald.ho...@gmail.com wrote: I think what we are seeing here is, that module loading saturates the udev workers here. So there are at least 16 modprobes (kmod) running and this hinders further processing of the uevents. In theory we could increase arg_children_max before builtin_kmod() and decrease it again afterwards. CC'ing Kay ___ Ok, I've decreased the arg_children_max number from CPU_COUNT * 256 to CPU_COUNT * 64. You where right, it was a bit too much. -- Robert Milasan L3 Support Engineer SUSE Linux (http://www.suse.com) email: rmila...@suse.com GPG fingerprint: B6FE F4A8 0FA3 3040 3402 6FE7 2F64 167C 1909 6D1A ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Processes running after a service has stopped
The handling of a service with KillMode set to something other than cgroup is a bit confusing (as of systemd 208). Suppose I have a service which has KillMode set to process and it happens to leave some children behind. # systemctl start tester # systemctl status tester tester.service - tester service Loaded: loaded (/etc/systemd/system/tester.service; static) Active: active (running) since Fri 2014-11-28 13:32:40 GMT; 2s ago Main PID: 5690 (tester) CGroup: /system.slice/tester.service ├─5690 /home/ross/tester start └─5691 /home/ross/tester start # systemctl stop tester # systemctl status tester tester.service - tester service Loaded: loaded (/etc/systemd/system/tester.service; static) Active: inactive (dead) Now even though there is still a process running, systemd doesn't indicate this. Furthermore, trying to kill these processes doesn't work because the service is stopped: # systemctl kill --kill-who=all tester.service Failed to issue method call: Unit tester.service is not loaded. Even more confusing, when the service is started again, the existing process reappears: # systemctl start tester # systemctl status tester tester.service - tester service Loaded: loaded (/etc/systemd/system/tester.service; static) Active: active (running) since Fri 2014-11-28 13:36:09 GMT; 7s ago Main PID: 5730 (tester) CGroup: /system.slice/tester.service ├─5691 /home/ross/tester start ├─5730 /home/ross/tester start └─5731 /home/ross/tester start Is there a reason for the way this is handled? Perhaps systemd could show existing processes for a service regardless of the state the service is in? Also, perhaps systemd could allow killing these processes even if the service is stopped? Regards -- Ross Lagerwall ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Native Journal source vs syslog forwarding
Hey Gergely, Gergely Nagy [2014-11-26 13:07 +0100]: Forwarding is enabled by default on Debian, as I wrote in my original mail. I have no control over the default, and I have no desire to argue for changing it. I'm just packaging systemd 217, and will revert the disabled forwarding by default (i. e. retain the behaviour of 215). As a systemd package maintainer I'm also not in a position to unilaterally changing the default and breaking rsyslog and friends. So this requires some coordination indeed (and either way, none of this is a matter for the frozen jessie). So we either need the journal.conf.d/ feature and have journal-pulling sysloggers disable forwarding along the way, or we need to wait until all packaged sysloggers can read from the journal before we turn off forwarding by default. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udevd: increase maximum number of children
On 28.11.2014 14:09, Harald Hoyer wrote: On 28.11.2014 13:59, Robert Milasan wrote: On Fri, 28 Nov 2014 13:51:04 +0100 Tom Gundersen t...@jklm.no wrote: Also the patch changes the logging level of 'maximum number of children reached' to an error, this should be visible as an error when the number has been reached. I don't think an error is appropriate here (as nothing actually fails). At most a warning I would say. Cheers, Tom This is an error, because the behavior makes the system not work correctly. I don't care about a warning that much, but in this case and reference bug, we see a bug and/or an error which is causes by the small amount of children, or the impossibility of udev daemon to create new children/workers, stopping the queue processing until the number of children is lower the children_max. Anyway, please do as you wish as long as it gets fixed. This is not true. It only defers the uevent until a worker is available. So logging it as an error is incorrect. It's a debug message. We don't have unlimited resources. You don't do make -j with unlimited make jobs either for a kernel build to get the minimum build time. Having 1024 concurrent blkid's running will slow down a machine significantly, because concurrent I/O over a single path is never good. So, what we see is a lot of I/O errors because of timeouts on huge systems, if the bottleneck is saturated. Ideally we would limit the concurrent I/O, but we can't know (in a simple way) where the bottleneck is. It might be a SAN behind FCoE, or iscsi and the network is the bottleneck. That being said, I don't have a sane solution to satisfy everybody. Also interesting: udev workers and the time to bring up 2000 LVM LVs on 2 disks. https://plus.google.com/117537647502636167748/posts/eRJFhjLbpta ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Fix hostnamectl exit code
Hello all, while packaging 217 my integration tests for hostnamed yelled at me that hostnamectl fails. Indeed it exits with 1 now even though it succeeds. Trivial patch attached. OK to push? Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) From 416e06379d58a3f5b22cf3bd3ad587cc59b10c3e Mon Sep 17 00:00:00 2001 From: Martin Pitt martin.p...@ubuntu.com Date: Fri, 28 Nov 2014 15:38:05 +0100 Subject: [PATCH] hostnamectl: Exit with zero on success In show_all_names(), bus_map_all_properties() returns 1 on success which is then used as the return code of show_all_names() and eventually main(). As for a shell command nonzero signals failure, and the documentation also says 0 on success, fix show_all_names() to return 0 on success just like show_one_name() and the other dispatchers. --- src/hostname/hostnamectl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index e487369..acd107d 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -216,7 +216,7 @@ fail: free(info.virtualization); free(info.architecture); -return r; +return r 0 ? r : 0; } static int show_status(sd_bus *bus, char **args, unsigned n) { -- 2.1.3 signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Fix hostnamectl exit code
Hi On Fri, Nov 28, 2014 at 3:43 PM, Martin Pitt martin.p...@ubuntu.com wrote: Hello all, while packaging 217 my integration tests for hostnamed yelled at me that hostnamectl fails. Indeed it exits with 1 now even though it succeeds. Trivial patch attached. OK to push? Why not fix all those other occurrences with one fix by changing hostnamectl_main() the last line from: return r 0 ? EXIT_FAILURE : r; to return r 0 ? EXIT_FAILURE : 0; Usually, =0 means success, 0 failure, in systemd. We should not return !=0 from main() if the return value wasn't negative. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-cgroups-agent not working in containers
Am 28.11.2014 um 06:33 schrieb Martin Pitt: Hello all, Cameron Norman [2014-11-27 12:26 -0800]: On Wed, Nov 26, 2014 at 1:29 PM, Richard Weinberger rich...@nod.at wrote: Hi! I run a Linux container setup with openSUSE 13.1/2 as guest distro. After some time containers slow down. An investigation showed that the containers slow down because a lot of stale user sessions slow down almost all systemd tools, mostly systemctl. loginctl reports many thousand sessions. All in state closing. This sounds similar to an issue that systemd-shim in Debian had. Martin Pitt (helps to maintain systemd in Debian) fixed that issue; he may have some ideas here. I CC'd him. The problem with systemd-shim under sysvinit or upstart was that shim didn't set a cgroup release agent like systemd itself does. Thus the cgroups were never cleaned up after all the session processes died. (See 1.4 on https://www.kernel.org/doc/Documentation/cgroups/cgroups.txt for details) I don't think that SUSE uses systemd-shim, I take it in that setup you are running systemd proper on both the host and the guest? Then I suggest checking the cgroups that correspond to the closing sessions in the container, i. e. /sys/fs/cgroup/systemd/.../session-XX.scope/tasks. If there are still processes in it, logind is merely waiting for them to exit (or set KillUserProcesses in logind.conf). If they are empty, check that /sys/fs/cgroup/systemd/.../session-XX.scope/notify_on_release is 1 and that /sys/fs/cgroup/systemd/release_agent is set? The problem is that within the container the release agent is not executed. It is executed on the host side. Lennart, how is this supposed to work? Is the theory of operation that the host systemd sends org.freedesktop.systemd1.Agent Released via dbus into the guest? The guests systemd definitely does not receive such a signal. Thanks, //richard ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Fix hostnamectl exit code
Hey David, David Herrmann [2014-11-28 15:49 +0100]: Why not fix all those other occurrences with one fix by changing hostnamectl_main() the last line from: return r 0 ? EXIT_FAILURE : r; to return r 0 ? EXIT_FAILURE : 0; Usually, =0 means success, 0 failure, in systemd. We should not return !=0 from main() if the return value wasn't negative. Yeah, that's even more robust and guards against similar errors in the future. Updated patch attached. Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) From c8c55d0aa621ad2998b15ff461b20b3fcb1361b0 Mon Sep 17 00:00:00 2001 From: Martin Pitt martin.p...@ubuntu.com Date: Fri, 28 Nov 2014 15:38:05 +0100 Subject: [PATCH] hostnamectl: Exit with zero on success In show_all_names(), bus_map_all_properties() returns 1 on success which is then used as the return code of show_all_names() and eventually main(). Exit with zero in main() on all nonnegative results to guard against similar errors. --- src/hostname/hostnamectl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index e487369..ff4e9c9 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -536,5 +536,5 @@ int main(int argc, char *argv[]) { r = hostnamectl_main(bus, argc, argv); finish: -return r 0 ? EXIT_FAILURE : r; +return r 0 ? EXIT_FAILURE : EXIT_SUCCESS; } -- 2.1.3 signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v3] build-sys: configure the list of system users, files and directories
Choose which system users defined in sysusers.d/systemd.conf and files or directories in tmpfiles.d/systemd.conf, should be provided depending on comile-time configuration. --- Makefile.am| 4 configure.ac | 2 ++ sysusers.d/.gitignore | 1 + sysusers.d/systemd.conf| 12 sysusers.d/systemd.conf.m4 | 20 tmpfiles.d/.gitignore | 3 ++- tmpfiles.d/systemd.conf| 32 tmpfiles.d/systemd.conf.m4 | 34 ++ 8 files changed, 63 insertions(+), 45 deletions(-) delete mode 100644 sysusers.d/systemd.conf create mode 100644 sysusers.d/systemd.conf.m4 delete mode 100644 tmpfiles.d/systemd.conf create mode 100644 tmpfiles.d/systemd.conf.m4 diff --git a/Makefile.am b/Makefile.am index 7ab1dea..fdd14e4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5820,6 +5820,10 @@ src/%: src/%.m4 $(AM_V_at)$(MKDIR_P) $(dir $@) $(AM_V_M4)$(M4) -P $(M4_DEFINES) $ $@ +sysusers.d/%: sysusers.d/%.m4 + $(AM_V_at)$(MKDIR_P) $(dir $@) + $(AM_V_M4)$(M4) -P $(M4_DEFINES) $ $@ + tmpfiles.d/%: tmpfiles.d/%.m4 $(AM_V_at)$(MKDIR_P) $(dir $@) $(AM_V_M4)$(M4) -P $(M4_DEFINES) $ $@ diff --git a/configure.ac b/configure.ac index a4e91e3..6e0b5f3 100644 --- a/configure.ac +++ b/configure.ac @@ -975,6 +975,7 @@ have_timesyncd=no AC_ARG_ENABLE(timesyncd, AS_HELP_STRING([--disable-timesyncd], [disable timesync daemon])) if test x$enable_timesyncd != xno; then have_timesyncd=yes +M4_DEFINES=$M4_DEFINES -DENABLE_TIMESYNCD fi AM_CONDITIONAL(ENABLE_TIMESYNCD, [test $have_timesyncd = yes]) @@ -1064,6 +1065,7 @@ AC_ARG_ENABLE(networkd, AS_HELP_STRING([--disable-networkd], [disable networkd]) AS_IF([test x$enable_networkd != xno], [ AC_DEFINE(ENABLE_NETWORKD, 1, [Define if networkd support is to be enabled]) have_networkd=yes +M4_DEFINES=$M4_DEFINES -DENABLE_NETWORKD ]) AM_CONDITIONAL(ENABLE_NETWORKD, [test x$have_networkd = xyes]) diff --git a/sysusers.d/.gitignore b/sysusers.d/.gitignore index f7957a9..bb3aaaf 100644 --- a/sysusers.d/.gitignore +++ b/sysusers.d/.gitignore @@ -1 +1,2 @@ /basic.conf +/systemd.conf diff --git a/sysusers.d/systemd.conf b/sysusers.d/systemd.conf deleted file mode 100644 index 95437b8..000 --- a/sysusers.d/systemd.conf +++ /dev/null @@ -1,12 +0,0 @@ -# This file is part of systemd. -# -# 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. - -g systemd-journal - - -u systemd-bus-proxy - systemd Bus Proxy -u systemd-network - systemd Network Management -u systemd-resolve - systemd Resolver -u systemd-timesync - systemd Time Synchronization diff --git a/sysusers.d/systemd.conf.m4 b/sysusers.d/systemd.conf.m4 new file mode 100644 index 000..23175de --- /dev/null +++ b/sysusers.d/systemd.conf.m4 @@ -0,0 +1,20 @@ +# This file is part of systemd. +# +# 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. + +g systemd-journal - - +m4_ifdef(`ENABLE_KDBUS', +u systemd-bus-proxy - systemd Bus Proxy +)m4_dnl +m4_ifdef(`ENABLE_NETWORKD', +u systemd-network - systemd Network Management +)m4_dnl +m4_ifdef(`ENABLE_RESOLVED', +u systemd-resolve - systemd Resolver +)m4_dnl +m4_ifdef(`ENABLE_TIMESYNCD', +u systemd-timesync - systemd Time Synchronization +)m4_dnl diff --git a/tmpfiles.d/.gitignore b/tmpfiles.d/.gitignore index eb32315..4f0ecaa 100644 --- a/tmpfiles.d/.gitignore +++ b/tmpfiles.d/.gitignore @@ -1 +1,2 @@ -etc.conf +/etc.conf +/systemd.conf diff --git a/tmpfiles.d/systemd.conf b/tmpfiles.d/systemd.conf deleted file mode 100644 index 9ca5ad2..000 --- a/tmpfiles.d/systemd.conf +++ /dev/null @@ -1,32 +0,0 @@ -# This file is part of systemd. -# -# 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. - -# See tmpfiles.d(5) for details - -d /run/user 0755 root root - -F! /run/utmp 0664 root utmp - - -d /run/systemd/ask-password 0755 root root - -d /run/systemd/seats 0755 root root - -d /run/systemd/sessions 0755 root root - -d /run/systemd/users 0755 root root - -d /run/systemd/machines 0755 root root - -d /run/systemd/shutdown 0755 root root - -d /run/systemd/netif 0755 systemd-network systemd-network - -d /run/systemd/netif/links 0755 systemd-network systemd-network - -d /run/systemd/netif/leases 0755 systemd-network systemd-network - - -d /run/log
Re: [systemd-devel] systemctl isolate foo.service expands to foo.service.target
How does this interact with snapshots? While I was looking at man systemctl it seems that one uses isolate to restore to a previous snapshot: Snapshot Commands snapshot [NAME] Create a snapshot. If a snapshot name is specified, the new snapshot will be named after it. If none is specified, an automatic snapshot name is generated. In either case, the snapshot name used is printed to standard output, unless --quiet is specified. A snapshot refers to a saved state of the systemd manager. It is implemented itself as a unit that is generated dynamically with this command and has dependencies on all units active at the time. At a later time, the user may return to this state by using the isolate command on the snapshot unit. Snapshots are only useful for saving and restoring which units are running or are stopped, they do not save/restore any other state. Snapshots are dynamic and lost on reboot. I created a named snapshot: $ sudo systemctl snapshot derpy derpy.snapshot I tried restoring the named snapshot, but the mandatory mangling to .target got in the way: $ sudo systemctl isolate derpy.snapshot Failed to start derpy.snapshot.target: Operation refused, unit may not be isolated. Results were the same with an automatically named snapshot. Did I misunderstand the man page and there's some other way to restore a snapshot? Thanks, ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Fix hostnamectl exit code
Hi On Fri, Nov 28, 2014 at 3:54 PM, Martin Pitt martin.p...@ubuntu.com wrote: Hey David, David Herrmann [2014-11-28 15:49 +0100]: Why not fix all those other occurrences with one fix by changing hostnamectl_main() the last line from: return r 0 ? EXIT_FAILURE : r; to return r 0 ? EXIT_FAILURE : 0; Usually, =0 means success, 0 failure, in systemd. We should not return !=0 from main() if the return value wasn't negative. Yeah, that's even more robust and guards against similar errors in the future. Updated patch attached. Looks good, pushed! Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl isolate foo.service expands to foo.service.target
On Fri, Nov 28, 2014 at 10:40:31AM -0500, Chris Atkinson wrote: I created a named snapshot: $ sudo systemctl snapshot derpy derpy.snapshot I tried restoring the named snapshot, but the mandatory mangling to .target got in the way: $ sudo systemctl isolate derpy.snapshot Failed to start derpy.snapshot.target: Operation refused, unit may not be isolated. This was an unintended side effect. I will push a commit to only append the suffix in there isn't one already. If I say 'systemctl set-default goo.device' I want to get an error, not get promoted to goo.device.service. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl isolate foo.service expands to foo.service.target
The commit (5e03c6e3b517286bbd65b48d88f60e5b83721894) seems to be having some side effects. When I attempt to query status, stop or start a service I get the following: $ sudo systemctl stop transmission Assertion 'suffix' failed at src/shared/unit-name.c:515, function unit_name_mangle_with_suffix(). Aborting. journalctl shows the following coredump: Process 4687 (systemctl) of user 0 dumped core. Stack trace of thread 4687: #0 0x7fb3bf640a97 raise (libc.so.6) #1 0x7fb3bf641e6a abort (libc.so.6) #2 0x7fb3c051b462 log_assert_failed (systemctl) #3 0x7fb3c052e4ef unit_name_mangle_with_suffix (systemctl) #4 0x7fb3c050d12d expand_names.lto_priv.398 (systemctl) #5 0x7fb3c05185cc check_unit_generic (systemctl) #6 0x7fb3c050abe6 main (systemctl) #7 0x7fb3bf62d040 __libc_start_main (libc.so.6) #8 0x7fb3c050b64c _start (systemctl) Attempts to isolate a snapshot fail silently with the same coredump. Attempts to isolate to default.target worked but attempts to isolate back to a user-defined .target failed. systemd was compiled with glibc 2.20 and the following flags: --libexecdir=/usr/lib \ --localstatedir=/var \ --sysconfdir=/etc \ --enable-introspection \ --enable-gtk-doc \ --enable-compat-libs \ --disable-audit \ --disable-ima \ --disable-multi-seat-x \ --disable-smack \ --with-sysvinit-path= \ --with-sysvrcnd-path= \ --with-firmware-path=/usr/lib/firmware/updates:/usr/lib/firmware\ --with-ntp-servers=${timeservers[*]} Please let me know if there are any items you'd like me to test or additional information you would find useful. Regards, ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl isolate foo.service expands to foo.service.target
On Fri, Nov 28, 2014 at 01:09:18PM -0500, Chris Atkinson wrote: The commit (5e03c6e3b517286bbd65b48d88f60e5b83721894) seems to be having some side effects. When I attempt to query status, stop or start a service I get the following: $ sudo systemctl stop transmission Assertion 'suffix' failed at src/shared/unit-name.c:515, function unit_name_mangle_with_suffix(). Aborting. Yeah, I noticed that too :) It was a last minute cleanup I added to the patch, it's now reverted. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Processes running after a service has stopped
On Fri, 28.11.14 13:42, Ross Lagerwall (rosslagerw...@gmail.com) wrote: The handling of a service with KillMode set to something other than cgroup is a bit confusing (as of systemd 208). Hmm, could you test this with newer systemd please? 208 is already quite old. Where (in terms of: which cgroup?) does systemd-cgls show the left-over processes? We should show the cgroup contents regardless of the state of a service actually, nothing should be hidden there. If things are hidden just because of the service state then this would be a bug. If you can reproduce it with 217 or so that would be great! Thanks! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl isolate foo.service expands to foo.service.target
Commit e80733be33e52d8ab2f1ae845326d39c600f5612 seems to have done the trick. I'm able to start, stop and status services, isolate .target and .snapshot files and a filename with no extension following isolate is treated as a .target, all of which seems right. The systemctl man page will still need tweaking; I will do so unless you object. Regards, ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl isolate foo.service expands to foo.service.target
On Fri, Nov 28, 2014 at 02:02:24PM -0500, Chris Atkinson wrote: Commit e80733be33e52d8ab2f1ae845326d39c600f5612 seems to have done the trick. I'm able to start, stop and status services, isolate .target and .snapshot files and a filename with no extension following isolate is treated as a .target, all of which seems right. The systemctl man page will still need tweaking; I will do so unless you object. No, of course not, please go ahead. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udevadm hwdb: discard extra leading whitespaces in hwdb
On Thu, Nov 27, 2014 at 07:21:55AM +0100, Zbigniew Jędrzejewski-Szmek wrote: On Wed, Nov 26, 2014 at 09:55:10PM -0800, Greg KH wrote: On Thu, Nov 27, 2014 at 03:19:44PM +1000, Peter Hutterer wrote: Currently a property in the form of FOO=bar is stored as FOO=bar, i.e. the property name contains a leading space. That's quite hard to spot. This patch discards all extra whitespaces but the first one which is required by libudev's hwdb_add_property. --- src/udev/udevadm-hwdb.c | 4 1 file changed, 4 insertions(+) diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c index 3ca755e..dcc6e0f 100644 --- a/src/udev/udevadm-hwdb.c +++ b/src/udev/udevadm-hwdb.c @@ -428,6 +428,10 @@ static int insert_data(struct trie *trie, struct udev_list *match_list, value[0] = '\0'; value++; +/* libudev requires properties to start with a space */ +while(line[0] != '\0' isblank(line[1])) Shouldn't this be while (isblank(line[0]) isblank(line[1])) ? Otherwise stuff like x yyy=111 might slip through. I pushed the patch now with the above change (and style fixes ;)). Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udevadm hwdb: discard extra leading whitespaces in hwdb
On Thu, Nov 27, 2014 at 12:31:33PM +0100, Lennart Poettering wrote: On Thu, 27.11.14 15:19, Peter Hutterer (peter.hutte...@who-t.net) wrote: Currently a property in the form of FOO=bar is stored as FOO=bar, i.e. the property name contains a leading space. That's quite hard to spot. This patch discards all extra whitespaces but the first one which is required by libudev's hwdb_add_property. --- src/udev/udevadm-hwdb.c | 4 1 file changed, 4 insertions(+) diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c index 3ca755e..dcc6e0f 100644 --- a/src/udev/udevadm-hwdb.c +++ b/src/udev/udevadm-hwdb.c @@ -428,6 +428,10 @@ static int insert_data(struct trie *trie, struct udev_list *match_list, value[0] = '\0'; value++; +/* libudev requires properties to start with a space */ +while(line[0] != '\0' isblank(line[1])) +line++; + Please use this instead: line += strspn(line, WHITESPACE); We don't want to skip all of it, so an explicit loop seems better. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Processes running after a service has stopped
On Fri, Nov 28, 2014 at 07:53:33PM +0100, Lennart Poettering wrote: On Fri, 28.11.14 13:42, Ross Lagerwall (rosslagerw...@gmail.com) wrote: The handling of a service with KillMode set to something other than cgroup is a bit confusing (as of systemd 208). Hmm, could you test this with newer systemd please? 208 is already quite old. Where (in terms of: which cgroup?) does systemd-cgls show the left-over processes? In it's own cgroup, as would normally be the case: │ ├─tester.service │ │ └─24709 /home/ross/Downloads/tester start We should show the cgroup contents regardless of the state of a service actually, nothing should be hidden there. If things are hidden just because of the service state then this would be a bug. If you can reproduce it with 217 or so that would be great! The same behavior seems to occur with 217 (on Arch): # systemctl start tester.service # systemctl status tester.service ● tester.service - Tester service Loaded: loaded (/etc/systemd/system/tester.service; static) Active: active (running) since Fri 2014-11-28 19:46:21 GMT; 4s ago Main PID: 25067 (tester) CGroup: /system.slice/tester.service ├─25067 /home/ross/Downloads/tester start └─25068 /home/ross/Downloads/tester start # systemctl stop tester # systemctl status tester.service ● tester.service - Tester service Loaded: loaded (/etc/systemd/system/tester.service; static) Active: inactive (dead) # ps aux | grep tester root 25068 0.0 0.0 404876 ?S19:46 0:00 /home/ross/Downloads/tester start # systemctl start tester.service # systemctl status tester.service ● tester.service - Tester service Loaded: loaded (/etc/systemd/system/tester.service; static) Active: active (running) since Fri 2014-11-28 19:50:58 GMT; 2s ago Main PID: 25148 (tester) CGroup: /system.slice/tester.service ├─25068 /home/ross/Downloads/tester start -- the left over process! ├─25148 /home/ross/Downloads/tester start └─25149 /home/ross/Downloads/tester start With 217, running systemctl kill --kill-who=all -s KILL tester.service doesn't fail, but it doesn't seem to do anything either. Thanks, -- Ross Lagerwall ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote: On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote: This adds auto detection for iSCSI and some FCoE drivers and treats mounts to file-systems on those devices as remote-fs. Signed-off-by: Chris Leech cle...@redhat.com No need for this. I now pushed patches 1-4 with some small changes here and there. Since libmount is not optional, I removed if from the version string. This patch I didn't push: this seems like something that would be better done through udev rules, by setting some tags. Then systemd could simply check for the presence of a tag on the device without having any special knowledge about iscsi and firends. Honestly, I am really not sure I like this patch. One one hand we now have a hard dep on libmount. Which I figure is mostly OK. However, what I find really weird about this is that even though libmount is supposed to abstract access to the utab away, it doesn't sufficiently: we still hardcode the utab path now so that we can watch it. I mean, either we use an abstracted interface or we don't. THis really smells to me as either libmount should provide some form of inotify iface to utab, or we should parse utab directly. If libmount is the only and official API for utab, then we should that. If the utab file however is API too, then we can well go ahead and parse it directly. (Karel, could you please comment on this?) THe new code looks also buggy to me. For example, am I missing something or is nobody creating /run/mount? I mean, we need to make sure that it exists before we can install an inotify watch on it. Then, the patch uses strcmp() and ints as bools, and things, misses error checking on unit_add_dependency_by_name(), defines its own desctruors instead of using DEFINE_TRIVIAL_CLEANUP. Also, it leaves the cescape() calls in for what we read from libmount, which makes a lot of alarm bells ring in my head: doesn't libmount do that on its own? Anyway, this patch really looks like it should have gone through a couple of more revisions before it got merged. And it raises a couple of general questions regarding utab and libmount and what is API and what isn't. (Sorry that I didn't find time to review this more quickly.) Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Documentation changes to reflect that isolate is for target and snapshot only and that no extension filenames default to .target.
See http://lists.freedesktop.org/archives/systemd-devel/2014-November/025644.html et seq. --- man/systemctl.xml | 12 src/systemctl/systemctl.c | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index 4a7abab..e6f438c 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -705,7 +705,10 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service listitem paraStart the unit specified on the command line and its -dependencies and stop all others./para +dependencies and stop all others. This applies only to +dependencies and stop all others. This applies only to + literal.target/literal will be assumed./para traditional init system. The commandisolate/command @@ -713,7 +716,8 @@ -paraNote that this is allowed only on units where +paraNote that for literaltarget/literal units this is +allowed only on units where optionAllowIsolate=/option is enabled. See citerefentryrefentrytitlesystemd.unit/refentrytitlemanvolnum5/manvolnum/citerefentry for details./para @@ -1597,9 +1601,9 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service programlisting# systemctl start sshd/programlisting and programlisting# systemctl start sshd.service/programlisting are equivalent, as are - programlisting# systemctl isolate snapshot-11/programlisting + programlisting# systemctl isolate default/programlisting and - programlisting# systemctl isolate snapshot-11.snapshot/programlisting + programlisting# systemctl isolate default.target/programlisting Note that (absolute) paths to device nodes are automatically converted to device unit names, and other (absolute) paths to mount unit names. diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index ffb97df..2176f15 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5770,7 +5770,7 @@ static void systemctl_help(void) { otherwise start or restart\n reload-or-try-restart NAME... Reload one or more units if possible,\n otherwise restart if active\n - isolate NAMEStart one unit and stop all others\n + isolate NAMEStart one target/snapshot unit and stop all others\n kill NAME...Send signal to processes of a unit\n is-active PATTERN...Check whether units are active\n is-failed PATTERN...Check whether units are failed\n -- 2.1.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
2014-10-29 16:22 GMT+01:00 Ronny Chevalier chevalier.ro...@gmail.com: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- Is it ok if I apply this patch, or is there other remarks ? (Minus the recent log_error_errno changes) lookup_paths_init does not concatenate root_dir, so I added a path_join with arg_root TODO | 4 +- man/less-variables.xml| 4 +- man/systemctl.xml | 64 +- src/systemctl/systemctl.c | 525 +- 4 files changed, 587 insertions(+), 10 deletions(-) diff --git a/TODO b/TODO index abe89b7..1cbedd4 100644 --- a/TODO +++ b/TODO @@ -84,7 +84,7 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them +* systemctl edit: add commented help text to the end, like git commit * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... @@ -776,7 +776,7 @@ External: * zsh shell completion: - command verb -TAB should complete options, but currently does not - - systemctl add-wants,add-requires + - systemctl add-wants,add-requires, edit Regularly: diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..0fb4d7f 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -6,7 +6,7 @@ titleEnvironment/title variablelist class='environment-variables' -varlistentry +varlistentry id='pager' termvarname$SYSTEMD_PAGER/varname/term listitemparaPager to use when @@ -17,7 +17,7 @@ option--no-pager/option./para/listitem /varlistentry -varlistentry +varlistentry id='less' termvarname$SYSTEMD_LESS/varname/term listitemparaOverride the default diff --git a/man/systemctl.xml b/man/systemctl.xml index 7cbaa6c..26f5235 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -465,7 +465,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. listitem paraWhen used with commandenable/command, - commanddisable/command, + commanddisable/command, commandedit/command, (and related commands), make changes only temporarily, so that they are lost on the next reboot. This will have the effect that changes are not made in subdirectories of @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service filenamedefault.target/filename to the given unit./para /listitem /varlistentry + +varlistentry + termcommandedit replaceableNAME/replaceable.../command/term + + listitem +paraEdit a drop-in snippet or a whole replacement file if +option--full/option is specified, to extend or override the +specified unit./para + +paraDepending on whether option--system/option (the default), +option--user/option, or option--global/option is specified, +this create a drop-in file for each units either for the system, +for the calling user or for all futures logins of all users. Then +the editor (see section Environment below) is invoked on temporary +files which will be saved as their corresponding files if the editor +exited successfully./para + +paraIf option--full/option is specified, this will copy the +original units instead of creating drop-in files./para + +paraIf option--runtime/option is specified, the changes will +be made temporarily in filename/run/filename and they will be +lost on the next reboot./para + +paraIf the temporary file is empty the modification of the related +unit is canceled/para + +paraAfter the units have been edited, the systemd configuration is +reloaded (in a way that is equivalent to commanddaemon-reload/command), +but it does not restart or reload the units./para + +paraNote that this command cannot be used to
Re: [systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
On Fri, Nov 28, 2014 at 09:27:43PM +0100, Lennart Poettering wrote: On Fri, 28.11.14 20:50, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote: On Sun, Nov 23, 2014 at 08:33:41PM -0800, Chris Leech wrote: This adds auto detection for iSCSI and some FCoE drivers and treats mounts to file-systems on those devices as remote-fs. Signed-off-by: Chris Leech cle...@redhat.com No need for this. I now pushed patches 1-4 with some small changes here and there. Since libmount is not optional, I removed if from the version string. This patch I didn't push: this seems like something that would be better done through udev rules, by setting some tags. Then systemd could simply check for the presence of a tag on the device without having any special knowledge about iscsi and firends. Honestly, I am really not sure I like this patch. One one hand we now have a hard dep on libmount. Which I figure is mostly OK. However, what I find really weird about this is that even though libmount is supposed to abstract access to the utab away, it doesn't sufficiently: we still hardcode the utab path now so that we can watch it. I mean, either we use an abstracted interface or we don't. THis really smells to me as either libmount should provide some form of inotify iface to utab, or we should parse utab directly. If libmount is the only and official API for utab, then we should that. If the utab file however is API too, then we can well go ahead and parse it directly. (Karel, could you please comment on this?) THe new code looks also buggy to me. For example, am I missing something or is nobody creating /run/mount? I mean, we need to make sure that it exists before we can install an inotify watch on it. This is done in a my follow up patch. Then, the patch uses strcmp() and ints as bools, and things, misses error checking on unit_add_dependency_by_name(), defines its own desctruors instead of using DEFINE_TRIVIAL_CLEANUP. I fixed one place in a follow up patch, but I might have missed some. As for the DEFINE_TRIVIAL_CLEANUP: yeah, I looked at that, but thought that since mnt_free_* accept NULLs the extra check added by DEFINE_TRIVIAL_CLEANUP would be unnecessary. Looking at it again, this kind of micro-opt is stupid. I'll remove those custom functions in favour of DEFINE_TRIVIAL_CLEANUP. Also, it leaves the cescape() calls in for what we read from libmount, which makes a lot of alarm bells ring in my head: doesn't libmount do that on its own? Anyway, this patch really looks like it should have gone through a couple of more revisions before it got merged. And it raises a couple of general questions regarding utab and libmount and what is API and what isn't. (Sorry that I didn't find time to review this more quickly.) Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Wed, Oct 29, 2014 at 04:22:02PM +0100, Ronny Chevalier wrote: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- lookup_paths_init does not concatenate root_dir, so I added a path_join with arg_root TODO | 4 +- man/less-variables.xml| 4 +- man/systemctl.xml | 64 +- src/systemctl/systemctl.c | 525 +- 4 files changed, 587 insertions(+), 10 deletions(-) diff --git a/TODO b/TODO index abe89b7..1cbedd4 100644 --- a/TODO +++ b/TODO @@ -84,7 +84,7 @@ Features: * systemctl: if it fails, show log output? -* maybe add systemctl edit that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them +* systemctl edit: add commented help text to the end, like git commit * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true... @@ -776,7 +776,7 @@ External: * zsh shell completion: - command verb -TAB should complete options, but currently does not - - systemctl add-wants,add-requires + - systemctl add-wants,add-requires, edit Regularly: diff --git a/man/less-variables.xml b/man/less-variables.xml index 09cbd42..0fb4d7f 100644 --- a/man/less-variables.xml +++ b/man/less-variables.xml @@ -6,7 +6,7 @@ titleEnvironment/title variablelist class='environment-variables' -varlistentry +varlistentry id='pager' termvarname$SYSTEMD_PAGER/varname/term listitemparaPager to use when @@ -17,7 +17,7 @@ option--no-pager/option./para/listitem /varlistentry -varlistentry +varlistentry id='less' termvarname$SYSTEMD_LESS/varname/term listitemparaOverride the default diff --git a/man/systemctl.xml b/man/systemctl.xml index 7cbaa6c..26f5235 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -465,7 +465,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. listitem paraWhen used with commandenable/command, - commanddisable/command, + commanddisable/command, commandedit/command, (and related commands), make changes only temporarily, so that they are lost on the next reboot. This will have the effect that changes are not made in subdirectories of @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service filenamedefault.target/filename to the given unit./para /listitem /varlistentry + +varlistentry + termcommandedit replaceableNAME/replaceable.../command/term + + listitem +paraEdit a drop-in snippet or a whole replacement file if +option--full/option is specified, to extend or override the +specified unit./para + +paraDepending on whether option--system/option (the default), +option--user/option, or option--global/option is specified, +this create a drop-in file for each units either for the system, creates each unit +for the calling user or for all futures logins of all users. Then , +the editor (see section Environment below) is invoked on temporary see the Environment section below +files which will be saved as their corresponding files if the editor which will be written to the real location if the editor exits successfully. +exited successfully./para + +paraIf option--full/option is specified, this will copy the +original units instead of creating drop-in files./para + +paraIf option--runtime/option is specified, the changes will +be made temporarily in filename/run/filename and they will be +lost on the next reboot./para + +paraIf the temporary file is empty the modification of the related empty upon exit +unit is canceled/para . + +paraAfter the units have been edited, the systemd configuration is drop the 'the' in 'the systemd configuration' +reloaded (in a
Re: [systemd-devel] [PATCH] systemctl: add edit verb
On Fri, Nov 28, 2014 at 09:48:55PM +0100, Ronny Chevalier wrote: 2014-10-29 16:22 GMT+01:00 Ronny Chevalier chevalier.ro...@gmail.com: It helps editing units by either creating a drop-in file, like /etc/systemd/system/my.service.d/override.conf, or by copying the original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full option is specified. It invokes an editor on temporary files related to the unit files and if the editor exited successfully, then it renames the temporary files to their original names (e.g. my.service or override.conf) and daemon-reload is invoked. If the temporary file is empty the modification is canceled. See https://bugzilla.redhat.com/show_bug.cgi?id=906824 --- Is it ok if I apply this patch, or is there other remarks ? (Minus the recent log_error_errno changes) Yes please go ahead! Your patch was one of the big things on my TODO list, it's great that you that your account is finally created and you can push it yourself. I replied to the last version of the patch with some comments. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Documentation changes to reflect that isolate is for target and snapshot only and that no extension filenames default to .target.
On Fri, Nov 28, 2014 at 03:37:25PM -0500, Chris Atkinson wrote: See http://lists.freedesktop.org/archives/systemd-devel/2014-November/025644.html et seq. Hi, your patch is line-wrapper. Please resend it using git send-email or as an attachment. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [RFC PATCH] proc, pidns: Add highpid
Pid reuse is common, which means that it's difficult or impossible to read information about a pid from /proc without races. This introduces a second number associated with each (task, pidns) pair called highpid. Highpid is a 64-bit number, and, barring extremely unlikely circumstances or outright error, a (highpid, pid) will never be reused. With just this change, a program can open /proc/PID/status, read the Highpid field, and confirm that it has the expected value. If the pid has been reused, then highpid will be different. The initial implementation is straightforward: highpid is simply a 64-bit counter. If a high-end system can fork every 3 ns (which would be amazing, given that just allocating a pid requires at atomic operation), it would take well over 1000 years for highpid to wrap. For CRIU's benefit, the next highpid can be set by a privileged user. NB: The sysctl stuff only works on 64-bit systems. If the approach looks good, I'll fix that somehow. Signed-off-by: Andy Lutomirski l...@amacapital.net --- If this goes in, there's plenty of room to add new interfaces to make this more useful. For example, we could add a fancier tgkill that adds and validates hightgid and highpid, and we might want to add a syscall to read one's own hightgid and highpid. These would be quite useful for pidfiles. David, would this be useful for kdbus? CRIU people: will this be unduly difficult to support in CRIU? If you all think this is a good idea, I'll fix the sysctl stuff and document it. It might also be worth adding Hightgid to status. fs/proc/array.c | 5 - include/linux/pid.h | 2 ++ include/linux/pid_namespace.h | 1 + kernel/pid.c | 19 +++ kernel/pid_namespace.c| 22 ++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index cd3653e4f35c..f1e0e69d19f9 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, int g; struct fdtable *fdt = NULL; const struct cred *cred; + const struct upid *upid; pid_t ppid, tpid; rcu_read_lock(); @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, if (tracer) tpid = task_pid_nr_ns(tracer, ns); } + upid = pid_upid_ns(pid, ns); cred = get_task_cred(p); seq_printf(m, State:\t%s\n Tgid:\t%d\n Ngid:\t%d\n Pid:\t%d\n + Highpid:\t%llu\n PPid:\t%d\n TracerPid:\t%d\n Uid:\t%d\t%d\t%d\t%d\n @@ -183,7 +186,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, get_task_state(p), task_tgid_nr_ns(p, ns), task_numa_group_id(p), - pid_nr_ns(pid, ns), + upid ? upid-nr : 0, upid ? upid-highnr : 0, ppid, tpid, from_kuid_munged(user_ns, cred-uid), from_kuid_munged(user_ns, cred-euid), diff --git a/include/linux/pid.h b/include/linux/pid.h index 23705a53abba..ece70b64d04c 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -51,6 +51,7 @@ struct upid { /* Try to keep pid_chain in the same cacheline as nr for find_vpid */ int nr; struct pid_namespace *ns; + u64 highnr; struct hlist_node pid_chain; }; @@ -170,6 +171,7 @@ static inline pid_t pid_nr(struct pid *pid) } pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns); +const struct upid *pid_upid_ns(struct pid *pid, struct pid_namespace *ns); pid_t pid_vnr(struct pid *pid); #define do_each_pid_task(pid, type, task) \ diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h index 1997ffc295a7..1f9f0455d7ef 100644 --- a/include/linux/pid_namespace.h +++ b/include/linux/pid_namespace.h @@ -26,6 +26,7 @@ struct pid_namespace { struct rcu_head rcu; int last_pid; unsigned int nr_hashed; + atomic64_t next_highpid; struct task_struct *child_reaper; struct kmem_cache *pid_cachep; unsigned int level; diff --git a/kernel/pid.c b/kernel/pid.c index 9b9a26698144..863d11a9bfbf 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -312,6 +312,8 @@ struct pid *alloc_pid(struct pid_namespace *ns) pid-numbers[i].nr = nr; pid-numbers[i].ns = tmp; + pid-numbers[i].highnr = + atomic64_inc_return(tmp-next_highpid) - 1; tmp = tmp-parent; } @@ -492,17 +494,26 @@ struct pid *find_get_pid(pid_t nr) } EXPORT_SYMBOL_GPL(find_get_pid); -pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns) +const struct upid *pid_upid_ns(struct pid *pid,
Re: [systemd-devel] [RFC PATCH] proc, pidns: Add highpid
[Adding CRIU people. Whoops.] On Fri, Nov 28, 2014 at 3:05 PM, Andy Lutomirski l...@amacapital.net wrote: Pid reuse is common, which means that it's difficult or impossible to read information about a pid from /proc without races. This introduces a second number associated with each (task, pidns) pair called highpid. Highpid is a 64-bit number, and, barring extremely unlikely circumstances or outright error, a (highpid, pid) will never be reused. With just this change, a program can open /proc/PID/status, read the Highpid field, and confirm that it has the expected value. If the pid has been reused, then highpid will be different. The initial implementation is straightforward: highpid is simply a 64-bit counter. If a high-end system can fork every 3 ns (which would be amazing, given that just allocating a pid requires at atomic operation), it would take well over 1000 years for highpid to wrap. For CRIU's benefit, the next highpid can be set by a privileged user. NB: The sysctl stuff only works on 64-bit systems. If the approach looks good, I'll fix that somehow. Signed-off-by: Andy Lutomirski l...@amacapital.net --- If this goes in, there's plenty of room to add new interfaces to make this more useful. For example, we could add a fancier tgkill that adds and validates hightgid and highpid, and we might want to add a syscall to read one's own hightgid and highpid. These would be quite useful for pidfiles. David, would this be useful for kdbus? CRIU people: will this be unduly difficult to support in CRIU? If you all think this is a good idea, I'll fix the sysctl stuff and document it. It might also be worth adding Hightgid to status. fs/proc/array.c | 5 - include/linux/pid.h | 2 ++ include/linux/pid_namespace.h | 1 + kernel/pid.c | 19 +++ kernel/pid_namespace.c| 22 ++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index cd3653e4f35c..f1e0e69d19f9 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, int g; struct fdtable *fdt = NULL; const struct cred *cred; + const struct upid *upid; pid_t ppid, tpid; rcu_read_lock(); @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, if (tracer) tpid = task_pid_nr_ns(tracer, ns); } + upid = pid_upid_ns(pid, ns); cred = get_task_cred(p); seq_printf(m, State:\t%s\n Tgid:\t%d\n Ngid:\t%d\n Pid:\t%d\n + Highpid:\t%llu\n PPid:\t%d\n TracerPid:\t%d\n Uid:\t%d\t%d\t%d\t%d\n @@ -183,7 +186,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, get_task_state(p), task_tgid_nr_ns(p, ns), task_numa_group_id(p), - pid_nr_ns(pid, ns), + upid ? upid-nr : 0, upid ? upid-highnr : 0, ppid, tpid, from_kuid_munged(user_ns, cred-uid), from_kuid_munged(user_ns, cred-euid), diff --git a/include/linux/pid.h b/include/linux/pid.h index 23705a53abba..ece70b64d04c 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -51,6 +51,7 @@ struct upid { /* Try to keep pid_chain in the same cacheline as nr for find_vpid */ int nr; struct pid_namespace *ns; + u64 highnr; struct hlist_node pid_chain; }; @@ -170,6 +171,7 @@ static inline pid_t pid_nr(struct pid *pid) } pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns); +const struct upid *pid_upid_ns(struct pid *pid, struct pid_namespace *ns); pid_t pid_vnr(struct pid *pid); #define do_each_pid_task(pid, type, task) \ diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h index 1997ffc295a7..1f9f0455d7ef 100644 --- a/include/linux/pid_namespace.h +++ b/include/linux/pid_namespace.h @@ -26,6 +26,7 @@ struct pid_namespace { struct rcu_head rcu; int last_pid; unsigned int nr_hashed; + atomic64_t next_highpid; struct task_struct *child_reaper; struct kmem_cache *pid_cachep; unsigned int level; diff --git a/kernel/pid.c b/kernel/pid.c index 9b9a26698144..863d11a9bfbf 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -312,6 +312,8 @@ struct pid *alloc_pid(struct pid_namespace *ns) pid-numbers[i].nr = nr; pid-numbers[i].ns = tmp; + pid-numbers[i].highnr = + atomic64_inc_return(tmp-next_highpid) - 1; tmp = tmp-parent;
Re: [systemd-devel] [PATCH] README: notice kernel config for CPUQuota
On Wed, Nov 19, 2014 at 12:13:43AM +0900, WaLyong Cho wrote: --- README | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README b/README index aefb349..70d1105 100644 --- a/README +++ b/README @@ -82,6 +82,9 @@ REQUIREMENTS: CONFIG_CGROUP_SCHED CONFIG_FAIR_GROUP_SCHED +Required for CPUQuota in resource control unit settings + CONFIG_CFS_BANDWIDTH + For systemd-bootchart, several proc debug interfaces are required: CONFIG_SCHEDSTATS CONFIG_SCHED_DEBUG Applied. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH] proc, pidns: Add highpid
On Fri, Nov 28, 2014 at 03:05:01PM -0800, Andy Lutomirski wrote: Pid reuse is common, which means that it's difficult or impossible to read information about a pid from /proc without races. This introduces a second number associated with each (task, pidns) pair called highpid. Highpid is a 64-bit number, and, barring extremely unlikely circumstances or outright error, a (highpid, pid) will never be reused. With just this change, a program can open /proc/PID/status, read the Highpid field, and confirm that it has the expected value. If the pid has been reused, then highpid will be different. The initial implementation is straightforward: highpid is simply a 64-bit counter. If a high-end system can fork every 3 ns (which would be amazing, given that just allocating a pid requires at atomic operation), it would take well over 1000 years for highpid to wrap. For CRIU's benefit, the next highpid can be set by a privileged user. NB: The sysctl stuff only works on 64-bit systems. If the approach looks good, I'll fix that somehow. Signed-off-by: Andy Lutomirski l...@amacapital.net --- If this goes in, there's plenty of room to add new interfaces to make this more useful. For example, we could add a fancier tgkill that adds and validates hightgid and highpid, and we might want to add a syscall to read one's own hightgid and highpid. These would be quite useful for pidfiles. David, would this be useful for kdbus? CRIU people: will this be unduly difficult to support in CRIU? If you all think this is a good idea, I'll fix the sysctl stuff and document it. It might also be worth adding Hightgid to status. fs/proc/array.c | 5 - include/linux/pid.h | 2 ++ include/linux/pid_namespace.h | 1 + kernel/pid.c | 19 +++ kernel/pid_namespace.c| 22 ++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index cd3653e4f35c..f1e0e69d19f9 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, int g; struct fdtable *fdt = NULL; const struct cred *cred; + const struct upid *upid; pid_t ppid, tpid; rcu_read_lock(); @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, if (tracer) tpid = task_pid_nr_ns(tracer, ns); } + upid = pid_upid_ns(pid, ns); cred = get_task_cred(p); seq_printf(m, State:\t%s\n Tgid:\t%d\n Ngid:\t%d\n Pid:\t%d\n + Highpid:\t%llu\n PPid:\t%d\n TracerPid:\t%d\n Uid:\t%d\t%d\t%d\t%d\n Changing existing proc files like this is dangerous and can cause lots of breakage in userspace programs if you are not careful. Usually adding fields to the end of the file is best, but sometimes even then things can break :( ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel