[systemd-devel] [PATCH] test: Make testing work on systems without or old systemd

[Holger Hans Peter Freyther]

From: Holger Hans Peter Freyther 

* Introduce a macro to conditionally execute tests. This avoids
  skipping the entire test if some parts require systemd
* Skip the journal tests when no /etc/machine-id is present
* Change test-catalog to load the catalog from the source directory
  of systemd.
* /proc/PID/comm got introduced in v2.6.33 but travis is still
  using v2.6.32.
* Enable make check and make distcheck on the travis build
* Use -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)" as a STRINGIY
  would result in the path '/home/ich/source/linux' to be expanded
  to '/home/ich/source/1' as linux is defined to 1.
---
 .travis.yml |  5 +++--
 Makefile.am | 13 +++--
 src/journal/test-catalog.c  | 20 +---
 src/journal/test-journal-interleaving.c |  4 
 src/journal/test-journal-stream.c   |  4 
 src/journal/test-journal-verify.c   |  4 
 src/journal/test-journal.c  |  4 
 src/test/test-cgroup-util.c |  5 +++--
 src/test/test-helper.h  | 31 +++
 src/test/test-id128.c   | 11 +++
 src/test/test-unit-file.c   |  3 ++-
 src/test/test-unit-name.c   |  5 -
 src/test/test-util.c|  9 +++--
 13 files changed, 101 insertions(+), 17 deletions(-)
 create mode 100644 src/test/test-helper.h

diff --git a/.travis.yml b/.travis.yml
index 42433fd..7e5251c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,8 +3,9 @@ compiler:
   - gcc
 before_install:
  - sudo apt-get update -qq
- - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf
-script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && make dist V=1
+ - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev
+script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && sudo ./systemd-machine-id-setup && make check && make distcheck
+after_failure: cat test-suite.log
 notifications:
   irc:
 channels:
diff --git a/Makefile.am b/Makefile.am
index c4b9b1a..e598585 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1130,6 +1130,9 @@ EXTRA_DIST += \
test/sched_rr_ok.service \
test/sched_rr_change.service
 
+EXTRA_DIST += \
+   src/test/test-helper.h
+
 test_engine_SOURCES = \
src/test/test-engine.c
 
@@ -1328,7 +1331,8 @@ test_cgroup_util_SOURCES = \
 
 test_cgroup_util_LDADD = \
libsystemd-label.la \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-daemon.la
 
 test_env_replace_SOURCES = \
src/test/test-env-replace.c
@@ -2647,7 +2651,8 @@ test_id128_SOURCES = \
 
 test_id128_LDADD = \
libsystemd-shared.la \
-   libsystemd-id128-internal.la
+   libsystemd-id128-internal.la \
+   libsystemd-daemon.la
 
 tests += \
test-id128
@@ -2810,6 +2815,10 @@ test_mmap_cache_LDADD = \
 test_catalog_SOURCES = \
src/journal/test-catalog.c
 
+test_catalog_CFLAGS = \
+   $(AM_CFLAGS) \
+   -D"STR(s)=\#s" -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)"
+
 test_catalog_LDADD = \
libsystemd-shared.la \
libsystemd-label.la \
diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c
index 987867f..5db5bed 100644
--- a/src/journal/test-catalog.c
+++ b/src/journal/test-catalog.c
@@ -31,6 +31,16 @@
 #include "sd-messages.h"
 #include "catalog.h"
 
+static const char *catalog_dirs[] = {
+CATALOG_DIR,
+NULL,
+};
+
+static const char *no_catalog_dirs[] = {
+"/bin/hopefully/with/no/catalog",
+NULL
+};
+
 static void test_import(Hashmap *h, struct strbuf *sb,
 const char* contents, ssize_t size, int code) {
 int r;
@@ -100,9 +110,13 @@ static void test_catalog_update(void) {
 r = catalog_update(database, NULL, NULL);
 assert(r >= 0);
 
-/* Note: this might actually not find anything, if systemd was
- * not installed before. That should be fine too. */
-r = catalog_update(database, NULL, catalog_file_dirs);
+/* Test what happens if there are no files in the directory. */
+r = catalog_update(database, NULL, no_catalog_dirs);
+assert(r >= 0);
+
+/* Make sure that we at least have some files loaded or the
+   catalog_list below will fail. */
+r = catalog_update(database, NULL, catalog_dirs);
 assert(r >= 0);
 }
 
diff --git a/src/journal/test-journal-interleaving.c 
b/src/

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

[Holger Hans Peter Freyther]

On Wed, Jul 17, 2013 at 04:02:26PM -0300, Lucas De Marchi wrote:

> Talking about kmod and travis-ci. In kmod we are using it. Pretty
> cool, with an IRC bot integration. I only wish there were more options
> of distributions.  Right now it's only Ubuntu 12.04 32 bits

You could ask the people that operate hydra.nixos.org to build
kmod for you. This will at least give you ia32 and x86-64 builds
and a coverage report for the x86-64. I am not sure if they have
irc notifications and how open they are to non GNU projects.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

[Holger Hans Peter Freyther]

On Wed, Jul 17, 2013 at 05:08:46PM +0200, Kay Sievers wrote:
> 
> I did something like that now. Let's see if it works ...

Thank you, it did[1].

[1] https://travis-ci.org/systemd/systemd
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journalctl: add --iso-dates for long timestamps

[Tomasz Torcz]

On Thu, Jul 18, 2013 at 01:36:51AM +0200, Lennart Poettering wrote:
> On Wed, 17.07.13 17:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> 
> > > +
> > > +
> > So, now we have -o short, -o short-monotonic, and --iso-dates.
> > I'm sure we'll add a relative timestamp mode like the excellent one in
> > dmesg --human output in recent util-linux. So I'd say that it makes
> > sense to deprecate short-monotonic and add a new switch --timestamps,
> > --timestamps=monotonic → old short-monotonic
> > --timestamps=iso-date → what you're proposing
> > --timestamps=...
> > 
> > This makes the whole thing easier to grok, imo. Journalctl already has
> > 1½ page listing of options...
> 
> I agree with Zbigniew here. Having this feature sounds useful, but
> please not in a new switch of its own. However I am not convinced that a
> --timestamps= switch would be necessary for this either, because I
> figure that the only --output= mode where iso dates would be useful
> would be "short". It's not useful for the "json", "json-pretty",
> "json-sse", "export", or "cat" modes, and only semi-useful for "verbose"
> mode. It's primarily useful for the "short" mode. Given that we already
> have a "short-monotonic" mode, maybe we should just add more like these?
> i.e. --output=short-iso or so?
> 
> If one day we really get multiple modes where ISO dates could be useful
> then maybe we could internally handles this by considering the --output=
> param no longer as single value but always as a pair separated by a dash
> or so. If you follow what I mean.
> 
> So, yeah, I'd think adding this as "--output=short-iso" or so would be
> the nicest?

  Additional --timestamp would be overkill, I think.  New output mode works
for me, I will rework the patch.

-- 
Tomasz TorczThere exists no separation between gods and men:
xmpp: zdzich...@chrome.pl   one blends softly casual into the other.

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

Re: [systemd-devel] [PATCH 2/2] test: Make testing work on systems without or old systemd

[Holger Hans Peter Freyther]

On Thu, Jul 18, 2013 at 06:16:48AM +0200, Zbigniew Jędrzejewski-Szmek wrote:

> Can we use the STRINGIFY macro instead?

I can try.

> >  /* Note: this might actually not find anything, if systemd was
> >   * not installed before. That should be fine too. */
> > -r = catalog_update(database, NULL, catalog_file_dirs);
> > +r = catalog_update(database, NULL, catalog_dirs);
> OK, so now the comment either was always inaccurate, or just became 
> inaccurate.
> Does it really fail if the catalog was not installed before?
> I wanted the case of no installed systemd to be a test for a missing catalog,
> so it was on purpose.
> 
> >  assert(r >= 0);

Yes, you will ran into this assert. We can pass a dir with ["/nonexistant"] and
stat that it is non-existant.


> Wouldn't access("/etc/machine-id", F_OK) look better, and avoid the extra
> variable?

okay.

> > -log_info("pid1 comm: '%s'", a);
> > +if (stat("/proc/1/comm", &st) == 0) {
> > +assert_se(get_process_comm(1, &a) >= 0);
> > +log_info("pid1 comm: '%s'", a);
> > +} else {
> > +log_info("/proc/1/comm does not exist.");
> log_warning?

I just made it match the other output level.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] test: Make testing work on systems without or old systemd

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jul 17, 2013 at 04:29:56PM +0200, Holger Hans Peter Freyther wrote:
> From: Holger Hans Peter Freyther 
> 
> * Introduce a macro to conditionally execute tests. This avoids
>   skipping the entire test if some parts require systemd
> * Skip the journal tests when no /etc/machine-id is present
> * Change test-catalog to load the catalog from the source directory
>   of systemd.
> * /proc/PID/comm got introduced in v2.6.33 but travis is still
>   using v2.6.32.
> * Enable make check and make distcheck on the travis build
> ---
>  .travis.yml |  5 +++--
>  Makefile.am | 13 +++--
>  src/journal/test-catalog.c  |  7 ++-
>  src/journal/test-journal-interleaving.c |  5 +
>  src/journal/test-journal-stream.c   |  5 +
>  src/journal/test-journal-verify.c   |  4 
>  src/journal/test-journal.c  |  5 +
>  src/test/test-cgroup-util.c |  5 +++--
>  src/test/test-helper.h  | 31 +++
>  src/test/test-id128.c   | 11 +++
>  src/test/test-unit-file.c   |  3 ++-
>  src/test/test-unit-name.c   |  5 -
>  src/test/test-util.c|  9 +++--
>  13 files changed, 93 insertions(+), 15 deletions(-)
>  create mode 100644 src/test/test-helper.h
> 
> diff --git a/.travis.yml b/.travis.yml
> index 42433fd..7e5251c 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -3,8 +3,9 @@ compiler:
>- gcc
>  before_install:
>   - sudo apt-get update -qq
> - - sudo apt-get install autotools-dev automake autoconf libtool 
> libdbus-1-dev libcap-dev libblkid-dev libpam-dev libcryptsetup-dev 
> libaudit-dev libacl1-dev libattr1-dev libselinux-dev liblzma-dev 
> libgcrypt-dev libqrencode-dev libmicrohttpd-dev gtk-doc-tools gperf
> -script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
> make V=1 && make dist V=1
> + - sudo apt-get install autotools-dev automake autoconf libtool 
> libdbus-1-dev libcap-dev libblkid-dev libpam-dev libcryptsetup-dev 
> libaudit-dev libacl1-dev libattr1-dev libselinux-dev liblzma-dev 
> libgcrypt-dev libqrencode-dev libmicrohttpd-dev gtk-doc-tools gperf 
> python2.7-dev
> +script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
> make V=1 && sudo ./systemd-machine-id-setup && make check && make distcheck
> +after_failure: cat test-suite.log
>  notifications:
>irc:
>  channels:
> diff --git a/Makefile.am b/Makefile.am
> index c4b9b1a..e598585 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1130,6 +1130,9 @@ EXTRA_DIST += \
>   test/sched_rr_ok.service \
>   test/sched_rr_change.service
>  
> +EXTRA_DIST += \
> + src/test/test-helper.h
> +
>  test_engine_SOURCES = \
>   src/test/test-engine.c
>  
> @@ -1328,7 +1331,8 @@ test_cgroup_util_SOURCES = \
>  
>  test_cgroup_util_LDADD = \
>   libsystemd-label.la \
> - libsystemd-shared.la
> + libsystemd-shared.la \
> + libsystemd-daemon.la
>  
>  test_env_replace_SOURCES = \
>   src/test/test-env-replace.c
> @@ -2647,7 +2651,8 @@ test_id128_SOURCES = \
>  
>  test_id128_LDADD = \
>   libsystemd-shared.la \
> - libsystemd-id128-internal.la
> + libsystemd-id128-internal.la \
> + libsystemd-daemon.la
>  
>  tests += \
>   test-id128
> @@ -2810,6 +2815,10 @@ test_mmap_cache_LDADD = \
>  test_catalog_SOURCES = \
>   src/journal/test-catalog.c
>  
> +test_catalog_CFLAGS = \
> + $(AM_CFLAGS) \
> + -D"STR(s)=\#s" -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)"
Can we use the STRINGIFY macro instead?

