[systemd-devel] [PATCH] Return error code 3 with systemctl status after killall LSB server

2011-07-01 Thread Steven Dake
This patch is probably not correct, but not having a clear
understanding of the systemd states, I'm not sure how to properly
fix the problem.

The test case is as follows:
service httpd start
killall httpd
service httpd status
echo $?
(0 printed, 3 should be printed according to LSB)

Then attempting to start the service again does not allow it to be
started, likely because its in the active state.  I would expect
it would be in some other state besides active, such as failed,
atleast for LSB scripts.

Signed-off-by: Steven Dake sd...@redhat.com
---
 src/systemctl.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/systemctl.c b/src/systemctl.c
index f6dca5b..93f4fda 100644
--- a/src/systemctl.c
+++ b/src/systemctl.c
@@ -2614,9 +2614,11 @@ static int show_one(const char *verb, DBusConnection 
*bus, const char *path, boo
 if (!show_properties)
 print_status_info(info);
 
-if (!streq_ptr(info.active_state, active) 
+if ((!streq_ptr(info.active_state, active) 
 !streq_ptr(info.active_state, reloading) 
-streq(verb, status))
+streq(verb, status)) ||
+(streq_ptr(info.active_state, active) 
+streq_ptr(info.sub_state, exited)))
 /* According to LSB: program not running */
 r = 3;
 
-- 
1.7.4.4

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


[systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Chris Ferron

MeeGo will use several consoles depending on the hardware adaptations.
This patch adds the most common to the serial-getty unit Install section 
so when systemd is built for MeeGo as its distribution, you get the most 
common Aliased for installation by the user.




From c50559ea74069061401c8a218d73d515d7b9cd09 Mon Sep 17 00:00:00 2001
From: Chris Ferron chris.e.fer...@linux.intel.com
Date: Fri, 1 Jul 2011 08:54:37 -0700
Subject: [PATCH] Add the most common consoles that MeeGo as a distribution
 will need.

---
 units/serial-getty@.service.m4 |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4
index 082290c..aea47be 100644
--- a/units/serial-getty@.service.m4
+++ b/units/serial-getty@.service.m4
@@ -9,6 +9,9 @@
 Description=Serial Getty on %I
 BindTo=dev-%i.device
 After=dev-%i.device systemd-user-sessions.service 
plymouth-quit-wait.service

+m4_ifdef(`TARGET_MEEGO',
+After=dev-%i.device systemd-user-sessions.service
+)m4_dnl
 m4_ifdef(`TARGET_FEDORA',
 After=rc-local.service
 )m4_dnl
@@ -44,3 +47,9 @@ KillMode=process
 # Some login implementations ignore SIGTERM, so we send SIGHUP
 # instead, to ensure that login terminates cleanly.
 KillSignal=SIGHUP
+
+m4_ifdef(`TARGET_MEEGO',
+[Install]
+Alias=sysinit.target.wants/serial-getty@ttyS0.service 
sysinit.target.wants/serial-getty@ttyS1.service 
sysinit.target.wants/serial-getty@tty01.service 
sysinit.target.wants/serial-getty@ttyO2.service

+)m4_dnl
+
--
1.7.4.4


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


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Kok, Auke-jan H
On Fri, Jul 1, 2011 at 10:11 AM, Koen Kooi k...@dominion.thruhere.net wrote:

 Op 1 jul 2011, om 18:19 heeft Chris Ferron het volgende geschreven:

 MeeGo will use several consoles depending on the hardware adaptations.
 This patch adds the most common to the serial-getty unit Install section so 
 when systemd is built for MeeGo as its distribution, you get the most common 
 Aliased for installation by the user.



 From c50559ea74069061401c8a218d73d515d7b9cd09 Mon Sep 17 00:00:00 2001
 From: Chris Ferron chris.e.fer...@linux.intel.com
 Date: Fri, 1 Jul 2011 08:54:37 -0700
 Subject: [PATCH] Add the most common consoles that MeeGo as a distribution
  will need.

 ---
  units/serial-getty@.service.m4 |    9 +
  1 files changed, 9 insertions(+), 0 deletions(-)

 diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4
 index 082290c..aea47be 100644
 --- a/units/serial-getty@.service.m4
 +++ b/units/serial-getty@.service.m4
 @@ -9,6 +9,9 @@
  Description=Serial Getty on %I
  BindTo=dev-%i.device
  After=dev-%i.device systemd-user-sessions.service plymouth-quit-wait.service
 +m4_ifdef(`TARGET_MEEGO',
 +After=dev-%i.device systemd-user-sessions.service
 +)m4_dnl
  m4_ifdef(`TARGET_FEDORA',
  After=rc-local.service
  )m4_dnl
 @@ -44,3 +47,9 @@ KillMode=process
  # Some login implementations ignore SIGTERM, so we send SIGHUP
  # instead, to ensure that login terminates cleanly.
  KillSignal=SIGHUP
 +
 +m4_ifdef(`TARGET_MEEGO',
 +[Install]
 +Alias=sysinit.target.wants/serial-getty@ttyS0.service 
 sysinit.target.wants/serial-getty@ttyS1.service 
 sysinit.target.wants/serial-getty@tty01.service 
 sysinit.target.wants/serial-getty@ttyO2.service
 +)m4_dnl

 An a lot of OMAP systems ttyO1 is hooked up to the bluetooth UART, so I don't 
 know how usefull running a getty on that is.

Note that just adding the Alias doesn't actually make a serial console
spawn on ttyO1 in the first place. The alias is there because on some
devices, ttyO1 *is* the serial console, and this is just a reference.

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


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Koen Kooi

Op 1 jul 2011, om 20:02 heeft Chris Ferron het volgende geschreven:

 On 07/01/2011 10:11 AM, Koen Kooi wrote:
 Op 1 jul 2011, om 18:19 heeft Chris Ferron het volgende geschreven:
 
 MeeGo will use several consoles depending on the hardware adaptations.
 This patch adds the most common to the serial-getty unit Install section so 
 when systemd is built for MeeGo as its distribution, you get the most 
 common Aliased for installation by the user.
 
 
 
 From c50559ea74069061401c8a218d73d515d7b9cd09 Mon Sep 17 00:00:00 2001
 From: Chris Ferronchris.e.fer...@linux.intel.com
 Date: Fri, 1 Jul 2011 08:54:37 -0700
 Subject: [PATCH] Add the most common consoles that MeeGo as a distribution
  will need.
 
 ---
  units/serial-getty@.service.m4 |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4
 index 082290c..aea47be 100644
 --- a/units/serial-getty@.service.m4
 +++ b/units/serial-getty@.service.m4
 @@ -9,6 +9,9 @@
  Description=Serial Getty on %I
  BindTo=dev-%i.device
  After=dev-%i.device systemd-user-sessions.service 
 plymouth-quit-wait.service
 +m4_ifdef(`TARGET_MEEGO',
 +After=dev-%i.device systemd-user-sessions.service
 +)m4_dnl
  m4_ifdef(`TARGET_FEDORA',
  After=rc-local.service
  )m4_dnl
 @@ -44,3 +47,9 @@ KillMode=process
  # Some login implementations ignore SIGTERM, so we send SIGHUP
  # instead, to ensure that login terminates cleanly.
  KillSignal=SIGHUP
 +
 +m4_ifdef(`TARGET_MEEGO',
 +[Install]
 +Alias=sysinit.target.wants/serial-getty@ttyS0.service 
 sysinit.target.wants/serial-getty@ttyS1.service 
 sysinit.target.wants/serial-getty@tty01.service 
 sysinit.target.wants/serial-getty@ttyO2.service
 +)m4_dnl
 An a lot of OMAP systems ttyO1 is hooked up to the bluetooth UART, so I 
 don't know how usefull running a getty on that is.
 
 regards,
 
 Koen
 Thanks for the comment. There may be a good amount of OMAP systems that will 
 use tty01 as a non console.
 But this change should only be included in the MeeGo distribution. In MeeGo 
 at this time we do not have any OMAP adaptations that have this behaviour,

Beagleboard and pandaboard have bluetooth on ttyO1 and meego runs on that, or 
have I misunderstood.

 but we do have supported hardware adaptations that do use tty01 as a console.
 
 So for MeeGo we want the Aliases so that if users using MeeGo on a supported 
 hardware adaptations want to enable the tty01 then they can.
 As Aliases do not mean the tty* will spawn, they are a perfect for giving the 
 users in MeeGo the support and option.

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


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Paul Menzel
Dear Chris,


please just post plain text messages (and no HTML) to lists [1].

Am Freitag, den 01.07.2011, 09:19 -0700 schrieb Chris Ferron:
 MeeGo will use several consoles depending on the hardware adaptations.
 This patch adds the most common to the serial-getty unit Install section 
 so when systemd is built for MeeGo as its distribution, you get the most 
 common Aliased for installation by the user.

Why don’t you use that as a commit message?

  From c50559ea74069061401c8a218d73d515d7b9cd09 Mon Sep 17 00:00:00 2001
 From: Chris Ferron chris.e.fer...@linux.intel.com
 Date: Fri, 1 Jul 2011 08:54:37 -0700
 Subject: [PATCH] Add the most common consoles that MeeGo as a distribution
   will need.

Please turn of line breaks when pasting stuff. You can also use `git
send-email` to send patches.

 ---
   units/serial-getty@.service.m4 |9 +
   1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4
 index 082290c..aea47be 100644
 --- a/units/serial-getty@.service.m4
 +++ b/units/serial-getty@.service.m4
 @@ -9,6 +9,9 @@
   Description=Serial Getty on %I
   BindTo=dev-%i.device
   After=dev-%i.device systemd-user-sessions.service 
 plymouth-quit-wait.service
 +m4_ifdef(`TARGET_MEEGO',
 +After=dev-%i.device systemd-user-sessions.service
 +)m4_dnl
   m4_ifdef(`TARGET_FEDORA',
   After=rc-local.service
   )m4_dnl
 @@ -44,3 +47,9 @@ KillMode=process
   # Some login implementations ignore SIGTERM, so we send SIGHUP
   # instead, to ensure that login terminates cleanly.
   KillSignal=SIGHUP
 +
 +m4_ifdef(`TARGET_MEEGO',
 +[Install]
 +Alias=sysinit.target.wants/serial-getty@ttyS0.service 
 sysinit.target.wants/serial-getty@ttyS1.service 
 sysinit.target.wants/serial-getty@tty01.service 
 sysinit.target.wants/serial-getty@ttyO2.service
 +)m4_dnl
 +

Unnecessary line at the end?

Are such customization wanted upstream or are there other ways to
implement that? I would imagine this becomes very complex if every
distribution starts adding such things.


Thanks,

Paul


signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Chris Ferron

On 07/01/2011 11:15 AM, Koen Kooi wrote:

Op 1 jul 2011, om 20:02 heeft Chris Ferron het volgende geschreven:


On 07/01/2011 10:11 AM, Koen Kooi wrote:

Op 1 jul 2011, om 18:19 heeft Chris Ferron het volgende geschreven:


MeeGo will use several consoles depending on the hardware adaptations.
This patch adds the most common to the serial-getty unit Install section so 
when systemd is built for MeeGo as its distribution, you get the most common 
Aliased for installation by the user.



 From c50559ea74069061401c8a218d73d515d7b9cd09 Mon Sep 17 00:00:00 2001
From: Chris Ferronchris.e.fer...@linux.intel.com
Date: Fri, 1 Jul 2011 08:54:37 -0700
Subject: [PATCH] Add the most common consoles that MeeGo as a distribution
  will need.

---
  units/serial-getty@.service.m4 |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4
index 082290c..aea47be 100644
--- a/units/serial-getty@.service.m4
+++ b/units/serial-getty@.service.m4
@@ -9,6 +9,9 @@
  Description=Serial Getty on %I
  BindTo=dev-%i.device
  After=dev-%i.device systemd-user-sessions.service plymouth-quit-wait.service
+m4_ifdef(`TARGET_MEEGO',
+After=dev-%i.device systemd-user-sessions.service
+)m4_dnl
  m4_ifdef(`TARGET_FEDORA',
  After=rc-local.service
  )m4_dnl
@@ -44,3 +47,9 @@ KillMode=process
  # Some login implementations ignore SIGTERM, so we send SIGHUP
  # instead, to ensure that login terminates cleanly.
  KillSignal=SIGHUP
+
+m4_ifdef(`TARGET_MEEGO',
+[Install]
+Alias=sysinit.target.wants/serial-getty@ttyS0.service 
sysinit.target.wants/serial-getty@ttyS1.service 
sysinit.target.wants/serial-getty@tty01.service 
sysinit.target.wants/serial-getty@ttyO2.service
+)m4_dnl

An a lot of OMAP systems ttyO1 is hooked up to the bluetooth UART, so I don't 
know how usefull running a getty on that is.

regards,

Koen

Thanks for the comment. There may be a good amount of OMAP systems that will 
use tty01 as a non console.
But this change should only be included in the MeeGo distribution. In MeeGo at 
this time we do not have any OMAP adaptations that have this behaviour,

Beagleboard and pandaboard have bluetooth on ttyO1 and meego runs on that, or 
have I misunderstood.
No you did not misunderstand. There are some who have been using 
beagleboards and pandaboards with MeeGo, and that is great. But I know 
of no hard plan to have full MeeGo support for them. *BUT* that may change.
Regardless, in the event that it changes, any vertical that uses an OMAP 
system would just not enable tty01, but would enable the tty that they 
need.


There are several options used in MeeGo with its current hardware 
adaptations. We are only up-streaming the support option for the most 
common, but not forcing.


Your point is a good one to consider, but I think this solution will work.

but we do have supported hardware adaptations that do use tty01 as a console.

So for MeeGo we want the Aliases so that if users using MeeGo on a supported 
hardware adaptations want to enable the tty01 then they can.
As Aliases do not mean the tty* will spawn, they are a perfect for giving the 
users in MeeGo the support and option.

Ok


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


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Lennart Poettering
On Fri, 01.07.11 09:19, Chris Ferron (chris.e.fer...@linux.intel.com) wrote:

Heya,

 MeeGo will use several consoles depending on the hardware adaptations.
 This patch adds the most common to the serial-getty unit Install
 section so when systemd is built for MeeGo as its distribution, you
 get the most common Aliased for installation by the user.

Hmm, this patch is borked, due to line breaks done by your mailer.

  After=dev-%i.device systemd-user-sessions.service
 plymouth-quit-wait.service
 +m4_ifdef(`TARGET_MEEGO',
 +After=dev-%i.device systemd-user-sessions.service
 +)m4_dnl

Why repeat the previous line? Note that multiple After= lines are
merged, so this appears fully redundant to me?

  m4_ifdef(`TARGET_FEDORA',
  After=rc-local.service
  )m4_dnl
 @@ -44,3 +47,9 @@ KillMode=process
  # Some login implementations ignore SIGTERM, so we send SIGHUP
  # instead, to ensure that login terminates cleanly.
  KillSignal=SIGHUP
 +
 +m4_ifdef(`TARGET_MEEGO',
 +[Install]
 +Alias=sysinit.target.wants/serial-getty@ttyS0.service
 sysinit.target.wants/serial-getty@ttyS1.service
 sysinit.target.wants/serial-getty@tty01.service
 sysinit.target.wants/serial-getty@ttyO2.service
 +)m4_dnl
 +

Hmm, what's the rationale behind this? if those devices never show up
then you end up spawning services with dependencies that necessarily
time out (i.e. the non-existing serial ports). To make this nicer I'd
turn this around: write a simple udev rule that matches against all ttys
you are interested in and pull in a getty for each with
SYSTEMD_WANTS. Something like this should do the job:

SUBSYSTEM==tty, KERNEL==ttyS*|ttyO*|tty0*, TAG=systemd, 
ENV{SYSTEMD_WANTS}=serial-getty@%k.service

If you do this, then you'll spawn exactly the gettys that match your
system, and this stuff is even hotpluggable. (i.e. if you extend this
for ttyUSB you could get a getty on it as you plug it in, how awesome is
that!).

Also, this really looks like something that is more appropriately done
with ln -s lines in the RPM .spec %post script?

And the other gettys are pulled in by getty.target, which appears more
appropriate than sysinit.target.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Chris Ferron

On 07/01/2011 01:54 PM, Lennart Poettering wrote:

On Fri, 01.07.11 09:19, Chris Ferron (chris.e.fer...@linux.intel.com) wrote:

Heya,


MeeGo will use several consoles depending on the hardware adaptations.
This patch adds the most common to the serial-getty unit Install
section so when systemd is built for MeeGo as its distribution, you
get the most common Aliased for installation by the user.

Hmm, this patch is borked, due to line breaks done by your mailer.

yes sorry about that.

  After=dev-%i.device systemd-user-sessions.service
plymouth-quit-wait.service
+m4_ifdef(`TARGET_MEEGO',
+After=dev-%i.device systemd-user-sessions.service
+)m4_dnl

Why repeat the previous line? Note that multiple After= lines are
merged, so this appears fully redundant to me?

good point

  m4_ifdef(`TARGET_FEDORA',
  After=rc-local.service
  )m4_dnl
@@ -44,3 +47,9 @@ KillMode=process
  # Some login implementations ignore SIGTERM, so we send SIGHUP
  # instead, to ensure that login terminates cleanly.
  KillSignal=SIGHUP
+
+m4_ifdef(`TARGET_MEEGO',
+[Install]
+Alias=sysinit.target.wants/serial-getty@ttyS0.service
sysinit.target.wants/serial-getty@ttyS1.service
sysinit.target.wants/serial-getty@tty01.service
sysinit.target.wants/serial-getty@ttyO2.service
+)m4_dnl
+

Hmm, what's the rationale behind this? if those devices never show up
then you end up spawning services with dependencies that necessarily
Oh so you will attempt to spawn service defined by Alias even if they 
have not been enabled?
I thought you would set up the Alias and then only if you enabled that 
name would it attempt start the service.



time out (i.e. the non-existing serial ports). To make this nicer I'd
turn this around: write a simple udev rule that matches against all ttys
you are interested in and pull in a getty for each with
SYSTEMD_WANTS. Something like this should do the job:

SUBSYSTEM==tty, KERNEL==ttyS*|ttyO*|tty0*, TAG=systemd, 
ENV{SYSTEMD_WANTS}=serial-getty@%k.service

interesting Idea.

If you do this, then you'll spawn exactly the gettys that match your
system, and this stuff is even hotpluggable. (i.e. if you extend this
for ttyUSB you could get a getty on it as you plug it in, how awesome is
that!).

Also, this really looks like something that is more appropriately done
with ln -s lines in the RPM .spec %post script?

yes I have something like this in our package as well.

And the other gettys are pulled in by getty.target, which appears more
appropriate than sysinit.target.

Thanks for the input. I will consider this PATCH punted, and will 
re-think the need. :

Lennart



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


Re: [systemd-devel] [PATCH] Return error code 3 with systemctl status after killall LSB server

2011-07-01 Thread Lennart Poettering
On Fri, 01.07.11 00:00, Steven Dake (sd...@redhat.com) wrote:

 This patch is probably not correct, but not having a clear
 understanding of the systemd states, I'm not sure how to properly
 fix the problem.
 
 The test case is as follows:
 service httpd start
 killall httpd
 service httpd status
 echo $?
 (0 printed, 3 should be printed according to LSB)
 
 Then attempting to start the service again does not allow it to be
 started, likely because its in the active state.  I would expect
 it would be in some other state besides active, such as failed,
 atleast for LSB scripts.

The problem here is that apache is a SysV init script, and for those it
is not really clear whether it is a problem that no process is running
anymore or if that's just the normal case. I.e. consider scripts like
the NFS or ALSA scripts which just set something up and exit. OTOH there
are daemons like apache where it is clearly a problem if nothing is
running anymore. But from systemd's perspective we don't really have a
chance figuring out which kind of service a particular init script is,
and hence we must assume that even though no process is running anymore
for the service it still is active, which systemctl status
httpd.service will show you. However we will show (exited) next to
it, to make clear that while we still consider the service active, it
doesn't have any running processes anymore.

Furthermore when you send SIGTERM to apache, it exits with exit code 0,
which is a clean error code. That means that systemd will not put the
service in failed state.

The right fix is probably to write a native systemd file for Apache,
where these problems don't really exist. Looking at the F15 version of
the Apache init script this should actually be very simple to
do. Something like this would probably already suffice:

snip
[Unit]
Description=Apache Web Server

[Service]
Type=forking
PIDFile=/var/run/httpd/httpd.pid
ExecStart=/usr/sbin/httpd
ExecReload=/usr/sbin/apachectl reload

[Install]
WantedBy=multi-user.target
snip

With a unit file like this in place systemd will properly detect whether
Apache is running or not, and show that in systemctl status and its
exit code.

(Note that in an ideal world we'd use Type=notify here instead of having
apache fork, but that requires minimal patching of Apache.)

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Add the most common consoles that MeeGo needs

2011-07-01 Thread Lennart Poettering
On Fri, 01.07.11 14:08, Chris Ferron (chris.e.fer...@linux.intel.com) wrote:

 +m4_ifdef(`TARGET_MEEGO',
 +[Install]
 +Alias=sysinit.target.wants/serial-getty@ttyS0.service
 sysinit.target.wants/serial-getty@ttyS1.service
 sysinit.target.wants/serial-getty@tty01.service
 sysinit.target.wants/serial-getty@ttyO2.service
 +)m4_dnl

  Hmm, what's the rationale behind this? if those devices never show up
  then you end up spawning services with dependencies that necessarily

 Oh so you will attempt to spawn service defined by Alias even if
 they have not been enabled?

Well, but with an Alias= line like the above you will enable the service
for four ttys at once, and if I understood things correctly, then only
one of them is right for a specific platform? If so you'd have three
gettys started whose dependencies will time out.

 I thought you would set up the Alias and then only if you enabled
 that name would it attempt start the service.

That is correct, but with your line you'd - if you enable the unit - the
getty end up with four instances, thee of which would time out, iiuc.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] nspawn: spawn shell under specified --user

2011-07-01 Thread Lennart Poettering
On Wed, 29.06.11 14:22, Michal Vyskocil (mvysko...@suse.cz) wrote:

 Add -u/--user option, which changes the effective and real user and
 group id to the new value. The user must exists in the chroot, otherwise
 it will fail. Both username and user id are accepted. The user home is
 created as well.
 
 It also setup HOME, USER, LOGNAME and SHELL variables .

Thanks a lot, applied both patches!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] configure: Add --enable-plymouth option

2011-07-01 Thread Lennart Poettering
On Tue, 28.06.11 01:49, Henry Gebhardt (hsggebha...@googlemail.com) wrote:

 As in the bug[1] it would be nice to configure plymouth support
 independently of the distro. This patch adds a --enable-plymouth and
 --disable-plymouth option to the configure script to overwrite the
 distro specific default.

Thanks, applied.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] encrypted /var with systemd - impossible?

2011-07-01 Thread Lennart Poettering
On Tue, 28.06.11 22:40, yoshi watanabe (yoshi...@gmail.com) wrote:

 reinstalled and got reproductible environment with the error.
 
 first, i tried booting with dracut luks management with default
 parameters. first thing i noticed, dracut didn't ask for cryptsetup
 password. i suppose that if / is not encrypted the feature is not
 included into initrd for the system. i got asked for the password by
 systemd, though.
 
 i tried forcing the luks uuid, and i also tried rd_NO_LUKS boot to see
 if it makes any difference. log of the last run is attached.
 
 i'm attaching a short info of my partition setup and dmesg from debug
 bootup. cryptsetup password was provided manually during boot, after a
 few seconds the big delay (evident in the log) happened.
 
 when i try booting, i get into text mode, get asked for maintenance
 root password and dropped into single mode shell. and that's a system
 that hasn't even ran the first install wizard to set up user accounts
 etc. nothing is changed in there.
 
 boot parameters used (if they are not visible in the log)
 
 systemd.log_level=debug
 systemd.log_target=kmsg
 rd_NO_LUKS
 video=1024x768@75
 root=UUID=ae028a28-be41-480b-ba16-6e648a3a3db3

Hmm, it seems dev-mapper-box\x2dfedoravar.device never shows up. Which
is an LVM device as it appears. Which makes me guess that this i
actually just an iteration of this bug:

https://bugzilla.redhat.com/show_bug.cgi?id=708684

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] umount: ignore missing /proc/swaps

2011-07-01 Thread Lennart Poettering
On Tue, 28.06.11 09:30, Michael Olbrich (m.olbr...@pengutronix.de) wrote:

 /proc/swaps does not exist when swap is disabled in the kernel.
 Just report an empty list of mountpoints to unmount in this case.
 ---
 
 Hi,
 
 Missed this one, when I made the last patch. This should take care of any
 bogus swap error messages during shutdown.

Thanks, applied!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd and LSB Required/Should-Stop

2011-07-01 Thread Lennart Poettering
On Tue, 28.06.11 12:54, Ville Skyttä (ville.sky...@iki.fi) wrote:

 
 On 06/27/2011 04:18 PM, Lennart Poettering wrote:
  On Fri, 24.06.11 14:04, Ville Skyttä (ville.sky...@iki.fi) wrote:
  
  Hello,
 
  Am I right that systemd does currently have no support for the LSB
  Required-Stop and Should-Stop keywords?  If yes, are there plans to
  support them in the future?
  
  That is true, we do not support this right now. In systemd the shutdown
  order is always the inverse startup order of two services. I am not
  entirely opposed to make this more flexible but so far I have not heard
  of a good usecase where the flexibility to detach the shutdown oder from
  the startup order was necessary. 
 
 I don't know if this is a good argument or not, but for now I don't know
 what to add as a start dependency to get stuff run before hwclock does
 its stuff at shutdown.  $time?  hwclock-save?  Something else?

Order against hwclock-save.service. That should do the job for you. Just
stick Before=hwclock-save.service in your service file if it is native,
or Should-Start=hwclock-save if it is SysV.

  The use case I have is in Fedora's vdr package:
  http://pkgs.fedoraproject.org/gitweb/?p=vdr.git .  The init script's
  stop action uses rtcwake to set the next wakeup time for the box, and
  even though the implementation is hairier than I'd like, it has worked
  well for me before Fedora 15.  But with Fedora 15 and systemd it often
  fails to do its job when shutting down the system (works fine if I just
  stop vdr with the machine running), which causes my PVR box not to wake
  up when it should and consequently missed timed recordings which is very
  annoying.
 
  The way the init script's stop action handles setting the wakeup time is
  that it looks up the time to set from the /var/run/vdr/next-timer file
  which may or may not be present; if it's not, it grabs the time using
  SVDRP (a simple TCP based protocol) from localhost, and finally if it
  gets the time from one of these sources, writes it to the RTC with
  rtcwake.
  
  Is there any good reason why this isn't stored in the RTC right-away,
  when needed? It sounds unnecessarily complex and fragile to delay this
  until shutdown.
 
 Yes, see hairier than I'd like above and some thoughts about fixing
 vdr, and for now there's nothing below quoted from my original mail.
  It boils down to 1) the code doesn't exist yet, 2) I'm not really a
 C(++) coder so implementing it might take a while (duplicate rtcwake
 code? the daemon starts up as root but drops privileges - libcap? needs
 to be pluggable because some systems don't work with rtcwake (dunno
 about CLOCK_REALTIME_ALARM) but require other nonstandard tools) etc,
 and 3) I have a hunch that such a fix might end up needing to be
 maintained outside of upstream vdr as a patch or a plugin which given 2)
 doesn't sound that nice.

CLOCK_REALTIME_ALARM should simplify most things, as you don't need to
deal with any low-level intricacies of rtcwake, and just can use a
normal timer that happens to wake up the system when it elapses when the
system is suspended.

  We have now removed this unit file in git. However can you elaborate why
  exactly these two service conflict when run at the same time? do you
  have any error messages?
 
 I don't have the exact one at hand now, but if I remember correctly, it
 was /dev/rtc0: device or resource busy and according to fuser, hwclock
 was the thing reserving /dev/rtc0 thus causing rtcwake to fail.

Oh, that would mean that /dev/rtc0 can only have a single opener at a
time? 

Indeed, just verified this. How ugly. CLOCK_REALTIME_ALARM FTW!

  If a service A and a service B are ordered against each other
  (regardless in which way) then systemd will ensure that when one is
  started and the other is stopped, the stopping is done first and the
  starting second. That means you can just add any kind of ordering dep
  between the two, and things should work.
 
 That's what I've understood, but what I don't know at the moment what to
 add to vdr's Should-Start so that it doesn't cause problems at
 startup, and ends up running before hwclock-save at shutdown.  Simply
 hwclock-save, even if it shouldn't be run at startup at all?  

Yes.

 And as you mentioned, the hwclock-save unit file has been removed in
 git, does something else replace what it was doing or fiddle with the
 RTC otherwise, or was the feature dropped altogether and I can use
 hwclock-save in Should-Start now and not worry about the time when a
 systemd that doesn't have it lands in Fedora?

