Re: [systemd-devel] Systemd askpassword

2014-04-12 Thread Ismael Bouya
Hi,
Thanks for your answer!
The attached journals (described below) are here:
http://www.normalesup.org/~bouya/temp/systemd/

So after investigation I have to ponder the "hang forever". In the attached
journal file (b2) it dies by himself after exactly one minute. Sometimes I
manually reset it through sysRq after 1min+10sec (b7), and the first time
it happened I'm sure I waited more than 2min before doing the hard reset
(estimate given by the --list-boots times between the last line of the
first incriminated boot and the start of the next one). This last case I
couldn't reproduce with debug lines.

I guess it's not so much like a bug then, but simply that he waits a little
before killing the incriminated tasks. However it seems not to be a
constant time, since during the tests it happened that it was as fast as
usual to stop (b5) while at the previous run and exactly the same way it
hanged.

(you can look for "chiffre" or "f56ef48b" to grep messages specific to the
encrypted media)

Hope it will help!
-- 
Ismael


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


Re: [systemd-devel] Get exit status inside ExecStopPost ?

2014-04-12 Thread Anand Patil
OK, no problem, thanks for the answer.

Anand


On Sat, Apr 12, 2014 at 2:45 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Fri, Apr 11, 2014 at 12:19:52PM -0700, Anand Patil wrote:
> > Hi everyone,
> >
> > In a service's ExecStopPost, is there an easy way to access the exit
> status
> > of the service's ExecStart process?
> No.
>
> You might be able to get something useful over dbus, although I don't know
> if
> the exit value is already available at this point.
>
> Zbyszek
>
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Systemd askpassword

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 10:08:27PM +0200, Ismael Bouya wrote:
> Hi,
> I'm facing a problem with systemd and his "askpassword" feature:
> 
> I want to add a luks partition, so I need to ask for the password at some
> time. However, I don't want to ask for it during the boot sequence, so I
> added the "nofail" flag and didn't put any timeout. 
> 
> Everything goes well, I can start and call the
> systemd-tty-ask-password-agent whenever I want to have access to the
> encrypted media. However... if I don't call that, then at shutdown the
> computer hangs forever with a black screen seemingly waiting for something
> (I had to try a few restarts to find out that the hanging comes from here).
Sounds like a bug. Can you enable verbose logs and post the messages from
a failed shutdown? It should be enough to 'systemd-analyze set-log-level debug',
and then reboot, wait a few minutes and reset, and extract the logs using
'journalctl -b-1'.