> +
>  test_catalog_LDADD = \
>   libsystemd-shared.la \
>   libsystemd-label.la \
> diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c
> index 987867f..9620301 100644
> --- a/src/journal/test-catalog.c
> +++ b/src/journal/test-catalog.c
> @@ -31,6 +31,11 @@
>  #include "sd-messages.h"
>  #include "catalog.h"
>  
> +static const char *catalog_dirs[] = {
> +CATALOG_DIR,
> +NULL,
> +};
> +
>  static void test_import(Hashmap *h, struct strbuf *sb,
>  const char* contents, ssize_t size, int code) {
>  int r;
> @@ -102,7 +107,7 @@ static void test_catalog_update(void) {
>  
>  /* Note: this might actually not find anything, if systemd was
>   * not installed before. That should be fine too. */
> -r = catalog_update(database, NULL, catalog_file_dirs);
> +r = catalog_update(database, NULL, catalog_dirs);
OK, so now the comment either was always inaccurate, or just became inaccurate.
Does it really fail if the catalog was not installed before?
I wanted the case of no installed systemd to be a test for a missing catalog,
so it was on purpose.

>  assert(r >= 0);
>  }
>  
> diff --git a/src/journal/test-journal-interleaving.c 
> b/src/journal/test-journal-interleaving.c
> index 069d297..bd3cb7f 100644
> --- a/src/journal/test-

Re: [systemd-devel] [PATCH 1/2] test: Keep the test-suite.log around in case of a test failure

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jul 17, 2013 at 04:29:55PM +0200, Holger Hans Peter Freyther wrote:
> From: Holger Hans Peter Freyther 
> 
> The addition of .DELETE_ON_ERROR will lead to the removal of the
> test-suite.log in case of a test failure. Mark the rule as PRECIOUS
> to keep that file around.
Applied.

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

Re: [systemd-devel] [PATCH v5] journalctl: Add support for showing messages from a previous boot

[Zbigniew Jędrzejewski-Szmek]

On Tue, Jul 16, 2013 at 05:46:04PM +0200, Lennart Poettering wrote:
> On Tue, 16.07.13 17:42, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> 
> > 
> > On Tue, Jul 16, 2013 at 05:39:54PM +0200, Lennart Poettering wrote:
> > > On Fri, 28.06.13 17:26, Jan Janssen (medhe...@web.de) wrote:
> > > Applied this one now. If people start complaining about its speed we can
> > > reinvestigate and do find some way for optimization...
> > We need to think about negative matches. Looking for previous boots
> > with negative matches should work nicely.
> 
> The bisection tables make this less efficient but certainly possible.
> 
> > I'd like to complain about the : in the syntax though.
> 
> Hmm, what would you propose? There's still time to fix it!
I went ahead, and removed : from the syntax. It feels OK in my testing.

And I also made one optimization, which is important imho: 'journactl -b'
will still use the boot id from sd_id128_get_boot() to avoid searching
through the tables, and 'journalctl -b BOOT_ID[+-0]' will just
use  BOOT_ID without searching through the tables. This should help
a lot when running with cold cache.

Zbyszek

-- 
they are not broken. they are refucktored
   -- alxchk
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Log failing start conditions

[Zbigniew Jędrzejewski-Szmek]

On Tue, Jul 16, 2013 at 07:15:56PM +0200, Lennart Poettering wrote:
> On Wed, 26.06.13 15:06, har...@redhat.com (har...@redhat.com) wrote:
> 
> > From: Harald Hoyer 
> > 
> > $ systemctl status dracut-initqueue.service
> > dracut-initqueue.service - dracut initqueue hook
> >Loaded: loaded (/usr/lib/systemd/system/dracut-initqueue.service;
> > static)
> >Active: inactive (dead)
> >   start condition failed at Wed 2013-06-26 13:01:05 UTC; 22s ago
> >  Docs: man:dracut-initqueue.service(8)
> 
> I'd prefer if we'd solve this without the journal. Instead, I think
> "struct Condition" should be extended to store the most recent test
> result in a tri-state, i.e. negative for "untested", zero for "failed",
> positive for "succeeded". Then we should add bus property to the Unit
> interface that is an array of structs with the conditions in them,
> including their results.
I've now extended the dbus api, except that 0 means untested,
>0 means OK, <0 means failed, in case we decide to extend failed 
later on.

> With that in place systemctl should simple retrieve this property and
> show the results of *all* individual condition checks if the overall
> condition result was negatzive.
systemctl now will add a line (at most one), describing which condition
or condtions failed. Full output like in Harald's patch is not shown,
although we could still add it without any trouble, because full information
is exported over dbus. Nevertheless, I think current output is fine.

I'll update the wiki shortly.

Zbyszek
-- 
they are not broken. they are refucktored
   -- alxchk
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] runtime directories for services vs. tmpfiles

[Zbigniew Jędrzejewski-Szmek]

On Thu, Jul 18, 2013 at 02:51:09AM +0200, Lennart Poettering wrote:
[snip repoquery magic]

> This means: ~81% of the packages have been converted from sysv to
> systemd. And ~10% of the converted packages make use of tmpfiles.
> 
> Now, my rpm/yum-fu is a bit too limited to easily figure out what
> precisely they use of the tmpfiles functionality.

So, as the person who originally porposed adding tmpfiles to units,
I'm now more or less convinced that it's too late and not worth it.
Since the tmpfiles snippets have been written, and a big chunk is
done, it would be too confusing to add duplicate functionality.
People pointed out problems with the limited
RuntimeDirectory=/RuntimeDirectoryMode= proposal, and Lennart and
Kay are against the original idea to add tmpfiles snippets directly.
The unproven benefits are not worth it, and we certainly have other
things waiting on the table. So let's close this thread.

Zbyszek
-- 
they are not broken. they are refucktored
   -- alxchk
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journalctl: add --iso-dates for long timestamps

[Zbigniew Jędrzejewski-Szmek]

On Thu, Jul 18, 2013 at 01:36:51AM +0200, Lennart Poettering wrote:
> On Wed, 17.07.13 17:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> 
> > > +
> > > +
> > So, now we have -o short, -o short-monotonic, and --iso-dates.
> > I'm sure we'll add a relative timestamp mode like the excellent one in
> > dmesg --human output in recent util-linux. So I'd say that it makes
> > sense to deprecate short-monotonic and add a new switch --timestamps,
> > --timestamps=monotonic → old short-monotonic
> > --timestamps=iso-date → what you're proposing
> > --timestamps=...
> > 
> > This makes the whole thing easier to grok, imo. Journalctl already has
> > 1½ page listing of options...
> 
> I agree with Zbigniew here. Having this feature sounds useful, but
> please not in a new switch of its own. However I am not convinced that a
> --timestamps= switch would be necessary for this either, because I
> figure that the only --output= mode where iso dates would be useful
> would be "short". It's not useful for the "json", "json-pretty",
> "json-sse", "export", or "cat" modes, and only semi-useful for "verbose"
> mode. It's primarily useful for the "short" mode. Given that we already
> have a "short-monotonic" mode, maybe we should just add more like these?
> i.e. --output=short-iso or so?
> 
> If one day we really get multiple modes where ISO dates could be useful
> then maybe we could internally handles this by considering the --output=
> param no longer as single value but always as a pair separated by a dash
> or so. If you follow what I mean.
> 
> So, yeah, I'd think adding this as "--output=short-iso" or so would be
> the nicest?
Yep, that will work too, with less work.

Zbyszek
-- 
they are not broken. they are refucktored
   -- alxchk
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCHv2] core: notify triggered by socket of a service

[Lennart Poettering]

On Wed, 17.07.13 09:46, Umut Tezduyar (u...@tezduyar.com) wrote:

> +static void socket_trigger_notify(Unit *u, Unit *other) {
> +Socket *s = SOCKET(u);
> +Service *se = SERVICE(other);
> +
> +assert(u);
> +assert(other);
> +
> +if (other->load_state != UNIT_LOADED ||
> +other->type != UNIT_SERVICE)
> +return;
> +
> +if (se->state == SERVICE_FAILED && se->socket_fd < 0)
> +socket_notify_service_dead(s, se->result == 
> SERVICE_FAILURE_START_LIMIT);
> +
> +if ((se->state == SERVICE_DEAD ||
> +se->state == SERVICE_STOP ||
> +se->state == SERVICE_STOP_SIGTERM ||
> +se->state == SERVICE_STOP_SIGKILL ||
> +se->state == SERVICE_STOP_POST ||
> +se->state == SERVICE_FINAL_SIGTERM ||
> +se->state == SERVICE_FINAL_SIGKILL ||
> +se->state == SERVICE_AUTO_RESTART) &&
> +se->socket_fd < 0)
> +socket_notify_service_dead(s, false);
> +
> +if (!s->accept && se->state == SERVICE_RUNNING)
> +socket_set_state(s, SOCKET_RUNNING);
> +}

Hmm, hmm, so, the "se->socket_fd < 0" check will be true for all services
where the socket has !s->accept set. (after all, s->accept=true indicates
that we should accept the sockets, and then pass the connection socket
to the service, and that is stored in se->socket_fd, since it becomes
private property of that service. That is different for s->accept=false
where the socket continues to be owned by the socket unit). 

Hence, since all three if blocks effectivel just check for !s->accept,
could you split the check out and replace it with an early

if (s->accept)
return;

Immediately after the initial load/service type check?

Otherwise the patch looks great! Have you tested that everything works
with it? 

Thanks for working on this!

Lennart

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

Re: [systemd-devel] [PATCH] service: don't try to kill the service more than once when the watchdog timeout hits

[Lennart Poettering]

On Wed, 17.07.13 09:20, Michael Olbrich (m.olbr...@pengutronix.de) wrote:

> Hi,
> 
> On Wed, Jul 17, 2013 at 03:53:09AM +0200, Lennart Poettering wrote:
> > On Wed, 12.06.13 01:22, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > 
> > > If ExecStopPost= is defined then it is executed after SIGKILL. Otherwise
> > > another round of SIGTERM/SIGSTOP is started which is rather useless when
> > > the watchdog timeout hits.
> > > So go directly to the final SIGKILL if ExecStopPost= is not defined.
> > 
> > Hmm, why not go always directly into SERVICE_FINAL_SIGKILL? Why bother
> > with SERVICE_STOP_SIGKILL at all? What am I missing?
> 
> I think a small summery is necessary here:
> The original watchdog code just called service_enter_dead(). This had the
> problem that no cleanup happened. The process was not killed.
> Then I posted a patch to go irectly into SERVICE_FINAL_SIGKILL to fix that.
> It got applied.
> Then you changed it to SERVICE_STOP_SIGKILL so that ExecStopPost= is still
> called.
> However, this has a annoying side effect. Consider a service that enters
> service_handle_watchdog() with a process that cannot be killed. What
> happens is this:
> 
> service_handle_watchdog()
> STOP_SIGKILL
> wait
> ExecStopPost= if available
> FINAL_SIGTERM
> wait
> FINAL_SIGKILL
> wait
> service_enter_dead()
> 
> Without a ExecStopPost= defined anything after the first wait is just more
> useless waiting.
> I wrote this patch to avoid waiting longer than necessary but still be able
> to run ExecStopPost=.
> I later realized that this problem is not limited to the watchdog usecase.
> So I wrote a second patch (it should be applied instead of this one) that
> changes the sequence to:
> 
> [...]
> STOP_SIGKILL
> wait
> if (ExecStopPost= is available) {
>   ExecStopPost=
>   FINAL_SIGTERM
>   wait
>   FINAL_SIGKILL
>   wait
> }
> service_enter_dead()
> 
> I sent it as a reply to the first patch. I've also attached it for
> reference.

Ah, thanks! Sorry for the confusion and the way too long review time for
the patch.

Committed the second patch now. Thanks!

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] runtime directories for services vs. tmpfiles

[Lennart Poettering]

On Wed, 17.07.13 11:24, Michael Biebl (mbi...@gmail.com) wrote:

> 
> 2013/7/17 Michael Biebl :
> > If this scheme is not flexible enough to cover the vast majority of
> > all cases (for services), then I fear we'd end up half of the services
> > using RuntimeDirectory, the other half a tmpfile. And that imho would
> > be even more confusing.
> 
> Could we have some stats from Fedora and/or Arch which have done a
> (complete) migration to systemd, about how many services/packages
> currently use a tmpfile and what they use in the tmpfile, i.e. if the
> proposed scheme from Lennart would suffice say for 90+ % of them

Fedora is not completely converted, but here are some stats.

The number of packages with tmpfiles:

$ repoquery --whatprovides '/usr/lib/tmpfiles.d/*' --qf "%{name}" | sort -u 
| wc -l
59

The full list is here: http://ur1.ca/eonvl

The number of packages with service files:

$ repoquery --whatprovides '/usr/lib/systemd/system/*' --qf "%{name}" | 
sort -u | wc -l
602
 
The full list is here: http://ur1.ca/eonxq

The number of holdouts with init scripts:

$ repoquery --whatprovides '/etc/rc.d/init.d/*' --qf "%{name}" | sort -u | 
wc -l
167

This is highly misleading however as it is permitted by the fedora
policy to continue sysv scripts if this is done in a separate package
from the main one. Let's manually sort out these cases:

$ repoquery --whatprovides '/etc/rc.d/init.d/*' --qf "%{name}" | sort -u | 
egrep -v '(-sysvinit|-initscript|-sysv)$' | wc -l
139

The full list is here: http://ur1.ca/eoo02

This means: ~81% of the packages have been converted from sysv to
systemd. And ~10% of the converted packages make use of tmpfiles.

Now, my rpm/yum-fu is a bit too limited to easily figure out what
precisely they use of the tmpfiles functionality.

This is Fedora 19.

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] Stop all uninstalled units

[Tim Cuthbertson]

On Thu, Jul 18, 2013 at 9:47 AM, Lennart Poettering
wrote:

> On Wed, 17.07.13 22:14, Tim Cuthbertson (t...@gfxmonk.net) wrote:
>
> > Hi Folks,
> >
> > I've posted a serverfault question [0], but it got no answers so I
> thought
> > I'd bring it to the mailing list:
> >
> > I have some systemd units installed and running. Let's say I manually
> > uninstall foo.service by
> >
> >  - removing its .service (and .socket) files
> >  - removing all symlinks (e.g from default.target.wants/)
> >
> > I can run systemctl daemon-reload, and then I see:
> >
> > # systemctl status foo.service
> > foo.service
> >Loaded: error (Reason: No such file or directory)
> >Active: active (running) since Mon 2013-07-08 13:50:29 EST; 48s
> ago
> >  Main PID: 1642 (node)
> >
> > So systemd knows that it's not installed, and that it is running. Is
> there
> > some command I can use to stop all running services which no longer have
> a
> > unit file?
> >
> > I do not want to have to somehow know what I've uninstalled, or for the
> > command to work only in some circumstances - I want something that I can
> > run at any point, with no additional knowledge, that will stop all units
> > not backed by a unit file.
> >
> > I've currently got a hacky script that:
> >  - Runs `list-units` to get all unit names
> >  - Then `show -p ActiveState -p UnitFileState $unit` on each
> >  - It then runs `systemctl stop` on each unit with a UnitFileState of ""
> > (empty string), and ActiveState of anything but "failed".
> >
> > This is almost certainly missing some edge case, as I couldn't really
> find
> > any documentation of these properties, it just seemed to be what occurred
> > with the examples I encountered.
> >
> > I'm hoping there's a better way, can anyone point me in the right
> direction?
> >
> > [0]:
> >
> http://serverfault.com/questions/521504/systemd-stop-all-uninstalled-units
>
> So, the correct way to handle this is to make sure the packages in
> question contain the right scriplets that terminate all units they
> include before deinstallation. Of course, there'll always be broken
> packages like this I fear, hence I can totally see your usecase.
>
> There's currently no nice way to handle this, but what you can do is this:
>
> systemctl --all --type=not-found --type=error
>
> This will list you all units where the load state is either "error" or
> "not-found" (note that 'not-found' is available only in very recent
> versions, and in older systemd versions was just a special case of
> 'error'. The command line above works for all versions). The --type=
> switch is used to filter unit types, but actually also can be used to
> filter for the load state.
>
> Then filter out the first column:
>
> systemctl --no-legend --all --type=not-found --type=error | awk '{ print
> $1 }'
>
> This will give you a list of unit files which got referenced or started
> but have no unit file installed. Then use this to stop the units:
>
> systemctl stop `systemctl --no-legend -all --type=not-found --type=error |
> awk '{ print $1}'`
>
> And there you go.
>
> Lennart
>
> --
> Lennart Poettering - Red Hat, Inc.
>

Thanks, Lennart (and Colin), that looks like a much better approach,
although --type=not-found is rejected by old versions:

$ systemctl --all --type=not-found --type=error
Unknown unit type or load state 'not-found'.
Use -t help to see a list of allowed values.



$ systemctl --version
systemd 204
+PAM +LIBWRAP +AUDIT +SELINUX +IMA +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ

But I can probably try both, and fall back to just --error if not-found is
denied.

I am not too concerned about systemd not knowing how best to kill the units
- they don't contain any special stop actions, so it would be using the
default action to kill the service anyway. But point taken, in the general
case this is probably not advisable.

For background of why this might happen (if you or Colin are still
curious), I'm installing systemd units not as part of any package -
basically, the user has some application-specific config that they can
change at any moment. They run:

$ foo install 

And (regardless of any previous state), the units generated by that config
file (and no units defined by previous invocations) should be installed /
running. I put an X-Generated-By-MyApp=true in each of the generated unit
files, so I can scan the /etc/systemd/system folder for previously
installed units. This should mean that I can stop them immediately, but I
wanted a failsafe way of doing it just in case things get left in an
unknown state (because of bugs or unanticipated failure modes in my code).

I also had the notion if being able to affect a system from another with
access to its filesystem, e.g. `foo install config-file
--root=/srv/container1/etc/systemd/system` or by running the unit
generation once and rsync'ing the installed units to multiple identical
VMs, and then running a routine task in the container to "fix up" the state
to reflect the inst

Re: [systemd-devel] [PATCHv3] systemctl, man: option to list units by state

[Lennart Poettering]

On Wed, 17.07.13 13:01, Maciej Wereski (m.were...@partner.samsung.com) wrote:

> This allows to show only units with specified SUB or ACTIVE state.
> 
> Signed-off-by: Maciej Wereski 

Hmm, could you make --state= check all three states please? i.e. active,
sub and load state equally. And leave --type= as it is right now for
compat reasons meaning you can check the load state via both --state=
and --type= that way.

Then, I'd like to see --failed and the use of --type= for filtering for
load state deprecated, even if we continue to support them. We usually
do that simply by removing them from the documentation and --help texts,
and just leaving them in as "secret" compat features...

With all that in place we'd not expose any redundancy to the user. We'd
have --state= for checking for any of the three states, and we'd have
--type= for checking for unit types, and that'd (officially) be all we
expose.