In systemd git (and soon on Rawhide/F16) we do not fiddle with the RTC
at all anymore at shutdown. Instead we want everybody to sync time
changes directly down to the RTC and if NTP is used the kernel does that
anyway. Hence your service will be the only user (well, unless some
other services decides to use the RTC too. You guys should really use
CLOCK_REALTIME_ALARM which allows multiple consumers of the logic).

 Another 

Re: [systemd-devel] autofs and hugetlbfs on ARM

2011-07-01 Thread Lennart Poettering
On Tue, 28.06.11 09:43, Michael Olbrich (m.olbr...@pengutronix.de) wrote:

 Hi,
 
 more stuff from the embedded front. On ARM hugetlbfs is not supported. The
 result is rather confusing for the user:
 At first /dev/hugepages exists. If I run ls /dev/hugepages,
 mount[494]: mount: unknown filesystem type 'hugetlbfs' is logged and
 ls says ls: /dev/hugepages: No such device.
 Afterwards /dev/hugepages is gone.

We probably shouldn't install those mount and automount units at all on
ARM. Would be happy to take a patch that ifdefs this out with automake
conditions on the archs that don't support this.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] autofs and hugetlbfs on ARM

2011-07-01 Thread Kok, Auke-jan H
On Fri, Jul 1, 2011 at 3:50 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 28.06.11 09:43, Michael Olbrich (m.olbr...@pengutronix.de) wrote:

 Hi,

 more stuff from the embedded front. On ARM hugetlbfs is not supported. The
 result is rather confusing for the user:
 At first /dev/hugepages exists. If I run ls /dev/hugepages,
 mount[494]: mount: unknown filesystem type 'hugetlbfs' is logged and
 ls says ls: /dev/hugepages: No such device.
 Afterwards /dev/hugepages is gone.

 We probably shouldn't install those mount and automount units at all on
 ARM. Would be happy to take a patch that ifdefs this out with automake
 conditions on the archs that don't support this.

We don't have it enabled in MeeGo... not even for x86.

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


Re: [systemd-devel] [PATCH] Use /etc/rc.d/rc.local as the path to rc.local in Fedora.

2011-07-01 Thread Lennart Poettering
On Mon, 27.06.11 10:51, Bill Nottingham (nott...@redhat.com) wrote:

 
 Lennart Poettering (lenn...@poettering.net) said: 
  On Tue, 21.06.11 15:35, Bill Nottingham (nott...@redhat.com) wrote:
  
   /etc/rc.local is a symlink.
  
  Applied.
  
  Hmm, should we drop the x bit on this script by default, as discussed on 
  IRC?
 
 I'd like to find a better solution than that ... perhaps just %ghosting it.

That would be great. Since the unit file already has a
ConditionPathExists this would just work. If you %ghost that file in the
initscripts package I would be quite happy!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] RFC: creating a set of systemd RPM macros across distributions ?

2011-07-01 Thread Lennart Poettering
On Tue, 28.06.11 13:11, Kay Sievers (kay.siev...@vrfy.org) wrote:

 Right, it doesn't cover: [ $1 -eq 1 ], which it should. Packaging
 enable-links in /etc/systemd should just be %ghosted not installed.
 Services which can not be enabled/disabled like udev, D-Bus,
 systemd-internal services might install the links directly to
 /lib/systemd/system/, but that should never be done for /etc.
 
 Anyway, when we think about general service integration into package
 management, I like to see something like some 'product package
 defaults' included from the first step on, or at least have thought
 about them and prepared the hooks in the same way we would use them,
 when such defaults would exist.
 
 It does not make much sense to encode the enabled-at-install,
 enabled-on-update, started-at-install, restarted-on-update flags into
 the spec file, it should all be derived from some database that comes
 along with the system to be installed.  Encoding such distro/product
 policy in the spec file is a bad start. General distros have several
 products based on the same package, and products might differ
 completely in the policy they apply to packages. The server product
 might have very different services enabled/disabled by default than
 the Live-CD, but they have exactly the same packages.
 
 I think %service_add() vs. %service_add_enabled() and so on, is
 nothing that should be done in the spec file, or which should differ
 from one service to the other. These calls should all be one and the
 same generic macro, and the policy should be executed behind the
 macro, depending on the product, not depending on the spec file.

