[systemd-devel] [Fwd: [PATCH] journal: fix dereferenced pointer in journal_file_rotate()]

[Sjoerd Simons]

On Tue, 2012-05-22 at 13:26 +0200, Lennart Poettering wrote:
> On Tue, 22.05.12 08:40, Sjoerd Simons (sjoerd at luon.net) wrote:
> 
> > 
> > On Mon, 2012-05-21 at 21:35 -0700, shawn wrote:
> > > > If journal_file_open() failed, due to (e.g.) -ENOSPC on open()
> > > > new_file might still be NULL.
> > > > 
> > > > On error, leave pointer to the old JournalFile (now closed),
> > > > and require caller to check for error approiately.
> > > > 
> > > > 	Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43020
> > > > 	Reported-by: Sjoerd Simons <sjoerd at luon.net>
> > 
> > The bugzilla link seems wrong ? 
> > 
> > This actually remind me though, i did submit a patch for this issue to
> > bugzilla (slightly different then your solution) more then a month ago.
> > And a companion patch to not make the issue occur so easily, bugs filed
> > here:
> > 
> > https://bugs.freedesktop.org/show_bug.cgi?id=48688
> > https://bugs.freedesktop.org/show_bug.cgi?id=48685
> > 
> > If the systemd bugzilla is just somewhat of a decoy i'm happy to repost
> > the patches to the list ofcourse :)
> 
> Nah, fdo bz is not a decoy. The reason I didnt merge this right away was
> actually that I wanted to rework the code in question in a bigger way,
> so that we have some logic in there that we automatically fallback to
> kmsg logging when the journal for some reason doesn't work. But I never
> found the time to.

Aha! A note on the bug would have been great :).

> Anyway, since this is a bug I have now merged your patch 48685, and we
> can add the kmsg fallback logic later on. Thanks for your work!

Np, thanks for merging :).

> About 48688 I am not sure sure. i.e. should we really bind the keep_free
> stuff to the reserved percentage of the FS? They are two different
> things, or are they not?

I'm not sure either as i mentioned in the bugreport. The standard 5% is
a bit odd though. Although I may be a bit odd, but my / partition tends
to never have a lot of free space, which means it's basically always
below the 5% (Which is still more then 0.5G on a 10G system..). 

I picked up using the reserved precentage mostly as it's the one place i
could think of where there is currently a configuration for "leave this
much space free please". 

Furthermore it means that you get a no space error only when df shows
you have 0 available space which is nice i'd think :).. It took me quite
some time to figure out why journald was giving out of space errors
while df was happily showing there was still quite a bit of space left.

-- 
Sjoerd Simons <sjoerd at luon.net>