Can I convince you to update your patch accordingly? i.e. removal of
--fail and the load state bits of --type= from the man page and --help,
and make --state= check work for load state too?

>  case ARG_FAILED:
> -arg_failed = true;
> +if (!strv_contains(arg_state, "failed")) {
> +char **tmp = arg_state;
> +arg_state = strv_append(arg_state, "failed");
> +if (!arg_state) {
> +strv_free(tmp);
> +return -ENOMEM;

Consider using strv_extend() for this, which makes this code much shorter.

> +FOREACH_WORD_SEPARATOR(word, size, optarg, ",", 
> state) {
> +char _cleanup_free_ *str;
> +
> +str = strndup(word, size);
> +if (!str)
> +return -ENOMEM;
> + if (!strv_contains(arg_state, str)) {
> +char **tmp = arg_state;
> +arg_state =
> strv_append(arg_state, str);

And for this one please use strv_push()!

strv_extend() and strv_push() have the same signatures. strv_extends()
just extends an existing strv array, and makes a copy of the passed in
string for that. This is what you want for adding the static const
string "failed" to it. And strv_push() also extends the strv array, but
takes possession of the string you pass. Which is great for the case
where you strndup()ed the string anyway.

Hope that makes sense, and thanks a lot for working on this and for
putting up with my awful review times for the original patch!

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] Stop all uninstalled units

[Lennart Poettering]

On Wed, 17.07.13 22:14, Tim Cuthbertson (t...@gfxmonk.net) wrote:

> Hi Folks,
> 
> I've posted a serverfault question [0], but it got no answers so I thought
> I'd bring it to the mailing list:
> 
> I have some systemd units installed and running. Let's say I manually
> uninstall foo.service by
> 
>  - removing its .service (and .socket) files
>  - removing all symlinks (e.g from default.target.wants/)
> 
> I can run systemctl daemon-reload, and then I see:
> 
> # systemctl status foo.service
> foo.service
>Loaded: error (Reason: No such file or directory)
>Active: active (running) since Mon 2013-07-08 13:50:29 EST; 48s ago
>  Main PID: 1642 (node)
> 
> So systemd knows that it's not installed, and that it is running. Is there
> some command I can use to stop all running services which no longer have a
> unit file?
> 
> I do not want to have to somehow know what I've uninstalled, or for the
> command to work only in some circumstances - I want something that I can
> run at any point, with no additional knowledge, that will stop all units
> not backed by a unit file.
> 
> I've currently got a hacky script that:
>  - Runs `list-units` to get all unit names
>  - Then `show -p ActiveState -p UnitFileState $unit` on each
>  - It then runs `systemctl stop` on each unit with a UnitFileState of ""
> (empty string), and ActiveState of anything but "failed".
> 
> This is almost certainly missing some edge case, as I couldn't really find
> any documentation of these properties, it just seemed to be what occurred
> with the examples I encountered.
> 
> I'm hoping there's a better way, can anyone point me in the right direction?
> 
> [0]:
> http://serverfault.com/questions/521504/systemd-stop-all-uninstalled-units

So, the correct way to handle this is to make sure the packages in
question contain the right scriplets that terminate all units they
include before deinstallation. Of course, there'll always be broken
packages like this I fear, hence I can totally see your usecase.

There's currently no nice way to handle this, but what you can do is this:

systemctl --all --type=not-found --type=error

This will list you all units where the load state is either "error" or
"not-found" (note that 'not-found' is available only in very recent
versions, and in older systemd versions was just a special case of
'error'. The command line above works for all versions). The --type=
switch is used to filter unit types, but actually also can be used to
filter for the load state.

Then filter out the first column:

systemctl --no-legend --all --type=not-found --type=error | awk '{ print $1 }'

This will give you a list of unit files which got referenced or started
but have no unit file installed. Then use this to stop the units:

systemctl stop `systemctl --no-legend -all --type=not-found --type=error | awk 
'{ print $1}'`

And there you go.

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] journalctl: add --iso-dates for long timestamps

[Lennart Poettering]

On Wed, 17.07.13 17:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> > +
> > +
> So, now we have -o short, -o short-monotonic, and --iso-dates.
> I'm sure we'll add a relative timestamp mode like the excellent one in
> dmesg --human output in recent util-linux. So I'd say that it makes
> sense to deprecate short-monotonic and add a new switch --timestamps,
> --timestamps=monotonic → old short-monotonic
> --timestamps=iso-date → what you're proposing
> --timestamps=...
> 
> This makes the whole thing easier to grok, imo. Journalctl already has
> 1½ page listing of options...

I agree with Zbigniew here. Having this feature sounds useful, but
please not in a new switch of its own. However I am not convinced that a
--timestamps= switch would be necessary for this either, because I
figure that the only --output= mode where iso dates would be useful
would be "short". It's not useful for the "json", "json-pretty",
"json-sse", "export", or "cat" modes, and only semi-useful for "verbose"
mode. It's primarily useful for the "short" mode. Given that we already
have a "short-monotonic" mode, maybe we should just add more like these?
i.e. --output=short-iso or so?

If one day we really get multiple modes where ISO dates could be useful
then maybe we could internally handles this by considering the --output=
param no longer as single value but always as a pair separated by a dash
or so. If you follow what I mean.

So, yeah, I'd think adding this as "--output=short-iso" or so would be
the nicest?

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] Stop all uninstalled units

[Tim Cuthbertson]

Hi Folks,

I've posted a serverfault question [0], but it got no answers so I thought
I'd bring it to the mailing list:

I have some systemd units installed and running. Let's say I manually
uninstall foo.service by

 - removing its .service (and .socket) files
 - removing all symlinks (e.g from default.target.wants/)

I can run systemctl daemon-reload, and then I see:

# systemctl status foo.service
foo.service
   Loaded: error (Reason: No such file or directory)
   Active: active (running) since Mon 2013-07-08 13:50:29 EST; 48s ago
 Main PID: 1642 (node)

So systemd knows that it's not installed (i.e backed by a file), and that
it is running. Is there some command I can use to stop all running services
which no longer have a unit file?

I do not want to have to somehow know what I've uninstalled, or for the
command to work only in some circumstances - I want something that I can
run at any point, with no additional knowledge, that will stop all units
not backed by a unit file.

I've currently got a hacky script that:
 - Runs `list-units` to get all unit names
 - Then `show -p ActiveState -p UnitFileState $unit` on each
  - It then runs `systemctl stop` on each unit with a UnitFileState of ""
(empty string), and ActiveState of anything but "failed".

This is almost certainly missing some edge case, as I couldn't really find
any documentation of these properties, it just seemed to be what occurred
with the examples I encountered.

I'm hoping there's a better way, can anyone point me in the right direction?

[0]:
http://serverfault.com/questions/521504/systemd-stop-all-uninstalled-units

Thanks,
- Tim Cuthbertson.

(apologies if this email arrives on the list twice, I think the first time
didn't make it through because I wasn't subscribed to the list)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journal: return -ECHILD after a fork

[Lennart Poettering]

On Wed, 17.07.13 11:05, Shawn (shawnland...@gmail.com) wrote:

> Wouldn't this be better using pthread_atfork() to avoid all the getpid()
> calls?

No pthread_atfork() is not useful for much since it gives no ordering
guarantees. 

Threading and forking doesn't mix well, and neither do single-use open
file descriptors.

I am just not interested in the bug reports. fork() on Unix is a very
special thing. The only reason to ever call it is because you want to
invoke exec() in the child quickly. Everything else is just broken, and
slow (page faults due to Cow...) and browsing the journal certainly is
certainly nothing you ever need for preparing before exec().

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] hwdb for gphoto2

[Kay Sievers]

On Thu, Jul 18, 2013 at 12:59 AM, Tom Gundersen  wrote:

> Out of interest, what's the best way to measure the time event processing 
> takes?