I agree with this. I'd really like to see someone working on getting
service enabling profiles or something like that solved at the same
time as we consider these macros. These two changes to service handling
should probably go hand in hand.

I wonder if we should actually help implementation of this with
systemd. i.e. add a switch --if-listed or so to systemctl enable
which enables a unit only if it is listed in some profile file in
/lib/systemd/enable or so. Might be something to think about. By default
we would not ship such a file, and hence would leave all services
disabled when this switch is applied, but distributions could just drop
a file in there and ship different versions on livecds, installs, on
desktops, servers and the various spins. 

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] systemd, ConsoleKit and vte

2011-07-01 Thread Lennart Poettering
On Thu, 23.06.11 01:02, Christian Persch (c...@gnome.org) wrote:

 Hi;
 
 Since systemd is going to subsume ConsoleKit and manage the user
 session, I was wondering if it would be a good idea for this new systemd
 session daemon to also take over the task of gnome-pty-helper?
 
 gnome-pty-helper[0] is a session daemon that the vte library uses to
 open a PTY and write a lastlog/utmp/wtmp record for it. 

Hmm, that's an interesting idea actually. I have now added this to the
TODO list. That said, I wonder if for the sake of utmp/wtmp we really
want to invent new infrastructure here. I mean, utmp/wtmp is kinda
crufty, and should probably go away sooner or later. If we do this then
the main reason should not be utmp/wtmp, but something else. cgroups
might be a convincing idea here.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] BUG: systemd tmpfiles-clean.service trips over afuse stuff in /tmp

