Re: [systemd-devel] /usr vs /etc for default distro units enablement

2014-11-28 Thread Didier Roche

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

2014-11-28 Thread Tom Gundersen
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

2014-11-28 Thread Robert Milasan
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

2014-11-28 Thread Harald Hoyer
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

2014-11-28 Thread Harald Hoyer
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

2014-11-28 Thread Robert Milasan
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

2014-11-28 Thread Ross Lagerwall
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

2014-11-28 Thread Martin Pitt
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

2014-11-28 Thread Harald Hoyer
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

2014-11-28 Thread Martin Pitt
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

2014-11-28 Thread David Herrmann
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

2014-11-28 Thread Richard Weinberger
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

2014-11-28 Thread Martin Pitt
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

2014-11-28 Thread Łukasz Stelmach
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

2014-11-28 Thread Chris Atkinson
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

2014-11-28 Thread David Herrmann
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Chris Atkinson
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Lennart Poettering
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

2014-11-28 Thread Chris Atkinson
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Ross Lagerwall
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

2014-11-28 Thread Lennart Poettering
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.

2014-11-28 Thread Chris Atkinson
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-11-28 Thread Ronny Chevalier
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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.

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Andy Lutomirski
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

2014-11-28 Thread Andy Lutomirski
[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

2014-11-28 Thread Zbigniew Jędrzejewski-Szmek
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

2014-11-28 Thread Greg KH
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