$ grep . /sys/bus/usb/devices/*/modalias
/sys/bus/usb/devices/1-0:1.0/modalias:usb:v1D6Bp0002d0309dc09dsc00dp00ic09isc00ip00in00
/sys/bus/usb/devices/1-1:1.0/modalias:usb:v8087p0024ddc09dsc00dp01ic09isc00ip00in00
/sys/bus/usb/devices/1-1.2:1.0/modalias:usb:v0424p2514ddc09dsc00dp02ic09isc00ip02in00
...

$ time udevadm hwdb --test=usb:v8087p0024ddc09dsc00dp01ic09isc00ip00in00
ID_MODEL_FROM_DATABASE=Integrated Rate Matching Hub
ID_USB_CLASS_FROM_DATABASE=Hub
ID_USB_PROTOCOL_FROM_DATABASE=Single TT
ID_VENDOR_FROM_DATABASE=Intel Corp.

real0m0.003s
user0m0.001s
sys 0m0.001s

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

Re: [systemd-devel] hwdb for gphoto2

[Tom Gundersen]

On Wed, Jul 17, 2013 at 11:54 AM, Kay Sievers  wrote:
> On Wed, Jul 17, 2013 at 11:31 AM, Tom Gundersen  wrote:
>> Hi guys,
>>
>> Here is an attempt at converting libgphoto2 to hwdb. Seems to work for my 
>> phone.
>>
>> hwdb file: 
>> patch: 
>>
>> Comments welcome.
>
> Looks both great, the sane and gphoto stuff.

Great.

Both submitted upstream, in case anyone is interested:

gphoto2 (accepted):


sane (pending):
.

> Should be a huge improvement over the insanely big udev rules hack.

Out of interest, what's the best way to measure the time event processing takes?

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

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

[Lucas De Marchi]

On Fri, Jul 12, 2013 at 2:51 PM, Lennart Poettering
 wrote:
> On Tue, 09.07.13 18:09, Holger Hans Peter Freyther (hol...@freyther.de) wrote:
>
>> From: Holger Hans Peter Freyther 
>>
>> Instruct travis-ci to build systemd and create a tarball. In case
>> of an error travis-ci will complain on IRC. The systemd testsuite
>> currently requires the host to have a recent version of systemd
>> installed and running. This is not the case for the Ubuntu VM of
>> travis-ci. This means make check and make distcheck will result in
>> a build failure and to avoid this these commands are not executed.
>>
>> This requires a one time configuration on travis-ci for the repo
>> on github by the owner of the repo.
>
> Commited. I figure this will not be useful right-away, since current git
> of systemd requires a very new version of kmod that is not available at
> Travis. However, it's still a good thing to have and to enable as soon
> as that issue is solved.
>
> BTW, I am totally happy if our tests get fixed (or even skipped) to not
> require a systemd running. It's simply that on our own machines we never
> have these issues, and that's where we run "make check" most of the
> time.
>
> I figure a simple
>
> if (sd_booted() <= 0)
> return EXIT_TEST_SKIP;
>
> at the top of the tests in question would already be a big step
> forward. And then we could work on enabling more tests as things go.
>

Talking about kmod and travis-ci. In kmod we are using it. Pretty
cool, with an IRC bot integration. I only wish there were more options
of distributions.  Right now it's only Ubuntu 12.04 32 bits

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

Re: [systemd-devel] [PATCH] journal: return -ECHILD after a fork

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jul 17, 2013 at 11:05:38AM -0700, Shawn wrote:
> Hi!
> 
> 
> On Tue, Jul 16, 2013 at 8:50 AM, Lennart Poettering
> wrote:
> 
> > On Wed, 26.06.13 19:55, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
> > wrote:
> >
> > > A few asserts are replaced with 'return -EINVAL'. I think that
> > > assert should not be used to check argument in public functions.
> > >
> > > Fields in struct sd_journal are rearranged to make it less
> > > swiss-cheesy.
> > > ---
> > > Does this look sound?
> >
> > Yupp! Looks great! Please commit!
> >
> Wouldn't this be better using pthread_atfork() to avoid all the getpid()
> calls?
For better or worse, getpid() results are cached by glibc anyway, so
current implementation should be efficient enough.

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

Re: [systemd-devel] [PATCH] journal: return -ECHILD after a fork

[Shawn]

Hi!


On Tue, Jul 16, 2013 at 8:50 AM, Lennart Poettering
wrote:

> On Wed, 26.06.13 19:55, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
> wrote:
>
> > A few asserts are replaced with 'return -EINVAL'. I think that
> > assert should not be used to check argument in public functions.
> >
> > Fields in struct sd_journal are rearranged to make it less
> > swiss-cheesy.
> > ---
> > Does this look sound?
>
> Yupp! Looks great! Please commit!
>
Wouldn't this be better using pthread_atfork() to avoid all the getpid()
calls?

I made a new patch using a global g_forked, but realized it had a race:

create journal
fork()
create second journal
log to first journal object

> >
> > Zbyszek
> >
> >  TODO   |  5 +--
> >  man/sd_journal_open.xml|  7 
> >  src/journal/journal-internal.h | 18 +-
> >  src/journal/sd-journal.c   | 80
> +++---
> >  4 files changed, 93 insertions(+), 17 deletions(-)
> >
> > diff --git a/TODO b/TODO
> > index caba4e3..2968d18 100644
> > --- a/TODO
> > +++ b/TODO
> > @@ -68,7 +68,7 @@ Features:
> >
> >  * document systemd-journal-flush.service properly
> >
> > -* chane systemd-journal-flush into a service that stays around during
> > +* change systemd-journal-flush into a service that stays around during
> >boot, and causes the journal to be moved back to /run on shutdown,
> >so that we don't keep /var busy. This needs to happen synchronously,
> >hence doing this via signals is not going to work.
> > @@ -76,9 +76,6 @@ Features:
> >  * allow implementation of InaccessibleDirectories=/ plus
> >ReadOnlyDirectories=... for whitelisting files for a service.
> >
> > -* libsystemd-journal:
> > -  - return ECHILD as soon as somebody tries to reuse a journal object
> across a fork()
> > -
> >  * libsystemd-bus:
> >- default policy (allow uid == 0 and our own uid)
> >- enforce alignment of pointers passed in
> > diff --git a/man/sd_journal_open.xml b/man/sd_journal_open.xml
> > index d7ea8ff..b2f6f9e 100644
> > --- a/man/sd_journal_open.xml
> > +++ b/man/sd_journal_open.xml
> > @@ -131,6 +131,13 @@
> >  can be rotated at any moment, and the opening of
> >  specific files is inherently racy.
> >
> > +sd_journal objects cannot be
> > +used in the child after a fork. Functions which take a
> > +journal object as an argument
> > +(sd_journal_next() and others)
> > +will return -ECHILD after a fork.
> > +
> > +
> >  sd_journal_close() will
> >  close the journal context allocated with
> >  sd_journal_open() or
> > diff --git a/src/journal/journal-internal.h
> b/src/journal/journal-internal.h
> > index 5b717f8..5bc6535 100644
> > --- a/src/journal/journal-internal.h
> > +++ b/src/journal/journal-internal.h
> > @@ -97,8 +97,6 @@ struct Directory {
> >  };
> >
> >  struct sd_journal {
> > -int flags;
> > -
> >  char *path;
> >
> >  Hashmap *files;
> > @@ -109,27 +107,29 @@ struct sd_journal {
> >  JournalFile *current_file;
> >  uint64_t current_field;
> >
> > -Hashmap *directories_by_path;
> > -Hashmap *directories_by_wd;
> > -
> > -int inotify_fd;
> > -
> >  Match *level0, *level1, *level2;
> >
> > +pid_t original_pid;
> > +
> > +int inotify_fd;
> >  unsigned current_invalidate_counter, last_invalidate_counter;
> > +usec_t last_process_usec;
> >
> >  char *unique_field;
> >  JournalFile *unique_file;
> >  uint64_t unique_offset;
> >
> > +int flags;
> > +
> >  bool on_network;
> >  bool no_new_files;
> >
> >  size_t data_threshold;
> >
> > -Set *errors;
> > +Hashmap *directories_by_path;
> > +Hashmap *directories_by_wd;
> >
> > -usec_t last_process_usec;
> > +Set *errors;
> >  };
> >
> >  char *journal_make_match_string(sd_journal *j);
> > diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
> > index 1e70739..81b0c13 100644
> > --- a/src/journal/sd-journal.c
> > +++ b/src/journal/sd-journal.c
> > @@ -50,6 +50,15 @@
> >
> >  #define DEFAULT_DATA_THRESHOLD (64*1024)
> >
> > +static bool journal_pid_changed(sd_journal *j) {
> > +assert(j);
> > +
> > +/* We don't support people creating a journal object and
> > + * keeping it around over a fork(). Let's complain. */
> > +
> > +return j->original_pid != getpid();
> > +}
> > +
> >  /* We return an error here only if we didn't manage to
> > memorize the real error. */
> >  static int set_put_error(sd_journal *j, int r) {
> > @@ -209,6 +218,8 @@ _public_ int sd_journal_add_match(sd_journal *j,
> const void *data, size_t size)
> >
> >  if (!j)
> >  return -EINVAL;
> > +if (journal_pid_changed(j))
> > + 

Re: [systemd-devel] [PATCH] journalctl: add --iso-dates for long timestamps

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jul 17, 2013 at 05:15:05PM +0200, Tomasz Torcz wrote:
> Add possibility have "short" output with long format ISO 8601 timestamps,
> like "2008-05-28T14:14:46.316223-04:00".
> ---
>  man/journalctl.xml   |  8 
>  src/journal/journalctl.c | 11 ++-
>  src/shared/logs-show.c   |  8 +++-
>  src/shared/output-mode.h |  3 ++-
>  4 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/man/journalctl.xml b/man/journalctl.xml
> index 3e03c45..8c9a796 100644
> --- a/man/journalctl.xml
> +++ b/man/journalctl.xml
> @@ -170,6 +170,14 @@
>  
>  
>  
> +--iso-dates
> +
> +In "short" output mode,
> +show timestamps in verbose, ISO 8601 format.
> +
> +
So, now we have -o short, -o short-monotonic, and --iso-dates.
I'm sure we'll add a relative timestamp mode like the excellent one in
dmesg --human output in recent util-linux. So I'd say that it makes
sense to deprecate short-monotonic and add a new switch --timestamps,
--timestamps=monotonic → old short-monotonic
--timestamps=iso-date → what you're proposing
--timestamps=...

This makes the whole thing easier to grok, imo. Journalctl already has
1½ page listing of options...

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

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

[Kay Sievers]

On Wed, Jul 17, 2013 at 9:18 AM, Holger Hans Peter Freyther
 wrote:
> On Fri, Jul 12, 2013 at 07:51:26PM +0200, Lennart Poettering wrote:
>
>> Commited. I figure this will not be useful right-away, since current git
>> of systemd requires a very new version of kmod that is not available at
>> Travis. However, it's still a good thing to have and to enable as soon
>> as that issue is solved.
>
> I could only check this today. The ubuntu/travis build is currently not
> using kmod at all so it could be useful right-away. What is missing is
> that any member of the systemd group on github visits the travis-ci.org
> website, clicks on 'Sign in with GitHub', go to the account section and
> enables travis-ci for the systemd repository. It should be < 1 minute of
> work.

I did something like that now. Let's see if it works ...

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

Re: [systemd-devel] Stop all uninstalled units

[Colin Guthrie]

'Twas brillig, and Tim Cuthbertson at 17/07/13 13:14 did gyre and gimble:
> Hi Folks,
> 
> I've posted a serverfault question [0], but it got no answers so I
> thought I'd bring it to the mailing list:
> 
> I have some systemd units installed and running. Let's say I manually
> uninstall foo.service by
> 
>  - removing its .service (and .socket) files
>  - removing all symlinks (e.g from default.target.wants/)
> 
> I can run systemctl daemon-reload, and then I see:
> 
> # systemctl status foo.service
> foo.service
>Loaded: error (Reason: No such file or directory)
>Active: active (running) since Mon 2013-07-08 13:50:29 EST; 48s ago
>  Main PID: 1642 (node)
> 
> So systemd knows that it's not installed, and that it is running. Is
> there some command I can use to stop all running services which no
> longer have a unit file?
> 
> I do not want to have to somehow know what I've uninstalled, or for the
> command to work only in some circumstances - I want something that I can
> run at any point, with no additional knowledge, that will stop all units
> not backed by a unit file.

You should really not uninstall something before stopping it. This is
why all the packaging systems will always be careful to stop things
before uninstalling.

I'm unsure, but I would have thought systemd would no longer know the
exact sequence of commands to run when stopping the unit without the
unit file to guide it (it depends how much of the original unit was kept
around when doing a daemon-reload, but I would hope it would clean out
pretty much everything).

Likely all that systemd is doing here is killing the processes that
remain in the cgroup rather than any kind of graceful shutdown (but then
sometimes a graceful shutdown would require running binaries that no
longer exist anyway so this is probably a good thing).

> I've currently got a hacky script that:
>  - Runs `list-units` to get all unit names
>  - Then `show -p ActiveState -p UnitFileState $unit` on each
>  - It then runs `systemctl stop` on each unit with a UnitFileState of ""
> (empty string), and ActiveState of anything but "failed".
> 
> This is almost certainly missing some edge case, as I couldn't really
> find any documentation of these properties, it just seemed to be what
> occurred with the examples I encountered.
> 
> I'm hoping there's a better way, can anyone point me in the right direction?

To be honest, if this is something you encounter so often that you need
to have a script for it, I would suggest "you're doing it wrong" ;)

The units should be shipped in the same package as the service binary so
the units should only really be removed when the service's package is
removed and it should be the job of the packaging system to ensure the
service is stopped before it the files relating to that package are
unlinked.

Maybe I'm missing the reason things end up in that state so you could
elaborate on how this situation crops up then perhaps people here could
advise better.

Regardless, of whether or not this is good or bad practice, I think the
principle you described above seems sensible enough.

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/

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

[systemd-devel] [PATCH] journalctl: add --iso-dates for long timestamps

[Tomasz Torcz]

Add possibility have "short" output with long format ISO 8601 timestamps,
like "2008-05-28T14:14:46.316223-04:00".
---
 man/journalctl.xml   |  8 
 src/journal/journalctl.c | 11 ++-
 src/shared/logs-show.c   |  8 +++-
 src/shared/output-mode.h |  3 ++-
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/man/journalctl.xml b/man/journalctl.xml
index 3e03c45..8c9a796 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -170,6 +170,14 @@
 
 
 
+--iso-dates
+
+In "short" output mode,
+show timestamps in verbose, ISO 8601 format.
+
+
+
+
 -f
 --follow
 
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 7099706..3709319 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -65,6 +65,7 @@ static bool arg_pager_end = false;
 static bool arg_follow = false;
 static bool arg_full = false;
 static bool arg_all = false;
+static bool arg_iso_dates = false;
 static bool arg_no_pager = false;
 static int arg_lines = -1;
 static bool arg_no_tail = false;
