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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed here and not in the perl one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. It wasn't needed when we introduced the test. Was the perl version taking transient mount units into account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
stc
shouldn't touch any transient units. Same reason why transient units haveIgnoreOnIsolate=yes
. Otherwise you'll kill things like people's graphical applications in GNOME and kill their docker containersThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably get_active_units should skip those units entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dbus interface doesn't seem to return
IgnoreOnIsolate
directly for the dbus call used, but it may be accessible through some other command.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a
Transient=yes
property exposed in dbus. Maybe we can use that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as IgnoreOnIsolate, it's
GetUnit()
in https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.systemd1.htmlso additional dbus call is needed.
It's just not clear to me how to do this with the generated dbus interface:
https://gist.github.com/Mic92/37f1789f9bfc91fa7376845442fa6d80
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel familiar enough with this library. Closing this pull request instead. Probably a release blocker if it indeed can break automounts and docker container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arianvp Won't those be user units, and thus unaffected?