Page MenuHomeFreeBSD

savecore: uncompress cores
ClosedPublic

Authored by glebius on Mon, Nov 16, 10:22 PM.

Details

Summary

A real life scenario is that cores are compressed to reduce
size of dumpon partition, but we either don't care about space
in the /var/crash or we have a filesystem level compression of
/var/crash. And we want cores to be uncompressed in /var/crash
because we'd like to instantily read them with kgdb.

In this case we want kernel to write cores compressed, but
savecore(1) write them uncompressed.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This revision is now accepted and ready to land.Mon, Nov 16, 10:53 PM
bcr added a subscriber: bcr.

OK from manpages.

Shouldn't this also include a change to default savecore_flags in /etc/defaults/rc.conf?

This patch doesn't support ZSTD yet, since the library isn't yet installed in the base system and I'm reluctant to statically compile it into savecore(1).

It's there, it's just a private lib: /usr/lib/libprivatezstd.so.5

sbin/savecore/savecore.c
750 ↗(On Diff #79630)

Why not set uncompress = false here too?

Add ZSTD support and address comments.

This revision now requires review to proceed.Tue, Nov 17, 9:02 PM

Shouldn't this also include a change to default savecore_flags in /etc/defaults/rc.conf?

I don't want to change the default behavior.

Looks fine to except some nits.

sbin/savecore/savecore.c
594 ↗(On Diff #79684)

The compression == KERNELDUMP_COMP_* cases should be lifted into their own subroutines IMO.

1024 ↗(On Diff #79684)

It is easier to understand this when written as (iscompressed && !uncompress) || compress).

Separate decompression loops into their own subroutines. Reword
logical statement.

Some extra cruft slept in previous update.

glebius marked an inline comment as done.
markj added inline comments.
sbin/savecore/savecore.c
569 ↗(On Diff #79734)

sizeof(z)

585 ↗(On Diff #79734)

Static analyzers will complain that this buffer is leaked. I know it's not a real problem, but it's also easy to fix.

586 ↗(On Diff #79734)

There's a missing newline here.

612 ↗(On Diff #79734)

I think compression != KERNELDUMP_COMP_NONE is preferable.

This revision is now accepted and ready to land.Thu, Nov 19, 2:13 AM
glebius added inline comments.
sbin/savecore/savecore.c
585 ↗(On Diff #79734)

Yes. But function has several returns with different allocation conditions. Make it "right" everywhere would make it really hairy.

sbin/savecore/savecore.c
585 ↗(On Diff #79734)

You would have to add three free(zbuf) calls, relying on the fact that zbuf is initialized to NULL and that free(NULL) is valid. Not really that hairy. Someone else is inevitably going to come along and fix it anyway, for better or worse, so IMO it's better to just do it now.

sbin/savecore/savecore.c
585 ↗(On Diff #79734)

To do it properly the zstd context also needs to be freed, and this is where it gets hairy.