systemd: support sending file descriptors to systemd - #16864
Merged
ZeyadYasser merged 4 commits intoApr 29, 2026
Conversation
This is needed for future work (fdstore). Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #16864 +/- ##
==========================================
+ Coverage 77.63% 79.04% +1.41%
==========================================
Files 1361 1370 +9
Lines 189322 190687 +1365
Branches 2446 2465 +19
==========================================
+ Hits 146971 150733 +3762
+ Misses 33478 30872 -2606
- Partials 8873 9082 +209
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Wed Apr 29 07:01:52 UTC 2026 Failures:Executing:
Skipped tests from snapd-testing-skipIf you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)
|
zyga
approved these changes
Apr 6, 2026
zyga
left a comment
zyga
left a comment
Contributor
There was a problem hiding this comment.
Looks good, just a few inline comments.
| if err != nil { | ||
| return err | ||
| } | ||
| // TODO: keep it open to avoid re-opening and make sure to have O_CLOEXEC |
Contributor
There was a problem hiding this comment.
Ditto, think cloexec + close is correct.
zyga
reviewed
Apr 6, 2026
zyga
left a comment
zyga
left a comment
Contributor
There was a problem hiding this comment.
One more comment on API design.
use *os.File instead of raw fds to avoid potential race with go GC. Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
bboozzoo
reviewed
Apr 28, 2026
| // descriptors associated with passed files to systemd. | ||
| // | ||
| // inspired by libsystemd/sd-daemon/sd-daemon.c from the systemd source | ||
| func SdNotifyWithFds(notifyState string, files ...*os.File) error { |
Contributor
There was a problem hiding this comment.
what files would be sent with FDSTOREREMOVE=1 ?
Contributor
Author
There was a problem hiding this comment.
zyga
reviewed
Apr 28, 2026
zyga
left a comment
zyga
left a comment
Contributor
There was a problem hiding this comment.
Quick pass, two points inline.
| return err | ||
| } | ||
|
|
||
| // SdNotifyWithFds sends the given state string notification and file |
Contributor
There was a problem hiding this comment.
Do you think this should be private and we should have a number of public wappers that call it with specific value of notifyState?
Contributor
Author
There was a problem hiding this comment.
it's a systemd primitive that might be used in different packages like SdNotify, also trying to follow libsystemd
bboozzoo
reviewed
Apr 28, 2026
| } | ||
|
|
||
| // SdNotifyWithFds sends the given state string notification and file | ||
| // descriptors associated with passed files to systemd. |
Contributor
There was a problem hiding this comment.
Use
SdNotifyto send fd removal requests.
Contributor
Author
There was a problem hiding this comment.
SdNotifyWithFds could be used to send fds to systemd unrelated to the fdstore
zyga
reviewed
Apr 28, 2026
zyga
left a comment
zyga
left a comment
Contributor
There was a problem hiding this comment.
Clarified location of keep-alive call
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
ZeyadYasser
added
the
Auto rerun spread
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is needed for future work (split from fdstore #16784).