Re: [systemd-devel] [PATCH v5] journalctl: Add support for showing messages from a previous boot
On Thu, Jul 18, 2013 at 01:05:25PM +0200, Jan Janssen wrote: > On 07/18/2013 06:10 AM, Zbigniew Jędrzejewski-Szmek wrote: > >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 > > > > I really don't like arguments to options that can start with "-", it > can easily be confused with another option. Especially if one were ever > to introduce options like "-0" to "-9". Also, not accepting long UUIDs > is kind of restricting the user. But ultimately, this is > bike-shedding... > > But more importantly, you've introduced a bug: > > $ ./journalctl -b a709bdcbaa1b422f8338a25fd2d4d61d > Relative boot ID offset must start with a '+' or a '-', found '' Arrgh. > Also, for the challenged people (me), does this really guard the > array access (count >= INT_MAX comes to my mind)? And if so, how? > > if (relative > (int) count || relative <= -(int)count) count comes from searching throught the database, so cannot really get too large without a really long wait. Nevertheless, if it was >= INT_MAX, than after casting to (int) it would become negative, and the left side of the if should be satisfied. > If you could silence this warning, it would be nice :P > > src/journal/journalctl.c: In function ‘get_relative_boot_id’: > src/journal/journalctl.c:747:63: warning: comparison between signed > and unsigned integer expressions [-Wsign-compare] > (id - all_ids) + relative >= count) Interesting that my gcc (4.7.2) didn't show that. > Anyway, gonna go sulk now for not having come up with such nice code > in the first place :( The pull towards bikeshedding and µ-opts was too strong for me to resist :) But I seriously think that the arguments with a dash will turn out OK in practice. 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 v5] journalctl: Add support for showing messages from a previous boot
On 07/18/2013 06:10 AM, Zbigniew Jędrzejewski-Szmek wrote: 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 I really don't like arguments to options that can start with "-", it can easily be confused with another option. Especially if one were ever to introduce options like "-0" to "-9". Also, not accepting long UUIDs is kind of restricting the user. But ultimately, this is bike-shedding... But more importantly, you've introduced a bug: $ ./journalctl -b a709bdcbaa1b422f8338a25fd2d4d61d Relative boot ID offset must start with a '+' or a '-', found '' Also, for the challenged people (me), does this really guard the array access (count >= INT_MAX comes to my mind)? And if so, how? if (relative > (int) count || relative <= -(int)count) If you could silence this warning, it would be nice :P src/journal/journalctl.c: In function ‘get_relative_boot_id’: src/journal/journalctl.c:747:63: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] (id - all_ids) + relative >= count) Anyway, gonna go sulk now for not having come up with such nice code in the first place :( Jan ___ 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
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 v5] journalctl: Add support for showing messages from a previous boot
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! 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 v5] journalctl: Add support for showing messages from a previous boot
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. I'd like to complain about the : in the syntax though. 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 v5] journalctl: Add support for showing messages from a previous boot
On Fri, 28.06.13 17:26, Jan Janssen (medhe...@web.de) wrote: > Unfortunately, order of sd_journal_enumerate_unique() is > undefined, so we have to sort the boot IDs. For that we > seach for any log entry with a specific boot ID and then > use the realtime stamp to order everything. > --- > > Hi, > > I redid the boot ID look up to use enumerate_unique. > > This is quite fast if the cache is warm but painfully slow if > it isn't. It has a slight chance of returning the wrong order if > realtime clock jumps around. > > This one has to do n searches for every boot ID there is plus > a sort, so it depends heavily on cache hotness. This is in contrast > to the other way of look-up through filtering by a MESSAGE_ID, > which only needs about 1 seek + whatever amount of relative IDs > you want to walk. > > I also have a linked-list + (in-place) mergesort version of this > patch, which has pretty much the same runtime. But since this one > is using libc sorting and armortized allocation, I prefer this > one. > > To summarize: The MESSAGE_ID way is a *lot* faster but can be > incomplete due to rotation, while the enumerate+sort will find > every boot ID out there but will be painfully slow for large > journals and cold caches. > > You choose :P Applied this one now. If people start complaining about its speed we can reinvestigate and do find some way for optimization... Thanks, 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] [PATCH v5] journalctl: Add support for showing messages from a previous boot
Unfortunately, order of sd_journal_enumerate_unique() is undefined, so we have to sort the boot IDs. For that we seach for any log entry with a specific boot ID and then use the realtime stamp to order everything. --- Hi, I redid the boot ID look up to use enumerate_unique. This is quite fast if the cache is warm but painfully slow if it isn't. It has a slight chance of returning the wrong order if realtime clock jumps around. This one has to do n searches for every boot ID there is plus a sort, so it depends heavily on cache hotness. This is in contrast to the other way of look-up through filtering by a MESSAGE_ID, which only needs about 1 seek + whatever amount of relative IDs you want to walk. I also have a linked-list + (in-place) mergesort version of this patch, which has pretty much the same runtime. But since this one is using libc sorting and armortized allocation, I prefer this one. To summarize: The MESSAGE_ID way is a *lot* faster but can be incomplete due to rotation, while the enumerate+sort will find every boot ID out there but will be painfully slow for large journals and cold caches. You choose :P Jan TODO | 1 - man/journalctl.xml | 54 --- shell-completion/bash/journalctl | 11 ++- src/journal/journalctl.c | 196 --- 4 files changed, 235 insertions(+), 27 deletions(-) diff --git a/TODO b/TODO index 055d973..dd0f4e8 100644 --- a/TODO +++ b/TODO @@ -318,7 +318,6 @@ Features: - journal-send.c, log.c: when the log socket is clogged, and we drop, count this and write a message about this when it gets unclogged again. - journal: find a way to allow dropping history early, based on priority, other rules - journal: When used on NFS, check payload hashes - - Introduce journalctl -b to show journal messages of a previous boot - journald: check whether it is OK if the client can still modify delivered journal entries - journal live copy, based on libneon (client) and libmicrohttpd (server) - journald: add kernel cmdline option to disable ratelimiting for debug purposes diff --git a/man/journalctl.xml b/man/journalctl.xml index fa29c41..83d8878 100644 --- a/man/journalctl.xml +++ b/man/journalctl.xml @@ -313,23 +313,51 @@ --b ---this-boot - -Show data only from -current boot. This will add a match -for _BOOT_ID= for -the current boot ID of the -kernel. +-b ID + --boot=ID + +Show messages from the specified +boot ID or from +current boot if no ID +is given. This will add a match for +_BOOT_ID=. + +The argument is a 128 bit ID given in +short or UUID form and optionally followed by +:n which identifies the nth +boot relative to the boot ID given to the left +of :. Supplying a negative +value for n will look for a past boot and a +positive value for a future boot. The boot IDs +are searched for in chronological order. If no +number is provided after :, +-1 is assumed. A value of 0 +is valid and equivalent to omitting +:0. + +Alternatively, the argument may constist +only of :n. In this case, a +positive value will look up the nth boot +starting from the beginning of the jouranl, a +negative value will look up a previous boot +relative to the current boot. :0 +will look for the current boot ID. Thus, +:1 is the first boot found in +the journal, :2 the second +and so on; while :-1 is the +previous boot, :-2 the boot +before that and so on. Omitting a value after +: will look for the previous +boot. -k --dmesg -