> What did I miss in the configuration? It also happen if you put a
> non-infinite timeout, but then the hanging finished at the
> timeout. Basically that means that the "nofail" flag in crypttab cannot be
> used reliably.
> 
> Thanks in advance for your help!
> 
> (Side question: Is there a way to say that systemd-tty-ask-password-agent
> can be run by the user and not only by root to mount the disk? If he know
> the disk password then he's most probably allowed to mount it...)
No, but this sounds like a useful feature.

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


Re: [systemd-devel] Get exit status inside ExecStopPost ?

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Apr 11, 2014 at 12:19:52PM -0700, Anand Patil wrote:
> Hi everyone,
> 
> In a service's ExecStopPost, is there an easy way to access the exit status
> of the service's ExecStart process?
No.

You might be able to get something useful over dbus, although I don't know if
the exit value is already available at this point.

Zbyszek

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


Re: [systemd-devel] [PATCH v3] fsck: Search for fsck.type in PATH

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 04:07:45PM -0400, Mike Gilbert wrote:
> Modifies find_binary() to accept NULL in the second argument.
> 
> fsck.type lookup logic moved to new fsck_exists() function, with a test.
Applied.

I added _cleanup_free_ to simplify one branch, and added a follow-up patch
to check absolute path too, which shouldn't change anything, but make things
more symmetrical.

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


Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 09:59:52PM +0200, Tom Gundersen wrote:
> On Sat, Apr 12, 2014 at 7:26 PM, Zbigniew Jędrzejewski-Szmek
>  wrote:
> > On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote:
> >> Patch doesn't apply anymore after reference counting changes. I will
> >> re-send it along with ipv4ll tests which is coming up right away.
> > Cool. Could you add a bit of a commit message explaining the change for
> > the poor ignorant sods like me who don't know why indexes start at 0 or 1?
> 
> FWIW: the kernel starts enumerating interfaces at 1 [0]. Varies API's
> accept ifindex=0 to refer to "unknown"/"any"/"all" interfaces. That's
> not useful in these libraries though, as we only want to run these
> clients on a given interface, so restricting to ifindex > 0 makes
> sense.
> 
> Umut, will you resend with a bit more detailed commit message or
> should I fix it up when applying your patch?
Thanks.

Zbyszek

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


[systemd-devel] Systemd askpassword

2014-04-12 Thread Ismael Bouya
Hi,
I'm facing a problem with systemd and his "askpassword" feature:

I want to add a luks partition, so I need to ask for the password at some
time. However, I don't want to ask for it during the boot sequence, so I
added the "nofail" flag and didn't put any timeout. 

Everything goes well, I can start and call the
systemd-tty-ask-password-agent whenever I want to have access to the
encrypted media. However... if I don't call that, then at shutdown the
computer hangs forever with a black screen seemingly waiting for something
(I had to try a few restarts to find out that the hanging comes from here).

What did I miss in the configuration? It also happen if you put a
non-infinite timeout, but then the hanging finished at the
timeout. Basically that means that the "nofail" flag in crypttab cannot be
used reliably.

Thanks in advance for your help!

(Side question: Is there a way to say that systemd-tty-ask-password-agent
can be run by the user and not only by root to mount the disk? If he know
the disk password then he's most probably allowed to mount it...)

-- 
Ismael


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


[systemd-devel] [PATCH v3] fsck: Search for fsck.type in PATH

2014-04-12 Thread Mike Gilbert
Modifies find_binary() to accept NULL in the second argument.

fsck.type lookup logic moved to new fsck_exists() function, with a test.
---
 src/fsck/fsck.c   | 10 +-
 src/shared/generator.c| 12 +---
 src/shared/path-util.c| 33 ++---
 src/shared/path-util.h|  2 ++
 src/test/test-path-util.c | 11 +++
 5 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 18f2aca..5ed837d 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -37,6 +37,7 @@
 #include "bus-errors.h"
 #include "fileio.h"
 #include "udev-util.h"
+#include "path-util.h"
 
 static bool arg_skip = false;
 static bool arg_force = false;
@@ -285,14 +286,13 @@ int main(int argc, char *argv[]) {
 
 type = udev_device_get_property_value(udev_device, "ID_FS_TYPE");
 if (type) {
-const char *checker = strappenda("/sbin/fsck.", type);
-r = access(checker, X_OK);
+r = fsck_exists(type);
 if (r < 0) {
-if (errno == ENOENT) {
-log_info("%s doesn't exist, not checking file 
system.", checker);
+if (r == -ENOENT) {
+log_info("fsck.%s doesn't exist, not checking 
file system.", type);
 return EXIT_SUCCESS;
 } else
-log_warning("%s cannot be used: %m", checker);
+log_warning("fsck.%s cannot be used: %s", 
type, strerror(-r));
 }
 }
 
diff --git a/src/shared/generator.c b/src/shared/generator.c
index 6110303..5ac7b5f 100644
--- a/src/shared/generator.c
+++ b/src/shared/generator.c
@@ -19,6 +19,7 @@
   along with systemd; If not, see .
 ***/
 
+#include 
 #include 
 
 #include "util.h"
@@ -26,6 +27,7 @@
 #include "mkdir.h"
 #include "unit-name.h"
 #include "generator.h"
+#include "path-util.h"
 
 int generator_write_fsck_deps(
 FILE *f,
@@ -45,16 +47,12 @@ int generator_write_fsck_deps(
 }
 
 if (!isempty(fstype) && !streq(fstype, "auto")) {
-const char *checker;
 int r;
-
-checker = strappenda("/sbin/fsck.", fstype);
-r = access(checker, X_OK);
+r = fsck_exists(fstype);
 if (r < 0) {
-log_warning("Checking was requested for %s, but %s 
cannot be used: %m", what, checker);
-
+log_warning("Checking was requested for %s, but 
fsck.%s cannot be used: %s", what, fstype, strerror(-r));
 /* treat missing check as essentially OK */
-return errno == ENOENT ? 0 : -errno;
+return r == -ENOENT ? 0 : r;
 }
 }
 
diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index bdc54a9..8ac96f9 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -425,19 +425,20 @@ int path_is_os_tree(const char *path) {
 
 int find_binary(const char *name, char **filename) {
 assert(name);
-assert(filename);
 
 if (strchr(name, '/')) {
-char *p;
+if (filename) {
+char *p;
 
-if (path_is_absolute(name))
-p = strdup(name);
-else
-p = path_make_absolute_cwd(name);
-if (!p)
-return -ENOMEM;
+if (path_is_absolute(name))
+p = strdup(name);
+else
+p = path_make_absolute_cwd(name);
+if (!p)
+return -ENOMEM;
+*filename = p;
+}
 
-*filename = p;
 return 0;
 } else {
 const char *path;
@@ -463,8 +464,11 @@ int find_binary(const char *name, char **filename) {
 continue;
 }
 
-path_kill_slashes(p);
-*filename = p;
+if (filename) {
+path_kill_slashes(p);
+*filename = p;
+} else
+free(p);
 
 return 0;
 }
@@ -507,3 +511,10 @@ bool paths_check_timestamp(const char* const* paths, 
usec_t *timestamp, bool upd
 
 return changed;
 }
+
+int fsck_exists(const char *fstype) {
+const char *checker;
+
+checker = strappenda("fsck.", fstype);
+return find_binary(checker, NULL);
+}
diff --git a/src/shared/path-util.h b/src/shared/path-util.h
index 2b8ea02..fdf1f6b 100644
--- a/src/shared/path

Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex

2014-04-12 Thread Tom Gundersen
On Sat, Apr 12, 2014 at 7:26 PM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote:
>> Patch doesn't apply anymore after reference counting changes. I will
>> re-send it along with ipv4ll tests which is coming up right away.
> Cool. Could you add a bit of a commit message explaining the change for
> the poor ignorant sods like me who don't know why indexes start at 0 or 1?

FWIW: the kernel starts enumerating interfaces at 1 [0]. Varies API's
accept ifindex=0 to refer to "unknown"/"any"/"all" interfaces. That's
not useful in these libraries though, as we only want to run these
clients on a given interface, so restricting to ifindex > 0 makes
sense.

Umut, will you resend with a bit more detailed commit message or
should I fix it up when applying your patch?

Cheers,

Tom

[0]: 

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


Re: [systemd-devel] [PATCH v2] fsck: Search for fsck.type in PATH

2014-04-12 Thread Mike Gilbert
On Sat, Apr 12, 2014 at 3:10 PM, Mike Gilbert  wrote:
> On Sat, Apr 12, 2014 at 1:40 PM, Zbigniew Jędrzejewski-Szmek
>  wrote:
>>> +static void test_fsck_exists(void) {
>>> +/* Ensure we use a sane default for PATH. */
>>> +unsetenv("PATH");
>>> +
>>> +/* fsck.minix is provided by util-linux and will probably exist. */
>>> +assert(fsck_exists("minix") == 0);
>>> +
>>> +assert(fsck_exists("AbCdE") == -ENOENT);
>>> +}
>> assert_se().
>>
>
> We are comparing integers, not strings. This matches the other integer
> comparisons in this source file.
>

I guess I have misinterpreted what "assert_se" means... is that
documented somewhere?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2] fsck: Search for fsck.type in PATH

2014-04-12 Thread Mike Gilbert
On Sat, Apr 12, 2014 at 1:40 PM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Sat, Apr 12, 2014 at 12:07:04PM -0400, Mike Gilbert wrote:
>> Modifies find_binary() to accept NULL in the second argument.
>>
>> fsck.type lookup logic moved to new fsck_exists() function, with a test.
>> ---
>>  src/fsck/fsck.c|  9 -
>>  src/shared/generator.c | 11 ---
>>  src/shared/path-util.c | 11 ---
>>  src/shared/util.c  |  8 
>>  src/shared/util.h  |  2 ++
>>  src/test/test-util.c   | 11 +++
>>  6 files changed, 37 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
>> index 18f2aca..520b1a6 100644
>> --- a/src/fsck/fsck.c
>> +++ b/src/fsck/fsck.c
>> @@ -285,14 +285,13 @@ int main(int argc, char *argv[]) {
>>
>>  type = udev_device_get_property_value(udev_device, "ID_FS_TYPE");
>>  if (type) {
>> -const char *checker = strappenda("/sbin/fsck.", type);
>> -r = access(checker, X_OK);
>> +r = fsck_exists(type);
>>  if (r < 0) {
>> -if (errno == ENOENT) {
>> -log_info("%s doesn't exist, not checking 
>> file system.", checker);
>> +if (r == -ENOENT) {
>> +log_info("fsck.%s doesn't exist, not 
>> checking file system.", type);
>>  return EXIT_SUCCESS;
>>  } else
>> -log_warning("%s cannot be used: %m", 
>> checker);
>> +log_warning("fsck.%s cannot be used: %s", 
>> type, strerror(-r));
>>  }
>>  }
>>
>> diff --git a/src/shared/generator.c b/src/shared/generator.c
>> index 6110303..86d30e7 100644
>> --- a/src/shared/generator.c
>> +++ b/src/shared/generator.c
>> @@ -19,6 +19,7 @@
>>along with systemd; If not, see .
>>  ***/
>>
>> +#include 
>>  #include 
>>
>>  #include "util.h"
>> @@ -45,16 +46,12 @@ int generator_write_fsck_deps(
>>  }
>>
>>  if (!isempty(fstype) && !streq(fstype, "auto")) {
>> -const char *checker;
>>  int r;
>> -
>> -checker = strappenda("/sbin/fsck.", fstype);
>> -r = access(checker, X_OK);
>> +r = fsck_exists(fstype);
>>  if (r < 0) {
>> -log_warning("Checking was requested for %s, but %s 
>> cannot be used: %m", what, checker);
>> -
>> +log_warning("Checking was requested for %s, but 
>> fsck.%s cannot be used: %s", what, what, strerror(-r));
>
> The second arg should probably be fstype not what.
>

Right, I'll fix that.

>>  /* treat missing check as essentially OK */
>> -return errno == ENOENT ? 0 : -errno;
>> +return r == -ENOENT ? 0 : r;
>>  }
>>  }
>>
>> diff --git a/src/shared/path-util.c b/src/shared/path-util.c
>> index bdc54a9..a530dbe 100644
>> --- a/src/shared/path-util.c
>> +++ b/src/shared/path-util.c
>> @@ -425,7 +425,6 @@ int path_is_os_tree(const char *path) {
>>
>>  int find_binary(const char *name, char **filename) {
>>  assert(name);
>> -assert(filename);
>>
>>  if (strchr(name, '/')) {
>>  char *p;
>> @@ -437,7 +436,10 @@ int find_binary(const char *name, char **filename) {
>>  if (!p)
>>  return -ENOMEM;
>>
>> -*filename = p;
>> +if (filename)
>> +*filename = p;
>> +else
>> +free(p);
> Allocating a string just to free it in the next line doesn't make sense.
>

Yeah, this one could probably be adjusted not to do that. I was trying
to change as little as possible.

>>  return 0;
>>  } else {
>>  const char *path;
>> @@ -464,7 +466,10 @@ int find_binary(const char *name, char **filename) {
>>  }
>>
>>  path_kill_slashes(p);
>> -*filename = p;
>> +if (filename)
>> +*filename = p;
>> +else
>> +free(p);
> Likewise, no need to mangle the string just to free it.
>

Ah, yeah. I guess this would work:

if (filename)
path_kill_slashes(p);
*filename = p;
else
free(p);

>>
>>  return 0;
>>  }
>> diff --git a/src/shared/util.c b/src/shared/util.c
>> index ffe6624..4cdb561 100644
>> --- a/src/shared/util.c
>> +++ b/src/shared/util.c
>> @@ -6391,3 +6391,11 @@ void hexdump(FILE *f, const void *p, size_t s) {
>>  s -= 16;
>>  }
>>  }
>> +
>> +int fsck_exists(const char *fstype)
>> +{
> Brace should go on the end of previous l

Re: [systemd-devel] [PATCH 1/1] networkd: Introduce ipip tunnel

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Apr 08, 2014 at 08:52:51AM +0530, Susant Sahani wrote:
> This patch enables basic ipip tunnel support.
> It works with kernel module ipip
> 
> Example configuration
> 
> file: ipip.netdev
> --
> [NetDev]
> Name=ipip-tun
> Kind=ipip
> 
> [Tunnel]
> Local=192.168.8.102
> Remote=10.4.4.4
> TTL=64
> MTUBytes=1480
> 
> file: ipip.network
> --
> [Match]
> Name=eth0
> 
> [Network]
> Tunnel=ipip-tun
> ---
>  Makefile.am   |   7 +-
>  src/libsystemd-network/network-internal.c |  33 ++
>  src/libsystemd-network/network-internal.h |   3 +
>  src/libsystemd/sd-rtnl/rtnl-types.c   |   4 +-
>  src/network/networkd-link.c   |  25 -
>  src/network/networkd-manager.c|  14 +++
>  src/network/networkd-netdev-gperf.gperf   |   4 +
>  src/network/networkd-netdev.c | 169 
> +-
>  src/network/networkd-network-gperf.gperf  |   1 +
>  src/network/networkd-network.c|  37 +++
>  src/network/networkd.c|   6 ++
>  src/network/networkd.h|  27 +
>  12 files changed, 323 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index c51f6ae..60c7016 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -4144,8 +4144,8 @@ systemd_networkd_SOURCES = \
>   src/network/networkd.c
>  
>  systemd_networkd_LDADD = \
> - libsystemd-networkd-core.la
> -
> + libsystemd-networkd-core.la \
> + -lkmod
>  noinst_LTLIBRARIES += \
>   libsystemd-networkd-core.la
>  
> @@ -4189,7 +4189,8 @@ test_network_SOURCES = \
>   src/network/test-network.c
>  
>  test_network_LDADD = \
> - libsystemd-networkd-core.la
> + libsystemd-networkd-core.la \
> + -lkmod
>  
>  tests += \
>   test-network
> diff --git a/src/libsystemd-network/network-internal.c 
> b/src/libsystemd-network/network-internal.c
> index 3686267..5b41cdb 100644
> --- a/src/libsystemd-network/network-internal.c
> +++ b/src/libsystemd-network/network-internal.c
> @@ -326,3 +326,36 @@ int net_parse_inaddr(const char *address, unsigned char 
> *family, void *dst) {
>  
>  return 0;
>  }
> +
> +int load_module(struct kmod_ctx *ctx, const char *mod_name) {
> +struct kmod_list *modlist = NULL, *l;
> +int r;
> +
> +assert(ctx);
> +assert(mod_name);
> +
> +r = kmod_module_new_from_lookup(ctx, mod_name, &modlist);
> +if (r < 0)
> +return r;
> +
> +if (!modlist) {
> +log_error("Failed to find module '%s'", mod_name);
> +return -ENOENT;
> +}
> +
> +kmod_list_foreach(l, modlist) {
> +struct kmod_module *mod = kmod_module_get_module(l);
> +
> +r = kmod_module_probe_insert_module(mod, 0, NULL, NULL, 
> NULL, NULL);
> +if (r >= 0)
> +r = 0;
> +else
> +r = -1;
Afaict, kmod_module_probe_insert_module returns normal negative-errno style 
errors, the
same as functions in systemd. Please don't use a meaningless constant, and use 
the errors
returned by kmod_module_probe_insert_module.

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


Re: [systemd-devel] [PATCH v2] fsck: Search for fsck.type in PATH

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 12:07:04PM -0400, Mike Gilbert wrote:
> Modifies find_binary() to accept NULL in the second argument.
> 
> fsck.type lookup logic moved to new fsck_exists() function, with a test.
> ---
>  src/fsck/fsck.c|  9 -
>  src/shared/generator.c | 11 ---
>  src/shared/path-util.c | 11 ---
>  src/shared/util.c  |  8 
>  src/shared/util.h  |  2 ++
>  src/test/test-util.c   | 11 +++
>  6 files changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
> index 18f2aca..520b1a6 100644
> --- a/src/fsck/fsck.c
> +++ b/src/fsck/fsck.c
> @@ -285,14 +285,13 @@ int main(int argc, char *argv[]) {
>  
>  type = udev_device_get_property_value(udev_device, "ID_FS_TYPE");
>  if (type) {
> -const char *checker = strappenda("/sbin/fsck.", type);
> -r = access(checker, X_OK);
> +r = fsck_exists(type);
>  if (r < 0) {
> -if (errno == ENOENT) {
> -log_info("%s doesn't exist, not checking 
> file system.", checker);
> +if (r == -ENOENT) {
> +log_info("fsck.%s doesn't exist, not 
> checking file system.", type);
>  return EXIT_SUCCESS;
>  } else
> -log_warning("%s cannot be used: %m", 
> checker);
> +log_warning("fsck.%s cannot be used: %s", 
> type, strerror(-r));
>  }
>  }
>  
> diff --git a/src/shared/generator.c b/src/shared/generator.c
> index 6110303..86d30e7 100644
> --- a/src/shared/generator.c
> +++ b/src/shared/generator.c
> @@ -19,6 +19,7 @@
>along with systemd; If not, see .
>  ***/
>  
> +#include 
>  #include 
>  
>  #include "util.h"
> @@ -45,16 +46,12 @@ int generator_write_fsck_deps(
>  }
>  
>  if (!isempty(fstype) && !streq(fstype, "auto")) {
> -const char *checker;
>  int r;
> -
> -checker = strappenda("/sbin/fsck.", fstype);
> -r = access(checker, X_OK);
> +r = fsck_exists(fstype);
>  if (r < 0) {
> -log_warning("Checking was requested for %s, but %s 
> cannot be used: %m", what, checker);
> -
> +log_warning("Checking was requested for %s, but 
> fsck.%s cannot be used: %s", what, what, strerror(-r));

The second arg should probably be fstype not what.

>  /* treat missing check as essentially OK */
> -return errno == ENOENT ? 0 : -errno;
> +return r == -ENOENT ? 0 : r;
>  }
>  }
>  
> diff --git a/src/shared/path-util.c b/src/shared/path-util.c
> index bdc54a9..a530dbe 100644
> --- a/src/shared/path-util.c
> +++ b/src/shared/path-util.c
> @@ -425,7 +425,6 @@ int path_is_os_tree(const char *path) {
>  
>  int find_binary(const char *name, char **filename) {
>  assert(name);
> -assert(filename);
>  
>  if (strchr(name, '/')) {
>  char *p;
> @@ -437,7 +436,10 @@ int find_binary(const char *name, char **filename) {
>  if (!p)
>  return -ENOMEM;
>  
> -*filename = p;
> +if (filename)
> +*filename = p;
> +else
> +free(p);
Allocating a string just to free it in the next line doesn't make sense.

>  return 0;
>  } else {
>  const char *path;
> @@ -464,7 +466,10 @@ int find_binary(const char *name, char **filename) {
>  }
>  
>  path_kill_slashes(p);
> -*filename = p;
> +if (filename)
> +*filename = p;
> +else
> +free(p);
Likewise, no need to mangle the string just to free it.

>  
>  return 0;
>  }
> diff --git a/src/shared/util.c b/src/shared/util.c
> index ffe6624..4cdb561 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -6391,3 +6391,11 @@ void hexdump(FILE *f, const void *p, size_t s) {
>  s -= 16;
>  }
>  }
> +
> +int fsck_exists(const char *fstype)
> +{
Brace should go on the end of previous line.

> +const char *checker;
> +
> +checker = strappenda("fsck.", fstype);
> +return find_binary(checker, NULL);
> +}
> diff --git a/src/shared/util.h b/src/shared/util.h
> index 90464c9..45a6f26 100644
> --- a/src/shared/util.h
> +++ b/src/shared/util.h
> @@ -922,3 +922,5 @@ uint64_t physical_memory(void);
>  char* mount_test_option(const char *haystack, const char *needle);
>  
>  void hexdump(FILE *f, con

Re: [systemd-devel] [PATCH 2/2] path-lookup: don't hardcode .config

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 08:37:39AM +0300, Tanu Kaskinen wrote:
> If XDG_CONFIG_HOME is set, then we should respect that.
> ---
>  src/shared/path-lookup.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
> index 63af43c..a885b66 100644
> --- a/src/shared/path-lookup.c
> +++ b/src/shared/path-lookup.c
> @@ -135,8 +135,12 @@ static char** user_dirs(
>   * then filter out this link, if it is actually is
>   * one. */
>  
> +/* If home is non-NULL, then config_home must be
> + * non-NULL as well. */
> +assert(config_home);
> +
>  mkdir_parents_label(data_home, 0777);
> -(void) symlink("../../../.config/systemd/user", data_home);
> +(void) symlink(config_home, data_home);
>  }
The change is good, as it fixes a misbehaviour for people who have
non-default settings for XDG_CONFIG_HOME and XDG_CONFIG_DATA. But we
really want to keep the path relative, since home directories tend to
be mounted at different points in the hierarchy (e.g. on the host, and
in the containers), so it would be annoying for the general case to have
an absolute path here. Can you add a function which makes the symlink relative
by stripping the common prefix and adding '../' as necessary and use it
here?

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


Re: [systemd-devel] [PATCH 1/2] man: mention XDG_CONFIG_HOME in systemd.unit

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 08:37:38AM +0300, Tanu Kaskinen wrote:
> ---
>  man/systemd.unit.xml | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
Applied 1/2.

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


Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote:
> Patch doesn't apply anymore after reference counting changes. I will
> re-send it along with ipv4ll tests which is coming up right away.
Cool. Could you add a bit of a commit message explaining the change for
the poor ignorant sods like me who don't know why indexes start at 0 or 1?

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


Re: [systemd-devel] [PATCH] build: if -fstack-protector-strong is available, use it.

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
Yeah, this seems a good thing. Applied.

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


[systemd-devel] [PATCH v2] fsck: Search for fsck.type in PATH

2014-04-12 Thread Mike Gilbert
Modifies find_binary() to accept NULL in the second argument.

fsck.type lookup logic moved to new fsck_exists() function, with a test.
---
 src/fsck/fsck.c|  9 -
 src/shared/generator.c | 11 ---
 src/shared/path-util.c | 11 ---
 src/shared/util.c  |  8 
 src/shared/util.h  |  2 ++
 src/test/test-util.c   | 11 +++
 6 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 18f2aca..520b1a6 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -285,14 +285,13 @@ int main(int argc, char *argv[]) {
 
 type = udev_device_get_property_value(udev_device, "ID_FS_TYPE");
 if (type) {
-const char *checker = strappenda("/sbin/fsck.", type);
-r = access(checker, X_OK);
+r = fsck_exists(type);
 if (r < 0) {
-if (errno == ENOENT) {
-log_info("%s doesn't exist, not checking file 
system.", checker);
+if (r == -ENOENT) {
+log_info("fsck.%s doesn't exist, not checking 
file system.", type);
 return EXIT_SUCCESS;
 } else
-log_warning("%s cannot be used: %m", checker);
+log_warning("fsck.%s cannot be used: %s", 
type, strerror(-r));
 }
 }
 
diff --git a/src/shared/generator.c b/src/shared/generator.c
index 6110303..86d30e7 100644
--- a/src/shared/generator.c
+++ b/src/shared/generator.c
@@ -19,6 +19,7 @@
   along with systemd; If not, see .
 ***/
 
+#include 
 #include 
 
 #include "util.h"
@@ -45,16 +46,12 @@ int generator_write_fsck_deps(
 }
 
 if (!isempty(fstype) && !streq(fstype, "auto")) {
-const char *checker;
 int r;
-
-checker = strappenda("/sbin/fsck.", fstype);
-r = access(checker, X_OK);
+r = fsck_exists(fstype);
 if (r < 0) {
-log_warning("Checking was requested for %s, but %s 
cannot be used: %m", what, checker);
-
+log_warning("Checking was requested for %s, but 
fsck.%s cannot be used: %s", what, what, strerror(-r));
 /* treat missing check as essentially OK */
-return errno == ENOENT ? 0 : -errno;
+return r == -ENOENT ? 0 : r;
 }
 }
 
diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index bdc54a9..a530dbe 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -425,7 +425,6 @@ int path_is_os_tree(const char *path) {
 
 int find_binary(const char *name, char **filename) {
 assert(name);
-assert(filename);
 
 if (strchr(name, '/')) {
 char *p;
@@ -437,7 +436,10 @@ int find_binary(const char *name, char **filename) {
 if (!p)
 return -ENOMEM;
 
-*filename = p;
+if (filename)
+*filename = p;
+else
+free(p);
 return 0;
 } else {
 const char *path;
@@ -464,7 +466,10 @@ int find_binary(const char *name, char **filename) {
 }
 
 path_kill_slashes(p);
-*filename = p;
+if (filename)
+*filename = p;
+else
+free(p);
 
 return 0;
 }
diff --git a/src/shared/util.c b/src/shared/util.c
index ffe6624..4cdb561 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -6391,3 +6391,11 @@ void hexdump(FILE *f, const void *p, size_t s) {
 s -= 16;
 }
 }
+
+int fsck_exists(const char *fstype)
+{
+const char *checker;
+
+checker = strappenda("fsck.", fstype);
+return find_binary(checker, NULL);
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 90464c9..45a6f26 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -922,3 +922,5 @@ uint64_t physical_memory(void);
 char* mount_test_option(const char *haystack, const char *needle);
 
 void hexdump(FILE *f, const void *p, size_t s);
+
+int fsck_exists(const char *fstype);
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 93929cd..148e54c 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -675,6 +675,16 @@ static void test_foreach_string(void) {
 assert_se(streq(x, "zzz"));
 }
 
+static void test_fsck_exists(void) {
+/* Ensure we use a sane default for PATH. */
+unsetenv("PATH");
+
+/* fsck.minix is provided by util-linux and will probably exist. */
+assert(fsck_exists("minix") == 0);
+

Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers

2014-04-12 Thread Zbigniew Jędrzejewski-Szmek
On Sat, Apr 12, 2014 at 01:02:53PM +0200, Tom Gundersen wrote:
> On Sat, Apr 12, 2014 at 12:57 PM, Thomas Bächler  wrote:
> > Am 05.04.2014 17:32, schrieb Thomas Bächler:
> >> Am 05.04.2014 11:35, schrieb Tom Gundersen:
> >>> On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler  
> >>> wrote:
>  If a persistent timer has no stamp file yet, it behaves just like a 
>  normal
>  timer until it runs for the first time. If the system is always shut down
>  while the timer is supposed to run, a stamp file is never created and
>  Peristent=true has no effect.
> 
>  This patch fixes this by creating a stamp file with the current time
>  when the timer is first started.
> >>>
> >>> If timers are started at early boot (which sounds like a common
> >>> scenario), I guess /var will not yet be writable so this will be a
> >>> noop, no? Maybe it would be better to write out these files at
> >>> shutdown instead (before unmounting anything)?
> >>
> >> I failed to hit "reply all" last time, so apologies for sending you this
> >> mail twice, Tom.
> >>
> >> Persistent=true timers have an implicit dependency on
> >> RequiresMountsFor=/var/lib/systemd/timers.
> >>
> >> $ systemctl show -p RequiresMountsFor updatedb.timer
> >> RequiresMountsFor=/var/lib/systemd/timers
> >>
> >> $ systemctl cat updatedb.timer
> >> # /usr/lib/systemd/system/updatedb.timer
> >> [Unit]
> >> Description=Daily locate database update
> >>
> >> [Timer]
> >> OnCalendar=daily
> >> AccuracySec=12h
> >> Persistent=true
> >
> > I don't want to be annoying, but I'd really like an ACK or NAK on that
> > patch.
Applied.

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


Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex

2014-04-12 Thread Umut Tezduyar Lindskog
Patch doesn't apply anymore after reference counting changes. I will
re-send it along with ipv4ll tests which is coming up right away.

On Thu, Apr 10, 2014 at 2:35 PM, Umut Tezduyar Lindskog
 wrote:
> From: Umut Tezduyar Lindskog 
>
> ---
>  src/libsystemd-network/sd-dhcp-client.c   |2 +-
>  src/libsystemd-network/sd-ipv4ll.c|2 +-
>  src/libsystemd-network/test-dhcp-client.c |4 +++-
>  3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/libsystemd-network/sd-dhcp-client.c 
> b/src/libsystemd-network/sd-dhcp-client.c
> index da41c47..70d1259 100644
> --- a/src/libsystemd-network/sd-dhcp-client.c
> +++ b/src/libsystemd-network/sd-dhcp-client.c
> @@ -135,7 +135,7 @@ int sd_dhcp_client_set_request_address(sd_dhcp_client 
> *client,
>  int sd_dhcp_client_set_index(sd_dhcp_client *client, int interface_index) {
>  assert_return(client, -EINVAL);
>  assert_return(client->state == DHCP_STATE_INIT, -EBUSY);
> -assert_return(interface_index >= -1, -EINVAL);
> +assert_return(interface_index > 0, -EINVAL);
>
>  client->index = interface_index;
>
> diff --git a/src/libsystemd-network/sd-ipv4ll.c 
> b/src/libsystemd-network/sd-ipv4ll.c
> index 81fe85b..393a213 100644
> --- a/src/libsystemd-network/sd-ipv4ll.c
> +++ b/src/libsystemd-network/sd-ipv4ll.c
> @@ -368,7 +368,7 @@ static int ipv4ll_receive_message(sd_event_source *s, int 
> fd,
>
>  int sd_ipv4ll_set_index(sd_ipv4ll *ll, int interface_index) {
>  assert_return(ll, -EINVAL);
> -assert_return(interface_index >= -1, -EINVAL);
> +assert_return(interface_index > 0, -EINVAL);
>  assert_return(ll->state == IPV4LL_STATE_INIT, -EBUSY);
>
>  ll->index = interface_index;
> diff --git a/src/libsystemd-network/test-dhcp-client.c 
> b/src/libsystemd-network/test-dhcp-client.c
> index a208b0d..3ba56b1 100644
> --- a/src/libsystemd-network/test-dhcp-client.c
> +++ b/src/libsystemd-network/test-dhcp-client.c
> @@ -77,7 +77,9 @@ static void test_request_basic(sd_event *e)
>
>  assert_se(sd_dhcp_client_set_index(client, 15) == 0);
>  assert_se(sd_dhcp_client_set_index(client, -42) == -EINVAL);
> -assert_se(sd_dhcp_client_set_index(client, -1) == 0);
> +assert_se(sd_dhcp_client_set_index(client, -1) == -EINVAL);
> +assert_se(sd_dhcp_client_set_index(client, 0) == -EINVAL);
> +assert_se(sd_dhcp_client_set_index(client, 1) == 0);
>
>  assert_se(sd_dhcp_client_set_request_option(client,
>  DHCP_OPTION_SUBNET_MASK) == -EEXIST);
> --
> 1.7.10.4
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] libnetworkd: add link local tests

2014-04-12 Thread Umut Tezduyar Lindskog
- Also only allow positive ifindex on both dhcp and ipv4ll
---
 Makefile.am   |   14 +-
 TODO  |1 -
 src/libsystemd-network/sd-dhcp-client.c   |2 +-
 src/libsystemd-network/sd-ipv4ll.c|7 +-
 src/libsystemd-network/test-dhcp-client.c |4 +-
 src/libsystemd-network/test-ipv4ll.c  |  225 +
 6 files changed, 247 insertions(+), 6 deletions(-)
 create mode 100644 src/libsystemd-network/test-ipv4ll.c

diff --git a/Makefile.am b/Makefile.am
index 5d84605..1fbae4b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2506,9 +2506,21 @@ test_dhcp_client_LDADD = \
libsystemd-internal.la \
libsystemd-shared.la
 
+test_ipv4ll_SOURCES = \
+src/systemd/sd-ipv4ll.h \
+src/libsystemd-network/ipv4ll-internal.h \
+src/libsystemd-network/test-ipv4ll.c
+
+test_ipv4ll_LDADD = \
+libsystemd-network.la \
+libsystemd-label.la \
+libsystemd-internal.la \
+libsystemd-shared.la
+
 tests += \
test-dhcp-option \
-   test-dhcp-client
+   test-dhcp-client \
+   test-ipv4ll
 
 # 
--
 if ENABLE_GTK_DOC
diff --git a/TODO b/TODO
index 0343b94..a02c532 100644
--- a/TODO
+++ b/TODO
@@ -665,7 +665,6 @@ Features:
- add support for more DHCPv4 options (and, longer term, other kinds of 
dynamic config)
- add proper initrd support (in particular generate .network/.link files 
based on /proc/cmdline)
- add reduced [Link] support to .network files
-   - add IPv4LL tests (inspire by DHCP)
- add Scope= parsing option for [Network]
 
 * sd-network:
diff --git a/src/libsystemd-network/sd-dhcp-client.c 
b/src/libsystemd-network/sd-dhcp-client.c
index c67de71..84e85d9 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -142,7 +142,7 @@ int sd_dhcp_client_set_index(sd_dhcp_client *client, int 
interface_index) {
 assert_return(client, -EINVAL);
 assert_return (IN_SET(client->state, DHCP_STATE_INIT,
   DHCP_STATE_STOPPED), -EBUSY);
-assert_return(interface_index >= -1, -EINVAL);
+assert_return(interface_index > 0, -EINVAL);
 
 client->index = interface_index;
 
diff --git a/src/libsystemd-network/sd-ipv4ll.c 
b/src/libsystemd-network/sd-ipv4ll.c
index 72289b2..fd39c12 100644
--- a/src/libsystemd-network/sd-ipv4ll.c
+++ b/src/libsystemd-network/sd-ipv4ll.c
@@ -382,7 +382,7 @@ static int ipv4ll_receive_message(sd_event_source *s, int 
fd,
 
 int sd_ipv4ll_set_index(sd_ipv4ll *ll, int interface_index) {
 assert_return(ll, -EINVAL);
-assert_return(interface_index >= -1, -EINVAL);
+assert_return(interface_index > 0, -EINVAL);
 assert_return(IN_SET(ll->state, IPV4LL_STATE_INIT,
  IPV4LL_STATE_STOPPED), -EBUSY);
 
@@ -469,10 +469,13 @@ int sd_ipv4ll_get_address(sd_ipv4ll *ll, struct in_addr 
*address){
 }
 
 int sd_ipv4ll_set_address_seed (sd_ipv4ll *ll, uint8_t seed[8]) {
-unsigned int entropy = *seed;
+unsigned int entropy;
 int r;
 
 assert_return(ll, -EINVAL);
+assert_return(seed, -EINVAL);
+
+entropy = *seed;
 
 free(ll->random_data);
 free(ll->random_data_state);
diff --git a/src/libsystemd-network/test-dhcp-client.c 
b/src/libsystemd-network/test-dhcp-client.c
index 9c316d7..b201aab 100644
--- a/src/libsystemd-network/test-dhcp-client.c
+++ b/src/libsystemd-network/test-dhcp-client.c
@@ -77,7 +77,9 @@ static void test_request_basic(sd_event *e)
 
 assert_se(sd_dhcp_client_set_index(client, 15) == 0);
 assert_se(sd_dhcp_client_set_index(client, -42) == -EINVAL);
-assert_se(sd_dhcp_client_set_index(client, -1) == 0);
+assert_se(sd_dhcp_client_set_index(client, -1) == -EINVAL);
+assert_se(sd_dhcp_client_set_index(client, 0) == -EINVAL);
+assert_se(sd_dhcp_client_set_index(client, 1) == 0);
 
 assert_se(sd_dhcp_client_set_request_option(client,
 DHCP_OPTION_SUBNET_MASK) == -EEXIST);
diff --git a/src/libsystemd-network/test-ipv4ll.c 
b/src/libsystemd-network/test-ipv4ll.c
new file mode 100644
index 000..9cea74e
--- /dev/null
+++ b/src/libsystemd-network/test-ipv4ll.c
@@ -0,0 +1,225 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2014 Axis Communications AB. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  M

Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers

2014-04-12 Thread Tom Gundersen
On Sat, Apr 12, 2014 at 12:57 PM, Thomas Bächler  wrote:
> Am 05.04.2014 17:32, schrieb Thomas Bächler:
>> Am 05.04.2014 11:35, schrieb Tom Gundersen:
>>> On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler  wrote:
 If a persistent timer has no stamp file yet, it behaves just like a normal
 timer until it runs for the first time. If the system is always shut down
 while the timer is supposed to run, a stamp file is never created and
 Peristent=true has no effect.

 This patch fixes this by creating a stamp file with the current time
 when the timer is first started.
>>>
>>> If timers are started at early boot (which sounds like a common
>>> scenario), I guess /var will not yet be writable so this will be a
>>> noop, no? Maybe it would be better to write out these files at
>>> shutdown instead (before unmounting anything)?
>>
>> I failed to hit "reply all" last time, so apologies for sending you this
>> mail twice, Tom.
>>
>> Persistent=true timers have an implicit dependency on
>> RequiresMountsFor=/var/lib/systemd/timers.
>>
>> $ systemctl show -p RequiresMountsFor updatedb.timer
>> RequiresMountsFor=/var/lib/systemd/timers
>>
>> $ systemctl cat updatedb.timer
>> # /usr/lib/systemd/system/updatedb.timer
>> [Unit]
>> Description=Daily locate database update
>>
>> [Timer]
>> OnCalendar=daily
>> AccuracySec=12h
>> Persistent=true
>
> I don't want to be annoying, but I'd really like an ACK or NAK on that
> patch.

To me it looks good, but I don't know this area too well, so would
prefer Lennart (or someone else) to look at it before applying.
Lennart is currently travelling, so may take a bit more time. Sorry
about that.

Cheers,

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


Re: [systemd-devel] [PATCH] core: Make sure a stamp file exists for all Persistent=true timers

2014-04-12 Thread Thomas Bächler
Am 05.04.2014 17:32, schrieb Thomas Bächler:
> Am 05.04.2014 11:35, schrieb Tom Gundersen:
>> On Wed, Apr 2, 2014 at 8:18 PM, Thomas Bächler  wrote:
>>> If a persistent timer has no stamp file yet, it behaves just like a normal
>>> timer until it runs for the first time. If the system is always shut down
>>> while the timer is supposed to run, a stamp file is never created and
>>> Peristent=true has no effect.
>>>
>>> This patch fixes this by creating a stamp file with the current time
>>> when the timer is first started.
>>
>> If timers are started at early boot (which sounds like a common
>> scenario), I guess /var will not yet be writable so this will be a
>> noop, no? Maybe it would be better to write out these files at
>> shutdown instead (before unmounting anything)?
> 
> I failed to hit "reply all" last time, so apologies for sending you this
> mail twice, Tom.
> 
> Persistent=true timers have an implicit dependency on
> RequiresMountsFor=/var/lib/systemd/timers.
> 
> $ systemctl show -p RequiresMountsFor updatedb.timer
> RequiresMountsFor=/var/lib/systemd/timers
> 
> $ systemctl cat updatedb.timer
> # /usr/lib/systemd/system/updatedb.timer
> [Unit]
> Description=Daily locate database update
> 
> [Timer]
> OnCalendar=daily
> AccuracySec=12h
> Persistent=true

I don't want to be annoying, but I'd really like an ACK or NAK on that
patch.




signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC PATCH] service: add FailureAction= option

2014-04-12 Thread Michael Olbrich
On Fri, Apr 11, 2014 at 04:07:46PM +0200, Lennart Poettering wrote:
> On Fri, 11.04.14 09:48, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > > > +else if (allow_restart &&
> > > 
> > > I would drop the "else" here, I think. Is there a reason not to do the
> > > restart thing anyway? If it is configured, it should run I think, just
> > > in case the failure action doesn't work or so...
> > 
> > Are you sure? With Restart=always and FailureAction=reboot this would
> > try to start the unit while shutting down. Will the Conflicts= from the
> > default dependencies handle this correctly?
> 
> It should handle this correctly, and if it doesn't we should fix this. I
> mean, my thinking here is that combining FailureAction= and
> Restart=failure might not make much sense but there isn't really a
> strong reason to totally prohibit it...

With lots of debugging enabled it looks like this:

wd-test.service failed, rebooting.
Trying to enqueue job reboot.target/start/replace
Installed new job reboot.target/start as 157
Installed new job systemd-reboot.service/start as 158
Installed new job shutdown.target/start as 159
[...]
wd-test.service changed failed -> auto-restart
wd-test.service: cgroup is empty
Accepted new private connection.
wd-test.service holdoff time over, scheduling restart.
Trying to enqueue job wd-test.service/restart/fail
[...]
wd-test.service failed to schedule restart job: Transaction is destructive.
wd-test.service changed auto-restart -> failed
Unit wd-test.service entered failed state.
wd-test.service failed, rebooting.
Trying to enqueue job reboot.target/start/replace
Merged into installed job reboot.target/start as 157
Merged into installed job systemd-reboot.service/start as 158
Merged into installed job shutdown.target/start as 159
[...]

The restart is canceled and the new reboot jobs are merged into the already
running jobs. I'd say the falls under 'handle this correctly'.
I'll send out a new patch when I'm done testing.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] Default file permissions in user mode

2014-04-12 Thread Tanu Kaskinen

Hi,

The systemd.socket man page says that the default value for 
DirectoryMode is 0755. Judging from the code in the socket_init() 
function, the documentation matches the implementation. 0755 seems 
appropriate for the system mode, but wouldn't 0700 be better for the 
user mode? Likewise for the default socket mode 0666 vs. 0600, and 
probably similar permissions are specified elsewhere too.


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