@@ -138,6 +139,7 @@ static int help(void) {
"  -x --catalog Add message explanations where 
available\n"
"  -l --fullDo not ellipsize fields\n"
"  -a --all Show all fields, including long and 
unprintable\n"
+   " --iso-dates   Show ISO 8601 full timestamps in 
short output mode\n"
"  -q --quiet   Don't show privilege warning\n"
" --no-pagerDo not pipe output into a pager\n"
"  -m --merge   Show entries from all available 
journals\n"
@@ -194,6 +196,7 @@ static int parse_argv(int argc, char *argv[]) {
 ARG_DUMP_CATALOG,
 ARG_UPDATE_CATALOG,
 ARG_FORCE,
+ARG_ISO_DATES,
 };
 
 static const struct option options[] = {
@@ -239,6 +242,7 @@ static int parse_argv(int argc, char *argv[]) {
 { "dump-catalog",   no_argument,   NULL, ARG_DUMP_CATALOG  
 },
 { "update-catalog", no_argument,   NULL, 
ARG_UPDATE_CATALOG },
 { "reverse",no_argument,   NULL, 'r'   
 },
+{ "iso-dates",  no_argument,   NULL, ARG_ISO_DATES 
 },
 { NULL, 0, NULL, 0 
 }
 };
 
@@ -300,6 +304,10 @@ static int parse_argv(int argc, char *argv[]) {
 arg_all = true;
 break;
 
+case ARG_ISO_DATES:
+arg_iso_dates = true;
+break;
+
 case 'n':
 if (optarg) {
 r = safe_atoi(optarg, &arg_lines);
@@ -1605,7 +1613,8 @@ int main(int argc, char *argv[]) {
 arg_all * OUTPUT_SHOW_ALL |
 (arg_full || !on_tty() || pager_have()) * 
OUTPUT_FULL_WIDTH |
 on_tty() * OUTPUT_COLOR |
-arg_catalog * OUTPUT_CATALOG;
+arg_catalog * OUTPUT_CATALOG |
+arg_iso_dates * OUTPUT_ISO_DATES;
 
 r = output_journal(stdout, j, arg_output, 0, flags);
 need_seek = true;
diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c
index ea47468..1753ca8 100644
--- a/src/shared/logs-show.c
+++ b/src/shared/logs-show.c
@@ -258,6 +258,7 @@ static int output_short(
 uint64_t x;
 time_t t;
 struct tm tm;
+const char *timestamp_format;
 
 r = -ENOENT;
 
@@ -272,8 +273,13 @@ static int output_short(
 return r;
 }
 
+   if (flags & OUTPUT_ISO_DATES)
+   timestamp_format = "%Y-%m-%dT%H:%M:%S%z";
+   else
+   timestamp_format = "%b %d %H:%M:%S";
+
 t = (time_t) (x / USEC_PER_SEC);
-if (strftime(buf, sizeof(buf), "%b %d %H:%M:%S", 
localtime_r(&t, &tm)) <= 0) {
+if (strftime(buf, sizeof(buf), timestamp_format, 
localtime_r(&t, &tm)) <= 0) {
 log_error("Failed to format time.");
 return r;
 }
diff --git a/src/shared/output-mode.h b/src/shared/output-mode.h
index 0efd430..0b860e5 100644
--- a/src/shared/output-mode.h
+++ b/src/shared/output-mode.h
@@ -40,5 +40,6 @@ typedef enum OutputFlags {
 OUTPUT_WARN_CUTOFF= 1 << 2,
 OUTP

[systemd-devel] [PATCH 2/2] test: Make testing work on systems without or old systemd

[Holger Hans Peter Freyther]

From: Holger Hans Peter Freyther 

* Introduce a macro to conditionally execute tests. This avoids
  skipping the entire test if some parts require systemd
* Skip the journal tests when no /etc/machine-id is present
* Change test-catalog to load the catalog from the source directory
  of systemd.
* /proc/PID/comm got introduced in v2.6.33 but travis is still
  using v2.6.32.
* Enable make check and make distcheck on the travis build
---
 .travis.yml |  5 +++--
 Makefile.am | 13 +++--
 src/journal/test-catalog.c  |  7 ++-
 src/journal/test-journal-interleaving.c |  5 +
 src/journal/test-journal-stream.c   |  5 +
 src/journal/test-journal-verify.c   |  4 
 src/journal/test-journal.c  |  5 +
 src/test/test-cgroup-util.c |  5 +++--
 src/test/test-helper.h  | 31 +++
 src/test/test-id128.c   | 11 +++
 src/test/test-unit-file.c   |  3 ++-
 src/test/test-unit-name.c   |  5 -
 src/test/test-util.c|  9 +++--
 13 files changed, 93 insertions(+), 15 deletions(-)
 create mode 100644 src/test/test-helper.h

diff --git a/.travis.yml b/.travis.yml
index 42433fd..7e5251c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,8 +3,9 @@ compiler:
   - gcc
 before_install:
  - sudo apt-get update -qq
- - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf
-script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && make dist V=1
+ - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev 
libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev 
libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev 
libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev
+script: ./autogen.sh && ./configure --enable-gtk-doc --enable-gtk-doc-pdf && 
make V=1 && sudo ./systemd-machine-id-setup && make check && make distcheck
+after_failure: cat test-suite.log
 notifications:
   irc:
 channels:
diff --git a/Makefile.am b/Makefile.am
index c4b9b1a..e598585 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1130,6 +1130,9 @@ EXTRA_DIST += \
test/sched_rr_ok.service \
test/sched_rr_change.service
 
+EXTRA_DIST += \
+   src/test/test-helper.h
+
 test_engine_SOURCES = \
src/test/test-engine.c
 
@@ -1328,7 +1331,8 @@ test_cgroup_util_SOURCES = \
 
 test_cgroup_util_LDADD = \
libsystemd-label.la \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-daemon.la
 
 test_env_replace_SOURCES = \
src/test/test-env-replace.c
@@ -2647,7 +2651,8 @@ test_id128_SOURCES = \
 
 test_id128_LDADD = \
libsystemd-shared.la \
-   libsystemd-id128-internal.la
+   libsystemd-id128-internal.la \
+   libsystemd-daemon.la
 
 tests += \
test-id128
@@ -2810,6 +2815,10 @@ test_mmap_cache_LDADD = \
 test_catalog_SOURCES = \
src/journal/test-catalog.c
 
+test_catalog_CFLAGS = \
+   $(AM_CFLAGS) \
+   -D"STR(s)=\#s" -D"CATALOG_DIR=STR($(abs_top_srcdir)/catalog)"
+
 test_catalog_LDADD = \
libsystemd-shared.la \
libsystemd-label.la \
diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c
index 987867f..9620301 100644
--- a/src/journal/test-catalog.c
+++ b/src/journal/test-catalog.c
@@ -31,6 +31,11 @@
 #include "sd-messages.h"
 #include "catalog.h"
 
+static const char *catalog_dirs[] = {
+CATALOG_DIR,
+NULL,
+};
+
 static void test_import(Hashmap *h, struct strbuf *sb,
 const char* contents, ssize_t size, int code) {
 int r;
@@ -102,7 +107,7 @@ static void test_catalog_update(void) {
 
 /* Note: this might actually not find anything, if systemd was
  * not installed before. That should be fine too. */
-r = catalog_update(database, NULL, catalog_file_dirs);
+r = catalog_update(database, NULL, catalog_dirs);
 assert(r >= 0);
 }
 
diff --git a/src/journal/test-journal-interleaving.c 
b/src/journal/test-journal-interleaving.c
index 069d297..bd3cb7f 100644
--- a/src/journal/test-journal-interleaving.c
+++ b/src/journal/test-journal-interleaving.c
@@ -286,8 +286,13 @@ static void test_sequence_numbers(void) {
 }
 
 int main(int argc, char *argv[]) {
+struct stat st;
 log_set_max_level(LOG_DEBUG);
 
+/* journal_file_open requires a valid machine id */
+if (stat("/etc/machine-id", &st) != 0)
+return EXIT_TEST_SKIP;
+
 arg_keep = argc > 1;
 
 test_skip(setup_sequential);
diff --git a/src/journal/test-journal-stream.c 
b/src/journal/test-journal-stream.c
index 4aba7fe..34f44f6 100644

[systemd-devel] [PATCH 1/2] test: Keep the test-suite.log around in case of a test failure

[Holger Hans Peter Freyther]

From: Holger Hans Peter Freyther 

The addition of .DELETE_ON_ERROR will lead to the removal of the
test-suite.log in case of a test failure. Mark the rule as PRECIOUS
to keep that file around.
---
 Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 7e6361c..c4b9b1a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -31,6 +31,9 @@ SUBDIRS = . po
 # keep intermediate files
 .SECONDARY:
 
+# Keep the test-suite.log
+.PRECIOUS: $(TEST_SUITE_LOG)
+
 LIBUDEV_CURRENT=4
 LIBUDEV_REVISION=6
 LIBUDEV_AGE=3
-- 
1.8.3.2

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

Re: [systemd-devel] runtime directories for services vs. tmpfiles

[Lennart Poettering]

On Wed, 17.07.13 11:07, Michael Biebl (mbi...@gmail.com) wrote:

> 
> 2013/7/16 Lennart Poettering :
> 
> > I'd be very conservative regarding adding full tmpfiles support into
> > unit files directly. Instead, I'd suggest adding two very minimal, very
> > specific new unit file settings:
> >
> > RuntimeDirectory=
> > RuntimeDirectoyMode=
> >
> > If RuntimeDirectory= is set we'd create it and chown() it to the UID/GID
> > set with User= and Group=. We'd apply the mode specified in
> > RuntimeDirectoryMode= to it.
> 
> What about daemons which drop privileges on their own? Shouldn't we
> provide a directive to set the directory owner/group?

If they drop privs on their own they *really* should also just create
the runtime dirs along with it, after all they are privileged first. And
most already do actually.

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] runtime directories for services vs. tmpfiles

[Lennart Poettering]

On Wed, 17.07.13 06:56, Andrey Borzenkov (arvidj...@gmail.com) wrote:

> > On Tue, 16.07.13 18:53, Lennart Poettering (lenn...@poettering.net) wrote:
> > 
> > I'd be very conservative regarding adding full tmpfiles support into
> > unit files directly. Instead, I'd suggest adding two very minimal, very
> > specific new unit file settings:
> > 
> > RuntimeDirectory=
> > RuntimeDirectoyMode=
> > 
> > If RuntimeDirectory= is set we'd create it and chown() it to the UID/GID
> > set with User= and Group=. We'd apply the mode specified in
> > RuntimeDirectoryMode= to it.
> > 
> > We'd even allow multiple runtime directories to be specified.
> 
> What if different directories need different modes? I'm afraid it will
> become hard to express in unit file.

All complex cases would be handled by tmpfiles as before.

This is not supposed to be a 100% solution. It would be a > 70% solution
though, if I look at the tmpfiles snippets I have installed on my local
machine.

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] Stop all uninstalled units

[Tim Cuthbertson]

Hi Folks,

I've posted a serverfault question [0], but it got no answers so I thought
I'd bring it to the mailing list:

I have some systemd units installed and running. Let's say I manually
uninstall foo.service by

 - removing its .service (and .socket) files
 - removing all symlinks (e.g from default.target.wants/)

I can run systemctl daemon-reload, and then I see:

# systemctl status foo.service
foo.service
   Loaded: error (Reason: No such file or directory)
   Active: active (running) since Mon 2013-07-08 13:50:29 EST; 48s ago
 Main PID: 1642 (node)

So systemd knows that it's not installed, and that it is running. Is there
some command I can use to stop all running services which no longer have a
unit file?

I do not want to have to somehow know what I've uninstalled, or for the
command to work only in some circumstances - I want something that I can
run at any point, with no additional knowledge, that will stop all units
not backed by a unit file.

I've currently got a hacky script that:
 - Runs `list-units` to get all unit names
 - Then `show -p ActiveState -p UnitFileState $unit` on each
 - It then runs `systemctl stop` on each unit with a UnitFileState of ""
(empty string), and ActiveState of anything but "failed".

This is almost certainly missing some edge case, as I couldn't really find
any documentation of these properties, it just seemed to be what occurred
with the examples I encountered.

I'm hoping there's a better way, can anyone point me in the right direction?

[0]:
http://serverfault.com/questions/521504/systemd-stop-all-uninstalled-units

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

[systemd-devel] [PATCHv3] systemctl, man: option to list units by state

[Maciej Wereski]

This allows to show only units with specified SUB or ACTIVE state.

Signed-off-by: Maciej Wereski 
---
 man/systemctl.xml | 15 +--
 src/systemctl/systemctl.c | 43 +--
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index f550215..2fb74c5 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -114,6 +114,16 @@ along with systemd; If not, see 
.
   
 
   
+--state=
+
+
+Argument should be a comma-separated list of unit
+SUB or ACTIVE states. When listing units show only those
+with specified SUB or ACTIVE state.
+
+  
+
+  
 -p
 --property=
 
@@ -169,8 +179,9 @@ along with systemd; If not, see 
.
 --failed
 
 
-  When listing units, show only failed units. Do not
-  confuse with --fail.
+  When listing units, show only failed units.
+  This is the same as --state=failed.
+  Do not confuse with --fail.
 
   
 
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 9574ff2..61b4730 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -93,13 +93,13 @@ static bool arg_quiet = false;
 static bool arg_full = false;
 static int arg_force = 0;
 static bool arg_ask_password = true;
-static bool arg_failed = false;
 static bool arg_runtime = false;
 static char **arg_wall = NULL;
 static const char *arg_kill_who = NULL;
 static int arg_signal = SIGTERM;
 static const char *arg_root = NULL;
 static usec_t arg_when = 0;
+static char **arg_state = NULL;
 static enum action {
 ACTION_INVALID,
 ACTION_SYSTEMCTL,
@@ -301,8 +301,8 @@ static int compare_unit_info(const void *a, const void *b) {
 static bool output_show_unit(const struct unit_info *u) {
 const char *dot;
 
-if (arg_failed)
-return streq(u->active_state, "failed");
+if (!strv_isempty(arg_state))
+return strv_contains(arg_state, u->sub_state) || 
strv_contains(arg_state, u->active_state);
 
 return (!arg_types || ((dot = strrchr(u->id, '.')) &&
strv_find(arg_types, dot+1))) &&
@@ -4660,12 +4660,13 @@ static int systemctl_help(void) {
"  -h --help   Show this help\n"
" --versionShow package version\n"
"  -t --type=TYPE  List only units of a particular type\n"
+   " --state=STATEShow only units with particular SUB or 
ACTIVE state\n"
"  -p --property=NAME  Show only properties by this name\n"
"  -a --allShow all loaded units/properties, 
including dead/empty\n"
"  ones. To list all units installed on the 
system, use\n"
"  the 'list-unit-files' command instead.\n"
" --reverseShow reverse dependencies with 
'list-dependencies'\n"
-   " --failed Show only failed units\n"
+   " --failed Show only failed units (the same as 
--state=failed)\n"
"  -l --full   Don't ellipsize unit names on output\n"
" --fail   When queueing a new job, fail if 
conflicting jobs are\n"
"  pending\n"
@@ -4886,7 +4887,8 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
 ARG_FAILED,
 ARG_RUNTIME,
 ARG_FORCE,
-ARG_PLAIN
+ARG_PLAIN,
+ARG_STATE
 };
 
 static const struct option options[] = {
@@ -4925,6 +4927,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
 { "lines", required_argument, NULL, 'n'   },
 { "output",required_argument, NULL, 'o'   },
 { "plain", no_argument,   NULL, ARG_PLAIN },
+{ "state", required_argument, NULL, ARG_STATE },
 { NULL,0, NULL, 0 }
 };
 
@@ -5086,7 +5089,14 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
 break;
 
 case ARG_FAILED:
-arg_failed = true;
+if (!strv_contains(arg_state, "failed")) {
+char **tmp = arg_state;
+arg_state = strv_append(arg_state, "failed");
+if (!arg_state) {
+strv_free(tmp);
+return -ENOMEM;
+}
+}
 break;
 
 case 'q':
@@ -5156,6 +5166,27 @@ st

Re: [systemd-devel] hwdb for gphoto2

[Kay Sievers]

On Wed, Jul 17, 2013 at 11:31 AM, Tom Gundersen  wrote:
> Hi guys,
>
> Here is an attempt at converting libgphoto2 to hwdb. Seems to work for my 
> phone.
>
> hwdb file: 
> patch: 
>
> Comments welcome.

Looks both great, the sane and gphoto stuff.

Should be a huge improvement over the insanely big udev rules hack.
Thanks a lot for doing this!

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

[systemd-devel] hwdb for gphoto2

[Tom Gundersen]

Hi guys,

Here is an attempt at converting libgphoto2 to hwdb. Seems to work for my phone.

hwdb file: 
patch: 

Comments welcome.

Cheers,

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

Re: [systemd-devel] hwdb for sane

[Tom Gundersen]

On Wed, Jul 17, 2013 at 9:36 AM, Tom Gundersen  wrote:
> On Wed, Jul 17, 2013 at 1:57 AM, Tom Gundersen  wrote:
>> Hi guys,
>>
>> I had a stab at converting sane to using hwdb rather than a huge udev
>> rules file.
>>
>> For now only the usb entries have been converted, I didn't look at how
>> to deal with scsi.
>>
>> hwdb file: 
>
> That should obviously have been:
>
>> hwdb file: 
>
>> udev rules: 
>> patch1: 
>> 
>> patch2: 
>> 
>>
>> Any comments welcome (especially as I don't have the hardware to do
>> any testing...).

Patch and hwdb file updated to get the right case.

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

Re: [systemd-devel] runtime directories for services vs. tmpfiles

[Michael Biebl]

2013/7/17 Michael Biebl :
> If this scheme is not flexible enough to cover the vast majority of
> all cases (for services), then I fear we'd end up half of the services
> using RuntimeDirectory, the other half a tmpfile. And that imho would
> be even more confusing.

Could we have some stats from Fedora and/or Arch which have done a
(complete) migration to systemd, about how many services/packages
currently use a tmpfile and what they use in the tmpfile, i.e. if the
proposed scheme from Lennart would suffice say for 90+ % of them

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] runtime directories for services vs. tmpfiles

[Michael Biebl]

2013/7/17 Harald Hoyer :
> If RuntimeDirectory would support variable substitutions, this feature could 
> not
> be provided with systemd-tmpfiles.

That would indeed be a nice feature.


--
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] runtime directories for services vs. tmpfiles

[Michael Biebl]

2013/7/16 Lennart Poettering :

> I'd be very conservative regarding adding full tmpfiles support into
> unit files directly. Instead, I'd suggest adding two very minimal, very
> specific new unit file settings:
>
> RuntimeDirectory=
> RuntimeDirectoyMode=
>
> If RuntimeDirectory= is set we'd create it and chown() it to the UID/GID
> set with User= and Group=. We'd apply the mode specified in
> RuntimeDirectoryMode= to it.

What about daemons which drop privileges on their own? Shouldn't we
provide a directive to set the directory owner/group?
If this scheme is not flexible enough to cover the vast majority of
all cases (for services), then I fear we'd end up half of the services
using RuntimeDirectory, the other half a tmpfile. And that imho would
be even more confusing.

--
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] runtime directories for services vs. tmpfiles

[Harald Hoyer]

On 07/16/2013 09:28 PM, "Jóhann B. Guðmundsson" wrote:
> On 07/16/2013 06:26 PM, Lennart Poettering wrote:
>> I discussed this a bit more with Kay on the phone. Here's what we'd propose:
>>
>> I'd be very conservative regarding adding full tmpfiles support into
>> unit files directly. Instead, I'd suggest adding two very minimal, very
>> specific new unit file settings:
>>
>> RuntimeDirectory=
>> RuntimeDirectoyMode=
> 
> What exactly are we trying to solve here which should not be fixed upstream in
> the daemons themselves?
> 

If RuntimeDirectory would support variable substitutions, this feature could not
be provided with systemd-tmpfiles.

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

[systemd-devel] [PATCHv2] core: notify triggered by socket of a service

[Umut Tezduyar]

Refer to:
http://lists.freedesktop.org/archives/systemd-devel/2013-June/011532.html
---
 TODO   |3 ---
 src/core/service.c |   31 ---
 src/core/socket.c  |   33 -
 src/core/socket.h  |3 ---
 4 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/TODO b/TODO
index ac5ae71..4d3c1c2 100644
--- a/TODO
+++ b/TODO
@@ -118,9 +118,6 @@ Features:
   maybe we should stop doing auto-activation of this after boot
   entirely. https://bugzilla.gnome.org/show_bug.cgi?id=701676
 
-* when a service changes state make reflect that in the
-  RUNNING/LISTENING states of its socket
-
 * when recursively showing the cgroup hierarchy, optionally also show
   the hierarchies of child processes
 
diff --git a/src/core/service.c b/src/core/service.c
index 2bc0dc5..ebc21d4 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1481,24 +1481,6 @@ static int service_search_main_pid(Service *s) {
 return 0;
 }
 
-static void service_notify_sockets_dead(Service *s, bool failed_permanent) {
-Iterator i;
-Unit *u;
-
-assert(s);
-
-/* Notifies all our sockets when we die */
-
-if (s->socket_fd >= 0)
-return;
-
-SET_FOREACH(u, UNIT(s)->dependencies[UNIT_TRIGGERED_BY], i)
-if (u->type == UNIT_SOCKET)
-socket_notify_service_dead(SOCKET(u), 
failed_permanent);
-
-return;
-}
-
 static void service_set_state(Service *s, ServiceState state) {
 ServiceState old_state;
 const UnitActiveState *table;
@@ -1550,19 +1532,6 @@ static void service_set_state(Service *s, ServiceState 
state) {
 s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID;
 }
 
-if (state == SERVICE_FAILED)
-service_notify_sockets_dead(s, s->result == 
SERVICE_FAILURE_START_LIMIT);
-
-if (state == SERVICE_DEAD ||
-state == SERVICE_STOP ||
-state == SERVICE_STOP_SIGTERM ||
-state == SERVICE_STOP_SIGKILL ||
-state == SERVICE_STOP_POST ||
-state == SERVICE_FINAL_SIGTERM ||
-state == SERVICE_FINAL_SIGKILL ||
-state == SERVICE_AUTO_RESTART)
-service_notify_sockets_dead(s, false);
-
 if (state != SERVICE_START_PRE &&
 state != SERVICE_START &&
 state != SERVICE_START_POST &&
diff --git a/src/core/socket.c b/src/core/socket.c
index cf88bae..c45b293 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2277,7 +2277,7 @@ int socket_collect_fds(Socket *s, int **fds, unsigned 
*n_fds) {
 return 0;
 }
 
-void socket_notify_service_dead(Socket *s, bool failed_permanent) {
+static void socket_notify_service_dead(Socket *s, bool failed_permanent) {
 assert(s);
 
 /* The service is dead. Dang!
@@ -2322,6 +2322,35 @@ static void socket_reset_failed(Unit *u) {
 s->result = SOCKET_SUCCESS;
 }
 
+static void socket_trigger_notify(Unit *u, Unit *other) {
+Socket *s = SOCKET(u);
+Service *se = SERVICE(other);
+
+assert(u);
+assert(other);
+
+if (other->load_state != UNIT_LOADED ||
+other->type != UNIT_SERVICE)
+return;
+
+if (se->state == SERVICE_FAILED && se->socket_fd < 0)
+socket_notify_service_dead(s, se->result == 
SERVICE_FAILURE_START_LIMIT);
+
+if ((se->state == SERVICE_DEAD ||
+se->state == SERVICE_STOP ||
+se->state == SERVICE_STOP_SIGTERM ||
+se->state == SERVICE_STOP_SIGKILL ||
+se->state == SERVICE_STOP_POST ||
+se->state == SERVICE_FINAL_SIGTERM ||
+se->state == SERVICE_FINAL_SIGKILL ||
+se->state == SERVICE_AUTO_RESTART) &&
+se->socket_fd < 0)
+socket_notify_service_dead(s, false);
+
+if (!s->accept && se->state == SERVICE_RUNNING)
+socket_set_state(s, SOCKET_RUNNING);
+}
+
 static int socket_kill(Unit *u, KillWho who, int signo, DBusError *error) {
 return unit_kill_common(u, who, signo, -1, SOCKET(u)->control_pid, 
error);
 }
@@ -2402,6 +2431,8 @@ const UnitVTable socket_vtable = {
 .sigchld_event = socket_sigchld_event,
 .timer_event = socket_timer_event,
 
+.trigger_notify = socket_trigger_notify,
+
 .reset_failed = socket_reset_failed,
 
 .bus_interface = "org.freedesktop.systemd1.Socket",
diff --git a/src/core/socket.h b/src/core/socket.h
index 8f9dfdb..5733322 100644
--- a/src/core/socket.h
+++ b/src/core/socket.h
@@ -156,9 +156,6 @@ struct Socket {
 /* Called from the service code when collecting fds */
 int socket_collect_fds(Socket *s, int **fds, unsigned *n_fds);
 
-/* Called from the service when it shut down */
-void socket_notify_service_dead(Socket *s, bool failed_permanent);
-
 /* Called from the mount code figure out if a mount is a dependency

Re: [systemd-devel] hwdb for sane

[Tom Gundersen]

On Wed, Jul 17, 2013 at 1:57 AM, Tom Gundersen  wrote:
> Hi guys,
>
> I had a stab at converting sane to using hwdb rather than a huge udev
> rules file.
>
> For now only the usb entries have been converted, I didn't look at how
> to deal with scsi.
>
> hwdb file: 

That should obviously have been:

> hwdb file: 

> udev rules: 
> patch1: 
> 
> patch2: 
> 
>
> Any comments welcome (especially as I don't have the hardware to do
> any testing...).
>
> Cheers,
>
> Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] service: don't try to kill the service more than once when the watchdog timeout hits

[Michael Olbrich]

Hi,

On Wed, Jul 17, 2013 at 03:53:09AM +0200, Lennart Poettering wrote:
> On Wed, 12.06.13 01:22, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> > If ExecStopPost= is defined then it is executed after SIGKILL. Otherwise
> > another round of SIGTERM/SIGSTOP is started which is rather useless when
> > the watchdog timeout hits.
> > So go directly to the final SIGKILL if ExecStopPost= is not defined.
> 
> Hmm, why not go always directly into SERVICE_FINAL_SIGKILL? Why bother
> with SERVICE_STOP_SIGKILL at all? What am I missing?

I think a small summery is necessary here:
The original watchdog code just called service_enter_dead(). This had the
problem that no cleanup happened. The process was not killed.
Then I posted a patch to go irectly into SERVICE_FINAL_SIGKILL to fix that.
It got applied.
Then you changed it to SERVICE_STOP_SIGKILL so that ExecStopPost= is still
called.
However, this has a annoying side effect. Consider a service that enters
service_handle_watchdog() with a process that cannot be killed. What
happens is this:

service_handle_watchdog()
STOP_SIGKILL
wait
ExecStopPost= if available
FINAL_SIGTERM
wait
FINAL_SIGKILL
wait
service_enter_dead()

Without a ExecStopPost= defined anything after the first wait is just more
useless waiting.
I wrote this patch to avoid waiting longer than necessary but still be able
to run ExecStopPost=.
I later realized that this problem is not limited to the watchdog usecase.
So I wrote a second patch (it should be applied instead of this one) that
changes the sequence to:

[...]
STOP_SIGKILL
wait
if (ExecStopPost= is available) {
ExecStopPost=
FINAL_SIGTERM
wait
FINAL_SIGKILL
wait
}
service_enter_dead()

I sent it as a reply to the first patch. I've also attached it for
reference.

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- |
>From 86efc9efe5a365de219457d033844cb1022fdacb Mon Sep 17 00:00:00 2001
From: Michael Olbrich 
Date: Wed, 12 Jun 2013 08:41:16 +0200
Subject: [PATCH] service: don't enter a second SIGTERM/SIGKILL cycle if no
 ExecStopPost= process is defined

It won't help if the main process is still there and there is no new
process to kill.

Signed-off-by: Michael Olbrich 
---
 src/core/service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index 2bc0dc5..b98f11a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1987,7 +1987,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) {
 
 service_set_state(s, SERVICE_STOP_POST);
 } else
-service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_SUCCESS);
+service_enter_dead(s, SERVICE_SUCCESS, true);
 
 return;
 
-- 
1.8.3.2

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

Re: [systemd-devel] [PATCH] travis: Add a travis.yml for doing CI after commits

[Holger Hans Peter Freyther]

On Fri, Jul 12, 2013 at 07:51:26PM +0200, Lennart Poettering wrote:

> Commited. I figure this will not be useful right-away, since current git
> of systemd requires a very new version of kmod that is not available at
> Travis. However, it's still a good thing to have and to enable as soon
> as that issue is solved.

I could only check this today. The ubuntu/travis build is currently not
using kmod at all so it could be useful right-away. What is missing is
that any member of the systemd group on github visits the travis-ci.org
website, clicks on 'Sign in with GitHub', go to the account section and
enables travis-ci for the systemd repository. It should be < 1 minute of
work.

cheers
holger

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