[systemd-devel] [PATCH] timedated: gather timezone from /etc/localtime sym target

2012-08-06 Thread Shawn Landen
keep other method for now, consider dropping later.

Supporting relative links here could be problematic as timezones in
/usr/share/zoneinfo are often themselves symlinks (and symlinks to
symlinks), so this implamentation only only support absolute links.
---
 src/timedate/timedated.c |   28 
 1 file changed, 28 insertions(+)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 09fd808..456e409 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "util.h"
 #include "strv.h"
@@ -174,9 +175,35 @@ static void verify_timezone(void) {
 
 static int read_data(void) {
 int r;
+struct stat st;
 
 free_data();
 
+r = lstat("/etc/localtime", &st);
+if (r < 0) {
+log_warning("lstat() of %s failed: %m", "/etc/localtime");
+} else if (!S_ISLNK(st.st_mode)) {
+log_warning("/etc/localtime should be an absolute symlink to a 
timezone data file in /usr/share/zoneinfo/");
+} else {
+char *t;
+
+r = readlink_malloc("/etc/localtime", &t);
+if (r < 0) {
+log_warning("Failed to get target of %s: %m", 
"/etc/localtime");
+} else if (!startswith(t, "/usr/share/zoneinfo/")) {
+log_warning("/etc/localtime should be an absolute 
symlink to a timezone data file in /usr/share/zoneinfo/");
+} else {
+tz.zone = strdup(t + strlen("/usr/share/zoneinfo/"));
+free(t);
+if (!tz.zone)
+return log_oom();
+
+goto have_timezone;
+}
+
+free(t);
+}
+
 r = read_one_line_file("/etc/timezone", &tz.zone);
 if (r < 0) {
 if (r != -ENOENT)
@@ -192,6 +219,7 @@ static int read_data(void) {
 #endif
 }
 
+have_timezone:
 if (isempty(tz.zone)) {
 free(tz.zone);
 tz.zone = NULL;
-- 
1.7.10.4

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


Re: [systemd-devel] Set environment variable system-wide

2012-08-06 Thread Lennart Poettering
On Mon, 06.08.12 23:09, Kay Sievers (k...@vrfy.org) wrote:

> 
> On Mon, Aug 6, 2012 at 10:49 PM, Daniel Drake  wrote:
> > Hi,
> >
> > I thought I read somewhere that systemd offers a mechanism to set an
> > environment variable system-wide - i.e. the variable assignment will
> > be present in all the processes started by systemd.
> >
> > But I can't find where I read this, or how to use it.
> >
> > Does this functionality exist or am I getting confused with something else?
> 
> systemctl set-environment ... ?

There's also systemd.setenv= on the kernel cmdline.

But as Kay said, don't use it for anything but debugging things. Most
likely you are doing something wrong if you think this is what you want.

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] Set environment variable system-wide

2012-08-06 Thread Daniel Drake
On Mon, Aug 6, 2012 at 3:09 PM, Kay Sievers  wrote:
> systemctl set-environment ... ?

Maybe thats what I read about.

In this case I'm looking to set it in early boot though, so that it
affects all spawned processes from the very start. Is there a nice way
of doing this?

> But it's in almost all use cases wrong to use anything like that isn't
> broken unix legacy that expects it that way.

I'm having trouble parsing that sentence. In this case I'm looking for
a clean way to set PYTHONOPTIMIZE system wide (to enable Python's
optimized bytecode usage).

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


Re: [systemd-devel] Set environment variable system-wide

2012-08-06 Thread Kay Sievers
On Mon, Aug 6, 2012 at 10:49 PM, Daniel Drake  wrote:
> Hi,
>
> I thought I read somewhere that systemd offers a mechanism to set an
> environment variable system-wide - i.e. the variable assignment will
> be present in all the processes started by systemd.
>
> But I can't find where I read this, or how to use it.
>
> Does this functionality exist or am I getting confused with something else?

systemctl set-environment ... ?

But it's in almost all use cases wrong to use anything like that isn't
broken unix legacy that expects it that way.

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


[systemd-devel] Set environment variable system-wide

2012-08-06 Thread Daniel Drake
Hi,

I thought I read somewhere that systemd offers a mechanism to set an
environment variable system-wide - i.e. the variable assignment will
be present in all the processes started by systemd.

But I can't find where I read this, or how to use it.

Does this functionality exist or am I getting confused with something else?

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


[systemd-devel] [PATCH] systemd: Fail a service if it exceeds its start limit

2012-08-06 Thread Eelco Dolstra
Previously, if a service configured to restart automatically exceeds
its start limit, it entered the "inactive/dead" state.  That seems
wrong to me, since there is nothing to indicate to the user that the
service has failed.  This patch causes it to enter the failed state
instead.
---
 src/core/service.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/core/service.c b/src/core/service.c
index 1c127bd..eafdbe5 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2487,6 +2487,7 @@ static int service_start(Unit *u) {
 r = service_start_limit_test(s);
 if (r < 0) {
 service_notify_sockets_dead(s, true);
+service_set_state(s, SERVICE_FAILED);
 return r;
 }
 
-- 
1.7.11.4

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


Re: [systemd-devel] [PATCH] use "Out of memory." consistantly (or with "\n")

2012-08-06 Thread shawn
On Mon, 2012-08-06 at 16:39 +0200, Lennart Poettering wrote: 
> On Fri, 03.08.12 17:33, shawn (shawnland...@gmail.com) wrote:
> 
> Applied!
> 

ha! clang didn't find that careless typo you fixed, while gcc points it
out. I call BS on the "better diagnostics".

> Note that we should be carefuly with adding additional erorr messages to
> the EXIT_SUCCESS/EXIT_FAILURE bits, since we might end up with two error
> messages instead of one, which is something we should try to avoid...
> 
> Lennart


Yeah... I was kinda negligent

I think this is better. 
> 
> > On Thu, 2012-07-26 at 11:51 +0200, Kay Sievers wrote: 
> > > On Thu, Jul 26, 2012 at 1:17 AM, shawn  wrote:
> > > > On Wed, 2012-07-25 at 18:11 +0200, Lennart Poettering wrote:
> > > >> On Wed, 25.07.12 11:24, Kay Sievers (k...@vrfy.org) wrote:
> > > >> > On Wed, Jul 25, 2012 at 6:12 AM, Shawn Landden 
> > > >> >  wrote:
> > > >> > > glibc/glib both use "out of memory" consistantly so maybe we should
> > > >> > > consider that instead of this.
> > > >> > >
> > > >> > > Eliminates one string out of a number of binaries. Also fixes 
> > > >> > > extra newline
> > > >> > > in udev/scsi_id
> > > >> >
> > > >> > Applied.
> > > >>
> > > >> Hmm, given that we might run into this again, it might make sense to
> > > >> define function for this? Maybe something like this in log.h?
> > > >>
> > > >> static inline void log_oom(void) {
> > > >>log_error("Out of memory.");
> > > >>return -ENOMEM;
> > > >> }
> > > >>
> > > >> Which we then could use everywhere?
> > > >
> > > > patch that does that
> > > 
> > > Applied.
> > 
> > another
> > 
> 
> > >From 9f7a1f61c733fa90874d9f813589ece9afbae942 Mon Sep 17 00:00:00 2001
> > From: Shawn Landden 
> > Date: Fri, 3 Aug 2012 17:22:09 -0700
> > Subject: [PATCH] continue work with error messages, log_oom()
> > 
> > Adds messages for formally silent errors: new "Failed on cmdline argument 
> > %s: %s".
> > 
> > Removes some specific error messages for -ENOMEM in mount-setup.c. A few 
> > specific
> > ones have been left in other binaries.
> > ---
> >  TODO   |2 +-
> >  src/binfmt/binfmt.c|4 ++--
> >  src/cgtop/cgtop.c  |7 ++-
> >  src/core/main.c|   34 --
> >  src/core/mount-setup.c |   12 
> >  5 files changed, 33 insertions(+), 26 deletions(-)
> > 
> > diff --git a/TODO b/TODO
> > index ac9bae4..c694ce0 100644
> > --- a/TODO
> > +++ b/TODO
> > @@ -460,7 +460,7 @@ Regularly:
> >  
> >  * Use PR_SET_PROCTITLE_AREA if it becomes available in the kernel
> >  
> > -* %m in printf() instead of strerror();
> > +* %m in printf() instead of strerror(errno);
> >  
> >  * pahole
> >  
> > diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c
> > index 0399ab7..788fd4b 100644
> > --- a/src/binfmt/binfmt.c
> > +++ b/src/binfmt/binfmt.c
> > @@ -40,7 +40,7 @@ static int delete_rule(const char *rule) {
> >  assert(rule[0]);
> >  
> >  if (!(x = strdup(rule)))
> > -return -ENOMEM;
> > +return log_oom();
> >  
> >  e = strchrnul(x+1, x[0]);
> >  *e = 0;
> > @@ -49,7 +49,7 @@ static int delete_rule(const char *rule) {
> >  free(x);
> >  
> >  if (!fn)
> > -return -ENOMEM;
> > +return log_oom();
> >  
> >  r = write_one_line_file(fn, "-1");
> >  free(fn);
> > diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
> > index a57a468..3756328 100644
> > --- a/src/cgtop/cgtop.c
> > +++ b/src/cgtop/cgtop.c
> > @@ -782,5 +782,10 @@ finish:
> >  group_hashmap_free(a);
> >  group_hashmap_free(b);
> >  
> > -return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
> > +if (r < 0) {
> > +log_error("Exiting with failure: %s", strerror(-r));
> > +return EXIT_FAILURE;
> > +}
> > +
> > +return EXIT_SUCCESS;
> >  }
> > diff --git a/src/core/main.c b/src/core/main.c
> > index 1326b94..3c0f5f9 100644
> > --- a/src/core/main.c
> > +++ b/src/core/main.c
> > @@ -168,12 +168,12 @@ _noreturn_ static void crash(int sig) {
> >  
> >  pid = fork();
> >  if (pid < 0)
> > -log_error("Failed to fork off crash shell: %s", 
> > strerror(errno));
> > +log_error("Failed to fork off crash shell: %m");
> >  else if (pid == 0) {
> >  make_console_stdio();
> >  execl("/bin/sh", "/bin/sh", NULL);
> >  
> > -log_error("execl() failed: %s", strerror(errno));
> > +log_error("execl() failed: %m");
> >  _exit(1);
> >  }
> >  
> > @@ -350,12 +350,12 @@ static int parse_proc_cmdline_word(const char *word) {
> >  if (!eq) {
> >  r = unsetenv(cenv);
> >  if (r < 0)
> > -log_war

Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Adam Spragg
On Monday 06 Aug 2012 12:16:02 Lennart Poettering wrote:
> On Mon, 06.08.12 12:06, Kay Sievers (k...@vrfy.org) wrote:
> > 
> > How about:
> >   ExitStatusFailure=
> >   ExitStatusSuccess=
> 
> success and failure should be parititions of
> the exit status set, i.e. all exits that are not failure are success and
> all that are success are not failure, and there is nothing but
> success/failure in the set.

Could allow either, but make it an error to use both. So if you configure 
ExitStatusSuccess, then any values not listed are failures, but if you 
configure ExitStatusFailure, then any values not listed are successes.

Or just have ExitStatusSuccess, but allow "!" as the first character which 
negates the list, meaning "any status except those listed is success"

Maybe?

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


Re: [systemd-devel] Re-exec()ing services for 'systemctl restart' ?

2012-08-06 Thread Kay Sievers
On Mon, Aug 6, 2012 at 6:14 PM, Daniel P. Berrange  wrote:
> On Mon, Aug 06, 2012 at 06:04:11PM +0200, Kay Sievers wrote:
>> On Mon, Aug 6, 2012 at 5:52 PM, Daniel P. Berrange  
>> wrote:
>> > For libvirt, we (will soon) have a daemon (virtlockd) which maintains
>> > exclusive fcntl() based locks on disk images/devices, on behalf of both
>> > libvirtd and any running QEMU or LXC instances. This is a safety critical
>> > daemon (hence separate from libvirtd), to the extent that if the daemon
>> > stops / crashes, the entire host should be immediately fenced using a
>> > kernel watchdog and/or hardware power control device.
>> >
>> > We still want to be able to restart this daemon during RPM upgrades to
>> > newer versions, but we can't use a normal stop+start sequence, because
>> > that will loose locks for any active VMs. Thus the daemon has the ability
>> > to re-exec() itself triggered by SIGUSR1, preserving its critical state.
>> > I've read the manpages for .service, .exec, etc but I've not seen any
>> > reference to changing config such that
>> >
>> >   # systemctl restart virtdlockd.service
>> >
>> > will simply send SIGUSR1 to the process, instead of stopping it and then
>> > starting it again. Obviously I could make the RPM %post send SIGUSR1
>> > directly and ignore systemctl, but that doesn't help admins who just
>> > expect to use systemctl. So I want to know if there is a recommended
>> > way to handle this kind of use case ?
>>
>> $ systemctl reload ... ?
>
> I thought about reload, but using that to re-exec the daemon seemed
> a little evil to me, since it was really for just reloading config
> files.

It's probably a not too interesting detail for systemd, the PID will
stay the same, so only reloading the config or also doing an exec() is
all not really visible to systemd. You can think of reloading the
executable image not only its config. :)

You know about the sd_notify(3) API, right? It might be nice to update
systemd with what the service is doing here.

>> or with the signal speficied:
>>
>> $ systemctl kill ...
>
> True, I guess I could use that.
>
> I'm infering from your response that there's no way to customize what
> 'restart' does, as you can do with stop/start/reload/etc.

Right, restart is more a stop+start, where the PID usually changes and
systemd needs to re-init the supervision/tracking.

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


Re: [systemd-devel] Re-exec()ing services for 'systemctl restart' ?

2012-08-06 Thread Daniel P. Berrange
On Mon, Aug 06, 2012 at 06:04:11PM +0200, Kay Sievers wrote:
> On Mon, Aug 6, 2012 at 5:52 PM, Daniel P. Berrange  
> wrote:
> > For libvirt, we (will soon) have a daemon (virtlockd) which maintains
> > exclusive fcntl() based locks on disk images/devices, on behalf of both
> > libvirtd and any running QEMU or LXC instances. This is a safety critical
> > daemon (hence separate from libvirtd), to the extent that if the daemon
> > stops / crashes, the entire host should be immediately fenced using a
> > kernel watchdog and/or hardware power control device.
> >
> > We still want to be able to restart this daemon during RPM upgrades to
> > newer versions, but we can't use a normal stop+start sequence, because
> > that will loose locks for any active VMs. Thus the daemon has the ability
> > to re-exec() itself triggered by SIGUSR1, preserving its critical state.
> > I've read the manpages for .service, .exec, etc but I've not seen any
> > reference to changing config such that
> >
> >   # systemctl restart virtdlockd.service
> >
> > will simply send SIGUSR1 to the process, instead of stopping it and then
> > starting it again. Obviously I could make the RPM %post send SIGUSR1
> > directly and ignore systemctl, but that doesn't help admins who just
> > expect to use systemctl. So I want to know if there is a recommended
> > way to handle this kind of use case ?
> 
> $ systemctl reload ... ?

I thought about reload, but using that to re-exec the daemon seemed
a little evil to me, since it was really for just reloading config
files.

> or with the signal speficied:
> 
> $ systemctl kill ...

True, I guess I could use that.

I'm infering from your response that there's no way to customize what
'restart' does, as you can do with stop/start/reload/etc.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Re-exec()ing services for 'systemctl restart' ?

2012-08-06 Thread Kay Sievers
On Mon, Aug 6, 2012 at 5:52 PM, Daniel P. Berrange  wrote:
> For libvirt, we (will soon) have a daemon (virtlockd) which maintains
> exclusive fcntl() based locks on disk images/devices, on behalf of both
> libvirtd and any running QEMU or LXC instances. This is a safety critical
> daemon (hence separate from libvirtd), to the extent that if the daemon
> stops / crashes, the entire host should be immediately fenced using a
> kernel watchdog and/or hardware power control device.
>
> We still want to be able to restart this daemon during RPM upgrades to
> newer versions, but we can't use a normal stop+start sequence, because
> that will loose locks for any active VMs. Thus the daemon has the ability
> to re-exec() itself triggered by SIGUSR1, preserving its critical state.
> I've read the manpages for .service, .exec, etc but I've not seen any
> reference to changing config such that
>
>   # systemctl restart virtdlockd.service
>
> will simply send SIGUSR1 to the process, instead of stopping it and then
> starting it again. Obviously I could make the RPM %post send SIGUSR1
> directly and ignore systemctl, but that doesn't help admins who just
> expect to use systemctl. So I want to know if there is a recommended
> way to handle this kind of use case ?

$ systemctl reload ... ?

or with the signal speficied:

$ systemctl kill ...

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


[systemd-devel] Re-exec()ing services for 'systemctl restart' ?

2012-08-06 Thread Daniel P. Berrange
For libvirt, we (will soon) have a daemon (virtlockd) which maintains
exclusive fcntl() based locks on disk images/devices, on behalf of both
libvirtd and any running QEMU or LXC instances. This is a safety critical
daemon (hence separate from libvirtd), to the extent that if the daemon
stops / crashes, the entire host should be immediately fenced using a
kernel watchdog and/or hardware power control device.

We still want to be able to restart this daemon during RPM upgrades to
newer versions, but we can't use a normal stop+start sequence, because
that will loose locks for any active VMs. Thus the daemon has the ability
to re-exec() itself triggered by SIGUSR1, preserving its critical state.
I've read the manpages for .service, .exec, etc but I've not seen any
reference to changing config such that

  # systemctl restart virtdlockd.service

will simply send SIGUSR1 to the process, instead of stopping it and then
starting it again. Obviously I could make the RPM %post send SIGUSR1
directly and ignore systemctl, but that doesn't help admins who just
expect to use systemctl. So I want to know if there is a recommended
way to handle this kind of use case ?

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Michael Biebl
2012/8/3 Lennart Poettering :
> On Tue, 24.07.12 16:43, Lukas Nykryn (lnyk...@redhat.com) wrote:
>
> Maybe DontRestartExitStatus=? The libc calls the generalization of
> exit code and exit signal the "exit status", so that sounds like the
> best term to use here.
>
> And then people could write:
>
>DontRestartExitStatus=SIGTERM 1 2 3 4

RestartNormalExitStatus = ...


>
> One day, if we really need to make the failure/success sets configurable
> too, we could then add:
>
>DontFailExitStatus=SIGSEGV 1 2

NormalExitStatus = ...

Doesn't sound too bad to me.

Michael
-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Lukáš Nykrýn
Lennart Poettering píše v Po 06. 08. 2012 v 16:52 +0200:
> On Mon, 06.08.12 16:45, Lukáš Nykrýn (lnyk...@redhat.com) wrote:
> 
> > +if (!*set) {
> > +set_free(*set);
> > +*set = NULL;
> > +}
> 
> You probably mean if (*set) here, not (!*set). But honestly I think this
> bit should just go entirely as for most other settings we actually do
> allow adding in additional values later on if it makes sense (example:
> Environment=). 
> 
> > +
> > +*set = set_new(trivial_hash_func, trivial_compare_func);
> > +if (!*set)
> > +return -ENOMEM;
> 
> We probably should start using log_oom() for things like this now that
> it is available.
> 
> > +
> > +FOREACH_WORD(w, l, rvalue, state)
> > +{
> > +char *temp = new0(char, l + 1);
> > +if (!temp)
> > +return -ENOMEM;
> 
> Please use strndup() here! (And also log_oom() here please).
> 
> > +
> > +strncpy(temp, w, l);
> > +r = safe_atoi(temp, &val);
> > +free(temp);
> > +if (r < 0) {
> > +log_error("[%s:%u] Failed to parse numeric value: 
> > %s", filename, line, w);
> > +set_free(*set);
> > +*set = NULL;
> 
> I'd leave the hashmap set, as it is freed anyway when the service goes
> away, and makes simpler and easier to read.
> 
> > +return r;
> > +}
> > +r = set_put(*set, INT_TO_PTR(val));
> > +if (r < 0) {
> > +log_error("[%s:%u] Unable to store: %s", filename, 
> > line, w);
> > +set_free(*set);
> > +*set = NULL;
> 
> Same here.
> 
> Otherwise looks good.
> 
> (We want the signal stuff eventually I guess, but we can easily add this 
> later).
> 
> Lennart
> 
Again thanks for review. Here is modified patch.
If you think that it would be better to add signal stuff before
accepting this patch I will not disagree.

Lukas
>From 986dc67a74a4386e3e95f1b09cfa36dd71b77321 Mon Sep 17 00:00:00 2001
From: Lukas Nykryn 
Date: Mon, 6 Aug 2012 14:54:03 +0200
Subject: [PATCH] service: allow service to inhibit respawn with special
 return code

In some cases, like wrong configuration, restarting after error
does not help, so administrator can specify statuses by RestartPreventExitStatus
which will not cause restart of a service.
---
 man/systemd.service.xml   |7 
 src/core/load-fragment-gperf.gperf.m4 |1 +
 src/core/service.c|7 -
 src/core/service.h|1 +
 src/shared/conf-parser.c  |   50 +
 src/shared/conf-parser.h  |1 +
 6 files changed, 66 insertions(+), 1 deletions(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index f43201d..8533136 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -558,6 +558,13 @@
 
 
 
+RestartPreventExitStatus=
+Specify return codes list, which
+will prevent service from restart. Codes are
+separated by whitespace.
+
+
+
 PermissionsStartOnly=
 Takes a boolean
 argument. If true, the permission
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 2b1cfa0..d077376 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -156,6 +156,7 @@ Service.PermissionsStartOnly,config_parse_bool,  0,
 Service.RootDirectoryStartOnly,  config_parse_bool,  0, offsetof(Service, root_directory_start_only)
 Service.RemainAfterExit, config_parse_bool,  0, offsetof(Service, remain_after_exit)
 Service.GuessMainPID,config_parse_bool,  0, offsetof(Service, guess_main_pid)
+Service.RestartPreventExitStatus, config_parse_set_int,   0, offsetof(Service, restart_ignore_status)
 m4_ifdef(`HAVE_SYSV_COMPAT',
 `Service.SysVStartPriority,  config_parse_sysv_priority, 0, offsetof(Service, sysv_start_priority)',
 `Service.SysVStartPriority,  config_parse_warn_compat,   0, 0')
diff --git a/src/core/service.c b/src/core/service.c
index 1c127bd..ae894b7 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -293,6 +293,9 @@ static void service_done(Unit *u) {
 s->control_command = NULL;
 s->main_com

Re: [systemd-devel] [PATCH] use "Out of memory." consistantly (or with "\n")

2012-08-06 Thread Lennart Poettering
On Mon, 06.08.12 16:47, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> 
> On 08/06/2012 04:39 PM, Lennart Poettering wrote:
> > On Fri, 03.08.12 17:33, shawn (shawnland...@gmail.com) wrote:
> > 
> > Applied!
> > 
> > Note that we should be carefuly with adding additional erorr messages to
> > the EXIT_SUCCESS/EXIT_FAILURE bits, since we might end up with two error
> > messages instead of one, which is something we should try to avoid...
> Can we specify that the policy is that the oom message is logged at the
> point where the NULL return value is converted into a ENOMEM return code?

It's probably not that easy. I mean, stuff such as hashmap_new()
probably should never log this kind of thing on its own.

So far we tried to follow the logic that "library"-like calls never log
on their own, and that logging is always done only in the final
tools. But this is followed only very losely.

I guess one could formulate a rule like this: calls in src/shared/
should never log, but other code should, but about a thousand exceptions
of this are OK...

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] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Lennart Poettering
On Mon, 06.08.12 16:45, Lukáš Nykrýn (lnyk...@redhat.com) wrote:

> +if (!*set) {
> +set_free(*set);
> +*set = NULL;
> +}

You probably mean if (*set) here, not (!*set). But honestly I think this
bit should just go entirely as for most other settings we actually do
allow adding in additional values later on if it makes sense (example:
Environment=). 

> +
> +*set = set_new(trivial_hash_func, trivial_compare_func);
> +if (!*set)
> +return -ENOMEM;

We probably should start using log_oom() for things like this now that
it is available.

> +
> +FOREACH_WORD(w, l, rvalue, state)
> +{
> +char *temp = new0(char, l + 1);
> +if (!temp)
> +return -ENOMEM;

Please use strndup() here! (And also log_oom() here please).

> +
> +strncpy(temp, w, l);
> +r = safe_atoi(temp, &val);
> +free(temp);
> +if (r < 0) {
> +log_error("[%s:%u] Failed to parse numeric value: 
> %s", filename, line, w);
> +set_free(*set);
> +*set = NULL;

I'd leave the hashmap set, as it is freed anyway when the service goes
away, and makes simpler and easier to read.

> +return r;
> +}
> +r = set_put(*set, INT_TO_PTR(val));
> +if (r < 0) {
> +log_error("[%s:%u] Unable to store: %s", filename, 
> line, w);
> +set_free(*set);
> +*set = NULL;

Same here.

Otherwise looks good.

(We want the signal stuff eventually I guess, but we can easily add this later).

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] use "Out of memory." consistantly (or with "\n")

2012-08-06 Thread Zbigniew Jędrzejewski-Szmek
On 08/06/2012 04:39 PM, Lennart Poettering wrote:
> On Fri, 03.08.12 17:33, shawn (shawnland...@gmail.com) wrote:
> 
> Applied!
> 
> Note that we should be carefuly with adding additional erorr messages to
> the EXIT_SUCCESS/EXIT_FAILURE bits, since we might end up with two error
> messages instead of one, which is something we should try to avoid...
Can we specify that the policy is that the oom message is logged at the
point where the NULL return value is converted into a ENOMEM return code?

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


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Lukáš Nykrýn
Lukáš Nykrýn píše v Po 06. 08. 2012 v 15:26 +0200:
> Lennart Poettering píše v Pá 03. 08. 2012 v 20:46 +0200:
> > On Tue, 24.07.12 16:43, Lukas Nykryn (lnyk...@redhat.com) wrote:
> > 
> > > In some cases, like wrong configuration, restarting after error
> > > exit code does not help, so administrator can specify RestartIgnoreCodes
> > > which will not cause restart of a service.
> > 
> > I am not happy with the term "Code" here, it's a bit too generic to be
> > self explanatory.
> > 
> > And I think I agree with Zbigniew to a certain degree: we might want to
> > think about whether we should also allow configuring non-failure exit
> > codes or so. I mean, I am not suggesting we should implement that
> > right-away, but just keep in mind how that setting would look like so
> > that we can keep things uniform if we add it later. (In fact, I'd
> > recommend not to implement it right away: adding an option nobody so far
> > needed seems like a bad idea if we want to keep our set of options
> > minimal).
> > 
> > Also, I think we need to think further than just exit codes,
> > i.e. signals and stuff.
> > 
> > Maybe DontRestartExitStatus=? The libc calls the generalization of
> > exit code and exit signal the "exit status", so that sounds like the
> > best term to use here.
> > 
> > And then people could write:
> > 
> >DontRestartExitStatus=SIGTERM 1 2 3 4
> > 
> > or something like this?
> > 
> > (Of course, the "don't" in the name is a negative option, which we try
> > to avoid, but it appears weird to have a positive option for this, so i
> > think it would be ok...)
> > 
> > One day, if we really need to make the failure/success sets configurable
> > too, we could then add:
> > 
> >DontFailExitStatus=SIGSEGV 1 2
> > 
> > (But please, don't implement this bit just yet, let's wait for somebody
> > actually needing this. Note though, that Upstart actually does have
> > functionality like this).
> > 
> > > +*ignored = new(int, k);
> > 
> > Maybe use a bitfield here, like we do for the syscalls list?
> > 
> > (Actualy two bitfields, one for the exit codes, one for the exit signals).
> > 
> > > +static int check_ignored_rc(Service *s){
> > 
> > Please don't use acronyms in symbols if it's easy to avoid them and no
> > strong incentive to use them.
> > 
> > > +int n_restart_ignored_codes;
> > 
> > Generally we try to used "unsigned" rather than "int" for values where
> > negative values really never make sense, like in this case. It gives
> > readers a bit of a hint that this field never can be negative and no
> > special cases like that are used.
> > 
> > Otherwise looks good.
> > 
> > Lennart
> > 
> 
> Thanks for the feedback. I have altered name of the option to
> RestartIgnoreExitStatus, but feel free to change to anything else and I
> have also stored the codes to a set instead of an array.
> 
> Lukas

forgotten patch

>From 81ffeefb01aa73c3be2b2bba7500df5f8ebcdfc9 Mon Sep 17 00:00:00 2001
From: Lukas Nykryn 
Date: Mon, 6 Aug 2012 14:54:03 +0200
Subject: [PATCH] service: allow service to inhibit respawn with special
 return code

In some cases, like wrong configuration, restarting after error
does not help, so administrator can specify statuses by RestartPreventExitStatus
which will not cause restart of a service.
---
 man/systemd.service.xml   |7 
 src/core/load-fragment-gperf.gperf.m4 |1 +
 src/core/service.c|7 +++-
 src/core/service.h|1 +
 src/shared/conf-parser.c  |   60 +
 src/shared/conf-parser.h  |1 +
 6 files changed, 76 insertions(+), 1 deletions(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index f43201d..8533136 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -558,6 +558,13 @@
 
 
 
+RestartPreventExitStatus=
+Specify return codes list, which
+will prevent service from restart. Codes are
+separated by whitespace.
+
+
+
 PermissionsStartOnly=
 Takes a boolean
 argument. If true, the permission
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 2b1cfa0..d077376 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -156,6 +156,7 @@ Service.PermissionsStartOnly,config_parse_bool,  0,
 Service.RootDirectoryStartOnly,  config_parse_bool,  0, offsetof(Service, root_directory_start_only)
 Service.RemainAfterExit, config_parse_bool,  0, offsetof(Service, remain_after_exit)
 Service.GuessMainPI

Re: [systemd-devel] [PATCH] use "Out of memory." consistantly (or with "\n")

2012-08-06 Thread Lennart Poettering
On Fri, 03.08.12 17:33, shawn (shawnland...@gmail.com) wrote:

Applied!

Note that we should be carefuly with adding additional erorr messages to
the EXIT_SUCCESS/EXIT_FAILURE bits, since we might end up with two error
messages instead of one, which is something we should try to avoid...

Lennart

> On Thu, 2012-07-26 at 11:51 +0200, Kay Sievers wrote: 
> > On Thu, Jul 26, 2012 at 1:17 AM, shawn  wrote:
> > > On Wed, 2012-07-25 at 18:11 +0200, Lennart Poettering wrote:
> > >> On Wed, 25.07.12 11:24, Kay Sievers (k...@vrfy.org) wrote:
> > >> > On Wed, Jul 25, 2012 at 6:12 AM, Shawn Landden 
> > >> >  wrote:
> > >> > > glibc/glib both use "out of memory" consistantly so maybe we should
> > >> > > consider that instead of this.
> > >> > >
> > >> > > Eliminates one string out of a number of binaries. Also fixes extra 
> > >> > > newline
> > >> > > in udev/scsi_id
> > >> >
> > >> > Applied.
> > >>
> > >> Hmm, given that we might run into this again, it might make sense to
> > >> define function for this? Maybe something like this in log.h?
> > >>
> > >> static inline void log_oom(void) {
> > >>log_error("Out of memory.");
> > >>return -ENOMEM;
> > >> }
> > >>
> > >> Which we then could use everywhere?
> > >
> > > patch that does that
> > 
> > Applied.
> 
> another
> 

> >From 9f7a1f61c733fa90874d9f813589ece9afbae942 Mon Sep 17 00:00:00 2001
> From: Shawn Landden 
> Date: Fri, 3 Aug 2012 17:22:09 -0700
> Subject: [PATCH] continue work with error messages, log_oom()
> 
> Adds messages for formally silent errors: new "Failed on cmdline argument %s: 
> %s".
> 
> Removes some specific error messages for -ENOMEM in mount-setup.c. A few 
> specific
> ones have been left in other binaries.
> ---
>  TODO   |2 +-
>  src/binfmt/binfmt.c|4 ++--
>  src/cgtop/cgtop.c  |7 ++-
>  src/core/main.c|   34 --
>  src/core/mount-setup.c |   12 
>  5 files changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/TODO b/TODO
> index ac9bae4..c694ce0 100644
> --- a/TODO
> +++ b/TODO
> @@ -460,7 +460,7 @@ Regularly:
>  
>  * Use PR_SET_PROCTITLE_AREA if it becomes available in the kernel
>  
> -* %m in printf() instead of strerror();
> +* %m in printf() instead of strerror(errno);
>  
>  * pahole
>  
> diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c
> index 0399ab7..788fd4b 100644
> --- a/src/binfmt/binfmt.c
> +++ b/src/binfmt/binfmt.c
> @@ -40,7 +40,7 @@ static int delete_rule(const char *rule) {
>  assert(rule[0]);
>  
>  if (!(x = strdup(rule)))
> -return -ENOMEM;
> +return log_oom();
>  
>  e = strchrnul(x+1, x[0]);
>  *e = 0;
> @@ -49,7 +49,7 @@ static int delete_rule(const char *rule) {
>  free(x);
>  
>  if (!fn)
> -return -ENOMEM;
> +return log_oom();
>  
>  r = write_one_line_file(fn, "-1");
>  free(fn);
> diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
> index a57a468..3756328 100644
> --- a/src/cgtop/cgtop.c
> +++ b/src/cgtop/cgtop.c
> @@ -782,5 +782,10 @@ finish:
>  group_hashmap_free(a);
>  group_hashmap_free(b);
>  
> -return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
> +if (r < 0) {
> +log_error("Exiting with failure: %s", strerror(-r));
> +return EXIT_FAILURE;
> +}
> +
> +return EXIT_SUCCESS;
>  }
> diff --git a/src/core/main.c b/src/core/main.c
> index 1326b94..3c0f5f9 100644
> --- a/src/core/main.c
> +++ b/src/core/main.c
> @@ -168,12 +168,12 @@ _noreturn_ static void crash(int sig) {
>  
>  pid = fork();
>  if (pid < 0)
> -log_error("Failed to fork off crash shell: %s", 
> strerror(errno));
> +log_error("Failed to fork off crash shell: %m");
>  else if (pid == 0) {
>  make_console_stdio();
>  execl("/bin/sh", "/bin/sh", NULL);
>  
> -log_error("execl() failed: %s", strerror(errno));
> +log_error("execl() failed: %m");
>  _exit(1);
>  }
>  
> @@ -350,12 +350,12 @@ static int parse_proc_cmdline_word(const char *word) {
>  if (!eq) {
>  r = unsetenv(cenv);
>  if (r < 0)
> -log_warning("unsetenv failed %s. Ignoring.", 
> strerror(errno));
> +log_warning("unsetenv failed %m. Ignoring.");
>  } else {
>  *eq = 0;
>  r = setenv(cenv, eq + 1, 1);
>  if (r < 0)
> -log_warning("setenv failed %s. Ignoring.", 
> strerror(errno));
> +log_warning("setenv failed %m. Ignoring.");
>  }
>  

Re: [systemd-devel] Not sure if I am doing something wrong or if this is a bug.

2012-08-06 Thread Daniel J Walsh
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/03/2012 03:45 PM, Lennart Poettering wrote:
> On Mon, 30.07.12 17:13, Daniel J Walsh (dwa...@redhat.com) wrote:
> 
>> -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
>> 
>> In containers we are blocking systemd from creating containers.  If I try
>> to run httpd within a container it asks for PrivateTmp and SELinux stops
>> systemd from setting up the PrivateTmp.  In order to get around this, I
>> decided to try to create a unit file based off of the httpd unit file.
>> 
>> cat /etc/systemd/system/sandbox.target.wants/httpd.service
> 
> Files in .wants/ directory should be symlinks (since they just are used to
> express deps, not the actual services). Hence you want to place this 
> service file in /etc/systemd/system/httpd.service and then make 
> /etc/systemd/system/sandbox.target.wants/httpd.service a symlink to it.
> 
> And then use "systemctl daemon-reload" to actviate these changes. And use
> "systemctl show httpd.service" to check whether your changes were properly
> applied.
> 
> Lennart
> 
Yes I figured this all out last week.  It now seems to work pretty well.
Hopefull new versions of libvirt and libvirt-sandbox get pushed into Rawhide
this week, so we can get people playing with this.


-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlAfzEsACgkQrlYvE4MpobN8UgCfX6PYDgalQvTas57pIMk9l/Jl
7sgAnApiyv/NzY1m8N/PaNjUaYl8XAMz
=x1Tu
-END PGP SIGNATURE-
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Lukáš Nykrýn
Lennart Poettering píše v Pá 03. 08. 2012 v 20:46 +0200:
> On Tue, 24.07.12 16:43, Lukas Nykryn (lnyk...@redhat.com) wrote:
> 
> > In some cases, like wrong configuration, restarting after error
> > exit code does not help, so administrator can specify RestartIgnoreCodes
> > which will not cause restart of a service.
> 
> I am not happy with the term "Code" here, it's a bit too generic to be
> self explanatory.
> 
> And I think I agree with Zbigniew to a certain degree: we might want to
> think about whether we should also allow configuring non-failure exit
> codes or so. I mean, I am not suggesting we should implement that
> right-away, but just keep in mind how that setting would look like so
> that we can keep things uniform if we add it later. (In fact, I'd
> recommend not to implement it right away: adding an option nobody so far
> needed seems like a bad idea if we want to keep our set of options
> minimal).
> 
> Also, I think we need to think further than just exit codes,
> i.e. signals and stuff.
> 
> Maybe DontRestartExitStatus=? The libc calls the generalization of
> exit code and exit signal the "exit status", so that sounds like the
> best term to use here.
> 
> And then people could write:
> 
>DontRestartExitStatus=SIGTERM 1 2 3 4
> 
> or something like this?
> 
> (Of course, the "don't" in the name is a negative option, which we try
> to avoid, but it appears weird to have a positive option for this, so i
> think it would be ok...)
> 
> One day, if we really need to make the failure/success sets configurable
> too, we could then add:
> 
>DontFailExitStatus=SIGSEGV 1 2
> 
> (But please, don't implement this bit just yet, let's wait for somebody
> actually needing this. Note though, that Upstart actually does have
> functionality like this).
> 
> > +*ignored = new(int, k);
> 
> Maybe use a bitfield here, like we do for the syscalls list?
> 
> (Actualy two bitfields, one for the exit codes, one for the exit signals).
> 
> > +static int check_ignored_rc(Service *s){
> 
> Please don't use acronyms in symbols if it's easy to avoid them and no
> strong incentive to use them.
> 
> > +int n_restart_ignored_codes;
> 
> Generally we try to used "unsigned" rather than "int" for values where
> negative values really never make sense, like in this case. It gives
> readers a bit of a hint that this field never can be negative and no
> special cases like that are used.
> 
> Otherwise looks good.
> 
> Lennart
> 

Thanks for the feedback. I have altered name of the option to
RestartIgnoreExitStatus, but feel free to change to anything else and I
have also stored the codes to a set instead of an array.

Lukas


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


Re: [systemd-devel] [PATCH] execute: Fix seccomp support on x32

2012-08-06 Thread Zbigniew Jędrzejewski-Szmek
On 08/06/2012 01:19 PM, Lennart Poettering wrote:
> On Sat, 04.08.12 10:50, Bryan Kadzban (br...@kadzban.is-a-geek.net) wrote:
> 
>>
>> microcai wrote:
>>> 2012/8/4 Lennart Poettering :
 On Tue, 24.07.12 22:45, Jonathan Callen (a...@gentoo.org) wrote:

> In the x32 ABI, syscall numbers start at 0x4000.  Mask that
> bit on x32 for lookups in the syscall_names array and
> syscall_filter and ensure that syscall.h is parsed correctly.
 Hmpf, can't say I find this patch particularly beautiful?

Why? Patch is mostly self-contained, shouldn't affect other
architectures, and actually changes just a few lines. Even if the
implementation is later on changed to use libseccomp, I don't see why we
shouldn't apply this now to make life easier for the x32 porters.

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


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Kay Sievers
On Mon, Aug 6, 2012 at 1:16 PM, Lennart Poettering
 wrote:
> On Mon, 06.08.12 12:06, Kay Sievers (k...@vrfy.org) wrote:

>> > Not sure if "Inhibit*" is maybe used somewhere already that would make
>> > this less desirable tho'.
>>
>> We have:
>>   Restart=on-success | on-failure | ...
>>
>> Does this setting really binds to the "Restart" logic only?
>
> Yes it does. PreventRestartExitStatus= would configure something that is
> orthogonal to failure/success.

That's why we have on-failure and on-success in the restart logic, right? :)

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


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Kay Sievers
On Mon, Aug 6, 2012 at 1:12 PM, Lennart Poettering
 wrote:
> On Mon, 06.08.12 01:14, Colin Guthrie (gm...@colin.guthr.ie) wrote:
>
>>
>> 'Twas brillig, and Lennart Poettering at 03/08/12 19:46 did gyre and gimble:
>> > Maybe DontRestartExitStatus=? The libc calls the generalization of
>> > exit code and exit signal the "exit status", so that sounds like the
>> > best term to use here.
>>
>> Would InhibitRestartExitStatus= work for you? Something feels wrong with
>> Dont* (partly because "don't" is a contraction of "do not" and
>> DoNotRestartExitStatus just feels wrong too).
>>
>> Not sure if "Inhibit*" is maybe used somewhere already that would make
>> this less desirable tho'.
>
> Hmm, using the word "inhibit" in this context might be confusing, since
> we already have this suspend/shutdown inibition framework and tools like
> "systemd-inhibit", which are unrelated to this.
>
> But what about "Prevent"?
>
> PreventRestartExitStatus= makes a lot of sense to me, but then again, I
> am not a native speaker... Does that sound good to you, too?

Starting key names with verbs or attributes is a mess. If it's really
bound to Restart, it should probably be RestartIgnoreExitStatus or
something like that instead.

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


Re: [systemd-devel] [PATCH] execute: Fix seccomp support on x32

2012-08-06 Thread Lennart Poettering
On Sat, 04.08.12 10:50, Bryan Kadzban (br...@kadzban.is-a-geek.net) wrote:

> 
> microcai wrote:
> > 2012/8/4 Lennart Poettering :
> >> On Tue, 24.07.12 22:45, Jonathan Callen (a...@gentoo.org) wrote:
> >> 
> >>> In the x32 ABI, syscall numbers start at 0x4000.  Mask that
> >>> bit on x32 for lookups in the syscall_names array and
> >>> syscall_filter and ensure that syscall.h is parsed correctly.
> >> Hmpf, can't say I find this patch particularly beautiful?
> >> 
> >> Can we solve this differently? For example, I'd be open to replace
> >> the direct seccomp code in systemd by some code based on libseccomp
> >> (now that libseccomp actually fixed its static global state
> >> issues). That way we should get this portability for free?
> > 
> > looks like some  source-based distro will blame you again :)
> 
> libseccomp uses pkg-config, so it's much less of a problem IMO;
> SECCOMP_CFLAGS=" " SECCOMP_LIBS=" " will allow configure to at least
> finish properly if the library is not present.  (Whether the system
> builds at all in this case is of course an issue that anyone sending
> those params to ./configure will have to deal with.)

libseccomp is a good candidate for an optional dep, much the same way as
the respective kernel option is.

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] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Lennart Poettering
On Mon, 06.08.12 12:06, Kay Sievers (k...@vrfy.org) wrote:

> 
> On Mon, Aug 6, 2012 at 2:14 AM, Colin Guthrie  wrote:
> > 'Twas brillig, and Lennart Poettering at 03/08/12 19:46 did gyre and gimble:
> >> Maybe DontRestartExitStatus=? The libc calls the generalization of
> >> exit code and exit signal the "exit status", so that sounds like the
> >> best term to use here.
> >
> > Would InhibitRestartExitStatus= work for you? Something feels wrong with
> > Dont* (partly because "don't" is a contraction of "do not" and
> > DoNotRestartExitStatus just feels wrong too).
> >
> > Not sure if "Inhibit*" is maybe used somewhere already that would make
> > this less desirable tho'.
> 
> We have:
>   Restart=on-success | on-failure | ...
> 
> Does this setting really binds to the "Restart" logic only?

Yes it does. PreventRestartExitStatus= would configure something that is
orthogonal to failure/success.

> How about:
>   ExitStatusFailure=
>   ExitStatusSuccess=

I'd claim that failure/success should be orthogonal to the restart
logic, and furthermore that success and failure should be parititions of
the exit status set, i.e. all exits that are not failure are success and
all that are success are not failure, and there is nothing but
success/failure in the set.

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] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Lennart Poettering
On Mon, 06.08.12 01:14, Colin Guthrie (gm...@colin.guthr.ie) wrote:

> 
> 'Twas brillig, and Lennart Poettering at 03/08/12 19:46 did gyre and gimble:
> > Maybe DontRestartExitStatus=? The libc calls the generalization of
> > exit code and exit signal the "exit status", so that sounds like the
> > best term to use here.
> 
> Would InhibitRestartExitStatus= work for you? Something feels wrong with
> Dont* (partly because "don't" is a contraction of "do not" and
> DoNotRestartExitStatus just feels wrong too).
> 
> Not sure if "Inhibit*" is maybe used somewhere already that would make
> this less desirable tho'.

Hmm, using the word "inhibit" in this context might be confusing, since
we already have this suspend/shutdown inibition framework and tools like
"systemd-inhibit", which are unrelated to this.

But what about "Prevent"? 

PreventRestartExitStatus= makes a lot of sense to me, but then again, I
am not a native speaker... Does that sound good to you, too?

Lennart

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


[systemd-devel] Question regarding ConditionPathExists= and systemctl status

2012-08-06 Thread Holger Freyther
Hi,

I have launched "systemd --user", created a bts.service file, added
'ConditionPathExists=' in the Unit section of my service. Then I
launched my service with 'systemctl --user start ...' and  as the
path does not exist, the condition_test fails and the service is not
started.

Next I tried to figure out if I my users would be able to understand
why the service was not started. This appears to be not possible.

The first part is that the Unit only holds condition_result and does
not store which test failed. E.g. if a Unit has multiple tests it
is not possible to know which test has failed. The debug output will
simply state that one condition has failed. The second issue is that
after the Unit has been garbage collected(???) the condition_result
is gone and will not be restored. My users would have to query the
status fast enough to maybe see the condition failure in the status.

Do you consider these issues worth fixing? Is this due running 
systemd as a user?

holger

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


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Kay Sievers
On Mon, Aug 6, 2012 at 2:14 AM, Colin Guthrie  wrote:
> 'Twas brillig, and Lennart Poettering at 03/08/12 19:46 did gyre and gimble:
>> Maybe DontRestartExitStatus=? The libc calls the generalization of
>> exit code and exit signal the "exit status", so that sounds like the
>> best term to use here.
>
> Would InhibitRestartExitStatus= work for you? Something feels wrong with
> Dont* (partly because "don't" is a contraction of "do not" and
> DoNotRestartExitStatus just feels wrong too).
>
> Not sure if "Inhibit*" is maybe used somewhere already that would make
> this less desirable tho'.

We have:
  Restart=on-success | on-failure | ...

Does this setting really binds to the "Restart" logic only?

How about:
  ExitStatusFailure=
  ExitStatusSuccess=

which matches the on-* values in Restart=, but also the C constants
for exit(), and we could possibly re-use these lists in other
conditions?

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


Re: [systemd-devel] [PATCH] service: allow service to inhibit respawn with special return code

2012-08-06 Thread Michael Biebl
2012/8/6 Colin Guthrie :
> 'Twas brillig, and Lennart Poettering at 03/08/12 19:46 did gyre and gimble:
>> Maybe DontRestartExitStatus=? The libc calls the generalization of
>> exit code and exit signal the "exit status", so that sounds like the
>> best term to use here.
>
> Would InhibitRestartExitStatus= work for you? Something feels wrong with
> Dont* (partly because "don't" is a contraction of "do not" and
> DoNotRestartExitStatus just feels wrong too).
>
> Not sure if "Inhibit*" is maybe used somewhere already that would make
> this less desirable tho'.

fwiw, upstart calls that configuration option "normal exit" [1]

Michael

[1] http://upstart.ubuntu.com/cookbook/#normal-exit



-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel