[systemd-devel] [PATCH V2] journalctl: fix logic with parameter "-u"

harald at redhat.com harald at redhat.com
Fri Mar 22 07:51:00 PDT 2013


From: Harald Hoyer <harald at redhat.com>

When using "-p" and "-b" in combination with "-u", the output is not
what you would expect. The reason is the sd_journal_add_disjunction()
call in add_matches_for_unit() and add_matches_for_user_unit(), which
adds two ORs without taking the other conditions to every OR.

Output before:

$ journalctl -o short-monotonic -ab -p 0 -u sshd.service + _SYSTEMD_UNIT=crond.service

-- Reboot --
[    3.216305] lenovo systemd[1]: Starting OpenSSH server daemon...
-- Reboot --
[    3.168666] lenovo systemd[1]: Starting OpenSSH server daemon...
[    3.169639] lenovo systemd[1]: Started OpenSSH server daemon.
[36285.635389] lenovo systemd[1]: Stopped OpenSSH server daemon.
-- Reboot --
[   10.838657] lenovo systemd[1]: Starting OpenSSH server daemon...
[   10.913698] lenovo systemd[1]: Started OpenSSH server daemon.
[ 6881.035183] lenovo systemd[1]: Stopped OpenSSH server daemon.
-- Reboot --
[    6.636228] lenovo systemd[1]: Starting OpenSSH server daemon...
[    6.662573] lenovo systemd[1]: Started OpenSSH server daemon.
[    6.681148] lenovo sshd[397]: Server listening on 0.0.0.0 port 22.
[    6.681379] lenovo sshd[397]: Server listening on :: port 22.

As we see, the output is from _every_ boot and priority 0 is not taken
into account.

Output after patch:

$ journalctl -o short-monotonic -ab -p 0 -u sshd.service + _SYSTEMD_UNIT=crond.service
-- Logs begin at Sun 2013-02-24 20:54:44 CET, end at Tue 2013-03-19 14:58:21 CET. --

Increasing the priority:

$ journalctl -o short-monotonic -ab -p 6 -u sshd.service + _SYSTEMD_UNIT=crond.service
-- Logs begin at Sun 2013-02-24 20:54:44 CET, end at Tue 2013-03-19 14:59:12 CET. --
[    6.636228] lenovo systemd[1]: Starting OpenSSH server daemon...
[    6.662573] lenovo systemd[1]: Started OpenSSH server daemon.
[    6.681148] lenovo sshd[397]: Server listening on 0.0.0.0 port 22.
[    6.681379] lenovo sshd[397]: Server listening on :: port 22.
[    7.191076] lenovo crond[416]: (CRON) INFO (Syslog will be used
		instead of sendmail.)
[    7.191302] lenovo crond[416]: (CRON) INFO (running with inotify
		support)
---
 src/journal/journalctl.c | 153 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 126 insertions(+), 27 deletions(-)

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index ddadc21..e95253c 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -519,16 +519,26 @@ static int generate_new_id128(void) {
         return 0;
 }
 
-static int add_matches(sd_journal *j, char **args) {
-        char **i;
+static int add_matches(sd_journal *j, char **prematch, char **args) {
+        char **i, **k;
+        int r;
 
         assert(j);
 
-        STRV_FOREACH(i, args) {
-                int r;
+        STRV_FOREACH(k, prematch) {
+                if ((r = sd_journal_add_match(j, *k, 0)))
+                        return r;
+        }
 
-                if (streq(*i, "+"))
+        STRV_FOREACH(i, args) {
+                if (streq(*i, "+")) {
                         r = sd_journal_add_disjunction(j);
+
+                        STRV_FOREACH(k, prematch) {
+                                if ((r = sd_journal_add_match(j, *k, 0)))
+                                        return r;
+                        }
+                }
                 else if (path_is_absolute(*i)) {
                         char _cleanup_free_ *p, *t = NULL;
                         const char *path;
@@ -569,13 +579,11 @@ static int add_matches(sd_journal *j, char **args) {
         return 0;
 }
 
-static int add_this_boot(sd_journal *j) {
+static int add_this_boot(char ***prematch) {
         char match[9+32+1] = "_BOOT_ID=";
         sd_id128_t boot_id;
         int r;
 
-        assert(j);
-
         if (!arg_this_boot)
                 return 0;
 
@@ -586,16 +594,110 @@ static int add_this_boot(sd_journal *j) {
         }
 
         sd_id128_to_string(boot_id, match + 9);
-        r = sd_journal_add_match(j, match, strlen(match));
-        if (r < 0) {
-                log_error("Failed to add match: %s", strerror(-r));
+
+
+        r = strv_extend(prematch, match);
+        if (r)
+                return log_oom();
+
+        return 0;
+}
+
+static int add_matches_for_unit_prematch(sd_journal *j, char **prematch, const char *unit) {
+        int r;
+        _cleanup_free_ char *m1 = NULL, *m2 = NULL, *m3 = NULL;
+        char **i;
+
+        assert(j);
+        assert(unit);
+
+        if (asprintf(&m1, "_SYSTEMD_UNIT=%s", unit) < 0 ||
+            asprintf(&m2, "COREDUMP_UNIT=%s", unit) < 0 ||
+            asprintf(&m3, "UNIT=%s", unit) < 0)
+                return -ENOMEM;
+
+        STRV_FOREACH(i, prematch) {
+                if ((r = sd_journal_add_match(j, *i, 0)))
+                        return r;
+        }
+        /* Look for messages from the service itself */
+        if ((r = sd_journal_add_match(j, m1, 0)) ||
+            /* Look for coredumps of the service */
+            (r = sd_journal_add_disjunction(j)))
                 return r;
+
+        STRV_FOREACH(i, prematch) {
+                if ((r = sd_journal_add_match(j, *i, 0)))
+                        return r;
         }
 
-        return 0;
+        if ((r = sd_journal_add_match(j, "MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1", 0)) ||
+            (r = sd_journal_add_match(j, m2, 0)) ||
+            /* Look for messages from PID 1 about this service */
+            (r = sd_journal_add_disjunction(j)))
+                return r;
+
+        STRV_FOREACH(i, prematch) {
+                if ((r = sd_journal_add_match(j, *i, 0)))
+                        return r;
+        }
+
+        if((r = sd_journal_add_match(j, "_PID=1", 0)) ||
+           (r = sd_journal_add_match(j, m3, 0)))
+                return r;
+
+        return r;
 }
 
-static int add_unit(sd_journal *j) {
+static int add_matches_for_user_unit_prematch(sd_journal *j, char **prematch, const char *unit, uid_t uid) {
+        int r;
+        _cleanup_free_ char *m1 = NULL, *m2 = NULL, *m3 = NULL, *m4 = NULL;
+        char **i;
+
+        assert(j);
+        assert(unit);
+
+        if (asprintf(&m1, "_SYSTEMD_USER_UNIT=%s", unit) < 0 ||
+            asprintf(&m2, "USER_UNIT=%s", unit) < 0 ||
+            asprintf(&m3, "COREDUMP_USER_UNIT=%s", unit) < 0 ||
+            asprintf(&m4, "_UID=%d", uid) < 0)
+                return -ENOMEM;
+
+        /* Look for messages from the user service itself */
+        STRV_FOREACH(i, prematch) {
+                if ((r = sd_journal_add_match(j, *i, 0)))
+                        return r;
+        }
+        if ((r = sd_journal_add_match(j, m1, 0)) ||
+            (r = sd_journal_add_match(j, m4, 0)) ||
+            /* Look for messages from systemd about this service */
+            (r = sd_journal_add_disjunction(j)))
+                return r;
+
+        STRV_FOREACH(i, prematch) {
+                if ((r = sd_journal_add_match(j, *i, 0)))
+                        return r;
+        }
+
+        if ((r = sd_journal_add_match(j, m2, 0)) ||
+            (r = sd_journal_add_match(j, m4, 0)) ||
+            /* Look for coredumps of the service */
+            (r = sd_journal_add_disjunction(j)))
+                return r;
+
+        STRV_FOREACH(i, prematch) {
+                if ((r = sd_journal_add_match(j, *i, 0)))
+                        return r;
+        }
+
+        if ((r = sd_journal_add_match(j, m3, 0)) ||
+            (r = sd_journal_add_match(j, m4, 0)))
+                return r;
+
+        return r;
+}
+
+static int add_unit(sd_journal *j, char **prematch) {
         _cleanup_free_ char *u = NULL;
         int r;
 
@@ -609,21 +711,19 @@ static int add_unit(sd_journal *j) {
                 return log_oom();
 
         if (arg_unit_system)
-                r = add_matches_for_unit(j, u);
+                r = add_matches_for_unit_prematch(j, prematch, u);
         else
-                r = add_matches_for_user_unit(j, u, getuid());
+                r = add_matches_for_user_unit_prematch(j, prematch, u, getuid());
         if (r < 0)
                 return r;
 
         return 0;
 }
 
-static int add_priorities(sd_journal *j) {
+static int add_priorities(char ***prematch) {
         char match[] = "PRIORITY=0";
         int i, r;
 
-        assert(j);
-
         if (arg_priorities == 0xFF)
                 return 0;
 
@@ -631,11 +731,9 @@ static int add_priorities(sd_journal *j) {
                 if (arg_priorities & (1 << i)) {
                         match[sizeof(match)-2] = '0' + i;
 
-                        r = sd_journal_add_match(j, match, strlen(match));
-                        if (r < 0) {
-                                log_error("Failed to add match: %s", strerror(-r));
-                                return r;
-                        }
+                        r = strv_extend(prematch, match);
+                        if (r)
+                                return log_oom();
                 }
 
         return 0;
@@ -905,6 +1003,7 @@ int main(int argc, char *argv[]) {
         sd_id128_t previous_boot_id;
         bool previous_boot_id_valid = false, first_line = true;
         int n_shown = 0;
+        char _cleanup_strv_free_ **arg_pre_match = NULL;
 
         setlocale(LC_ALL, "");
         log_parse_environment();
@@ -979,19 +1078,19 @@ int main(int argc, char *argv[]) {
                 return EXIT_SUCCESS;
         }
 
-        r = add_this_boot(j);
+        r = add_this_boot(&arg_pre_match);
         if (r < 0)
                 return EXIT_FAILURE;
 
-        r = add_unit(j);
+        r = add_priorities(&arg_pre_match);
         if (r < 0)
                 return EXIT_FAILURE;
 
-        r = add_matches(j, argv + optind);
+        r = add_unit(j, arg_pre_match);
         if (r < 0)
                 return EXIT_FAILURE;
 
-        r = add_priorities(j);
+        r = add_matches(j, arg_pre_match, argv + optind);
         if (r < 0)
                 return EXIT_FAILURE;
 
-- 
1.8.1



More information about the systemd-devel mailing list