2011-07-01 Thread Lennart Poettering
On Mon, 27.06.11 08:39, Kok, Auke-jan H (auke-jan.h@intel.com) wrote:

  I am tempted to say that this behaviour makes a lot of sense, but I can
  also understand if you'd prefer it not to return an error exit code.
 
  To understand this better, what is afuse, and why does it return EPERM
  for us even though we are root?
 
 afuse is a userspace helper for the fuse filesystem. In my case, I use
 it to automount  sshfs
 
 afuse sets up a temp mount point for each automounted fs in /tmp, and
 since this is done using credentials, I guess that root is not
 permitted to inspect them:
 
 root@desktop /tmp/afuse-CAUhnW # ls -la
 ls: cannot access server: Permission denied
 total 0
 drwx-- 1 sofar sofar   10 Jun 25 16:20 .
 drwxrwxrwt 1 root  root  7888 Jun 27 08:30 ..
 d? ? ? ??? server
 
 root@hannah /tmp/afuse-CAUhnW # stat server
 stat: cannot stat `server': Permission denied
 
 In general, I'm not sure what to do with this case. The error had me
 wondering if tmpfile.d did complete, which wasn't entirely clear from
 the error message in the service. On top of that, I'd say that it was
 successful in the first place, it just encountered something strange
 on the way. Worth a syslog message for sure, but I'm not entirely sure
 this should result in the service being marked as 'failed', which
 reads a lot differently.

I have now modified systemd git to not exit with a failure code in this
case. In the context of fuse and selinux (where even the root user might
get EPERM or EACCES) it's probably wise to avoid any confusion and log
the files we cannot access but not return a non-zero exit code. We will
still exit with an error if there's an error in the config files
however.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Problem with netwrok broadband modem

2011-07-01 Thread Lennart Poettering
On Mon, 27.06.11 14:59, Christian Gagneraud (ch...@techworks.ie) wrote:

 
 On 27/06/11 12:54, Lennart Poettering wrote:
 On Fri, 24.06.11 15:59, Christian Gagneraud (ch...@techworks.ie) wrote:
 
 Hi,
 
 I'm trying to use a USB broadband modem with ofono and connman, when
 i plug the modem, i get an error from systemd Failed to load device
 unit: Invalid argument (full log below).
 
 Hmm, this smells like a bug in systemd.
 
 Do you have any service file for this device installed?
 
 Is it looking for a dev-tty@.device file?
 
 Yes, but usually this should just fail with ENOENT and be ignored. For
 some reason this generates EINVAL for you however.
 
 As i'm having trouble to get ofono working, I wonder if the problem
 doesn't come from here instead, or even perhaps it's due to the
 manufaturer and serial number of the USB device being garbage...
 
 That could actually be. Of course we should fix systemd to handle this.
 
 I wonder how I could easily reproduce this issue here, without that hw.
 
 My first guess is that i's actually the length of the name, and not the
 contents of it that triggers the EINVAL.
 
 
 Could you point me to the right source file and/or functions where I
 could add some debug statement to see if these garbage in the name
 cause troubles.

The various files in unit-name.h are probably the right place for this:
unit_name_is_valid_no_type() in particular. My guess is that the
strlen() check in there is simply the cause of failure.

BTW, could you paste the output of udevadm info -qall
-p/sys/class/tty/ttyXX for the tty in question? This might give me a
hint what systemd is choking on without the debug data from
unit_name_is_valid_no_type(). 

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] autofs and hugetlbfs on ARM

2011-07-01 Thread Kok, Auke-jan H
On Fri, Jul 1, 2011 at 4:23 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Fri, 01.07.11 16:06, Kok, Auke-jan H (auke-jan.h@intel.com) wrote:


 On Fri, Jul 1, 2011 at 3:50 PM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Tue, 28.06.11 09:43, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
 
  Hi,
 
  more stuff from the embedded front. On ARM hugetlbfs is not supported. The
  result is rather confusing for the user:
  At first /dev/hugepages exists. If I run ls /dev/hugepages,
  mount[494]: mount: unknown filesystem type 'hugetlbfs' is logged and
  ls says ls: /dev/hugepages: No such device.
  Afterwards /dev/hugepages is gone.
 
  We probably shouldn't install those mount and automount units at all on
  ARM. Would be happy to take a patch that ifdefs this out with automake
  conditions on the archs that don't support this.

 We don't have it enabled in MeeGo... not even for x86.

 Do have just the file system disabled or hugepage support at all?

 If you disabled just the fs, did you do that because it is an obsolete
 interface? Or simply because it is primarily relevant for huge databases
 and people presumably don't run those on Meego? ;-)

We don't even have HIGHMEM enabled, again not even for x86. HUGEPAGEs
(transparent) are actually enabled, but I'm not entirely sure if we do
have users for them in reality(although I could see we would), I can
poke my friendly meego kernel maintainer if you'd really want to know
;)

The biggest database we have on MeeGo installs is the rpm database
Does that answer that part?

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


Re: [systemd-devel] [PATCH] systemd restart patch

2011-07-01 Thread Lennart Poettering
On Fri, 01.07.11 15:35, Cristian Patrascu (cristian.patra...@windriver.com) 
wrote:

Heya!

(BTW, got your first post of this patch too, just didn't find the time
to review the patch, sorry. It was still in my mail queue however.)

  After x restarts have happened, the OnFailure unit will start
 (if specified) but only after all unsuccessfull restarts (after x
 restarts reached).

Patch looks pretty good. A few comments though.
 
 This patch is made for systemd-29, on master branch with SHA1 ID:
 3661ac04b4f2840d3345605aa35963bbde3c469d
 
 systemd-restart-on-fail.patch : ___
 
 Index: systemd-29/src/dbus-service.c
 ===
 --- systemd-29.orig/src/dbus-service.c2011-06-22 10:47:14.0 
 +0300
 +++ systemd-29/src/dbus-service.c 2011-06-22 13:48:51.292321742 +0300
 @@ -42,6 +42,8 @@
  property name=\PIDFile\ type=\s\ access=\read\/\n   \
  property name=\NotifyAccess\ type=\s\ access=\read\/\n \
  property name=\RestartUSec\ type=\t\ access=\read\/\n \
 +property name=\MaxRestartRetries\ type=\i\ 
 access=\read\/\n \
 +property name=\RestartRetry\ type=\i\
 access=\read\/\n \

I Think it would make sense to make both of these unsigned, in order to
avoid confusion whether these actually ever can be negative.

If I read your patch correctly right now MaxRestartRetries=-1 means that
there is no limit on the number of retries. I'd use 0 for that instead
(0 is not needed to indicate disabling, since we can indicate that with
Restart=no already.)

 +s-restart_retries = DEFAULT_RESTART_RETRIES;
 +s-restart_crt_retry = 0;

We generally try not to abbrievate variables unnecessarily, so I'd suggest
to call this variable restart_current_retry or so.

  #ifdef HAVE_SYSV_COMPAT
  s-sysv_start_priority = -1;
  s-sysv_start_priority_from_rcnd = -1;
 @@ -1151,6 +1153,22 @@
  if (s-meta.default_dependencies)
  if ((r = service_add_default_dependencies(s))  0)
  return r;
 +
 +/* If the option RestartRetries= is set then the service 
 will be revived */
 +if (s-restart_retries != DEFAULT_RESTART_RETRIES){
 +/* If the option Restart= is not set, then for the 
 service to be revived
 +   we must set Restart= to on-failure */
 +if (s-restart == SERVICE_RESTART_NO){
 +s-restart = SERVICE_RESTART_ON_FAILURE;
 +}

Please follow coding style. We generally place no {} brackets around
single line blocks.

  case SERVICE_AUTO_RESTART:
 -log_info(%s holdoff time over, scheduling restart., 
 u-meta.id);
 +if (0= s-restart_retries){

Please follow coding style, compare variables with constants not
constants with variables. Also, please place spaces before the = and
the {.

 
 +if (u-meta.type == UNIT_SERVICE
 +u-service.restart_crt_retry= u-service.restart_retries)
 +return;
 +

Huhm, I wonder if we could fine a better place for this, not sure where though.

  log_info(Triggering OnFailure= dependencies of %s., u-meta.id);
 
  SET_FOREACH(other, u-meta.dependencies[UNIT_ON_FAILURE], i) {
 @@ -1245,6 +1249,17 @@
  log_notice(Unit %s entered failed state., 
 u-meta.id);
  unit_trigger_on_failure(u);
  }
 +
 +/* When a unit is of type service and has finished,
 +   reset restart-retry counter, if applicable */
 +if (ns != os
 +u-meta.type == UNIT_SERVICE
 +UNIT_IS_INACTIVE_OR_FAILED(ns)
 +u-service.restart_crt_retry  
 u-service.restart_retries ){
 +log_debug(Unit %s entered inactive or failed, 
 restart retries at max, reset counter (%d -  0)!,
 +  u-meta.id, 
 u-service.restart_crt_retry, u-service.restart_retries);
 +u-service.restart_crt_retry = 0;

Hmm, I think it would be nicer to reset this counter in service.c only, not
in the generic code. I'd like to avoid that we access too many
service-private fields from generic unit code.

  #include execute.h
  #include condition.h
 
 +/* default = infinite retries (= systemd behaviour not patched) */
 +#define DEFAULT_RESTART_RETRIES -1
 +

Hee, if I commit this, then it won't be patched anymore, so the comment
should not refer to it being unpatched ;-)

Patch looks quite OK in general.

Thanks for the work,

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel