Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

stage1: break down overlong property lines #3279

Merged
merged 1 commit into from
Oct 13, 2016

Conversation

lucab
Copy link
Member

@lucab lucab commented Oct 12, 2016

This commit workarounds a systemd bug, which causes unit file
to be incorrectly parsed.
This is triggered by property lines longer than LINE_MAX (2048),
which frequently occur when passing a large set of values (eg. seccomp
and paths).
In this case, we will now split such values across multiple lines.

Reference systemd/systemd#3302
Closes #3159

@lucab
Copy link
Member Author

lucab commented Oct 12, 2016

This comes without any specific test, as the default seccomp whitelist/blacklist are already long enough to exercise this. Should I add more?

@euank @jonboulle PTAL. This only workarounds the case where we have a set of values. A single overlong line (eg. for exec) still breaks, but we need systemd patching for that.

@s-urbaniak feel free to re-milestone in case this is not fine for this release.

@euank
Copy link
Member

euank commented Oct 12, 2016

LGTM on green. Thanks!

Copy link
Contributor

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on green, I think we can land this now, thanks!

This commit workarounds a systemd bug, which causes unit file
to be incorrectly parsed.
This is triggered by property lines longer than `LINE_MAX` (2048),
which frequently occur when passing a large set of values (eg. seccomp
and paths).
In this case, we will now split such values across multiple lines.

Reference: systemd/systemd#3302
@lucab lucab force-pushed the to-upstream/break-systemd-unit-lines branch from 4e025a2 to 8bab5bb Compare October 12, 2016 19:19
@lucab
Copy link
Member Author

lucab commented Oct 12, 2016

Looks like I was too aggressive with this and broke capabilities logic. Rebased to skip those.

@s-urbaniak s-urbaniak merged commit 7ede1e1 into rkt:master Oct 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants