mount and swap improvements #10980
Merged
mount and swap improvements #10980
Conversation
This file contains 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
|
This pull request fixes 1 alert when merging ca7d7db into 50ae773 - view on LGTM.com fixed alerts:
Comment posted by LGTM.com |
keszybz
requested changes
Dec 6, 2018
keszybz
added
the
reviewed/needs-rework 🔨
No change of logic, just some renaming, in order to match more closely the naming of the other, similar functions.
…ult_dependencies() too This deps are very similar to the -pre deps, hence establish them at the same place, in particular as they should only be generated if default deps are on. This allows us to later on remove similar code that adds in these deps whenever /proc/self/mountinfo changes.
…mount unit We need to make sure that the slice property is initialized whenever mount_load() is invoked, even if we fail to load things properly off disk. This is important since we generally don't allow changing the slice after a unit has been started. But given that we must track the state of external objects with mount units we must hence initialize the property no matter what.
If we can't process a specific line in /proc/self/mountinfo we should log about it (which we do), but this should not affect other lines, nor further processing of mount units. Let's keep these failures local. Fixes: systemd#10874
This should encapsulate things in a nicer way.
Let's initialize two fields with free_and_strdup() rather than directly with strdup(). The fields should not be initialized so far, but it's still nicer to be prepared for futzre code changes and always free what's stored before replacing it.
In a previous commit we added logic that mount_add_extras() (or more precisely mount_add_default_dependencies()) adds in dependencies on remote-fs.target and local-fs.target, hence we can drop this from mount_setup_new_unit() and let the usual load queue dispatching take care of this.
…ark it so Let's set 'from_proc_self_mountinfo' right away, since we know its from there. This is important so that when the load queue is dispatched (and thus mount_load() called) this fact is already known.
Whenever we notice a change on an existing /proc/self/mountinfo line, let's update the deps generated from it. For that, let's flush out the old deps generated this way, and add in the new ones. This takes benefit of the fact that today (unlike a comment this patch removes says) we can remove deps in a somewhat reasonable way.
…nds of load failures It doesn't matter what kind of precise failure we had earlier with loading the unit, let's report that it loaded successfully now, after all the kernel is an OK source for that, like any other.
We never know what the changes triggered by mount_set_state() do to the unit. Let's be safe and copy the device path into our set, so that we are safe against that.
…enum We pass these flags around, and even created a structure for them. Let's fix things properly, and make them a flags value of its own.
…round For services (and other units) we generally follow the rule that at the beginning of each cycle, i.e. when the INACTIVE/FAILED state is left for ACTIVATING/ACTIVE we flush out various state variables. Mount units handled this differently so far when the unit state change was effected outside of systemd: in that case these variables would be flushed out when going back to INACTIVE/FAILED already. Let's fix that, and flush out this state always during the activating transition, not during the deactivating transition.
This is similar to the previous commit which did the same change for mount units.
…of its own This adds swap_add_extras() similar to mount_add_extras(). No change in behaviour, just some refactoring.
This follows similar logic in the mount unit.
This follows similar recent changes in mount.c: error should be consider local, and not be propagated.
We don't actually return any valid 'r' here, let's explicitly return 0 here hence instead.
Much like for the mount units we need fields such as the slice initialized by the time we activate the swap, hence when the kernel let's us know about a new swap that appeared we need to initialize the slice in any Swap object we allocated for that right-away, even if we can't read the real unit file for the swap device.
poettering
force-pushed
the
mount-no-extras
branch
from
ca7d7db
to
06721f3
Compare
|
Force pushed a new version. Only made the requested changes. Given how trivial the changes were (one comment, and one if branch reordering) taking liberty to set green label. |
poettering
added
good-to-merge/waiting-for-ci 👍
|
This pull request fixes 1 alert when merging 06721f3 into 1478aa4 - view on LGTM.com fixed alerts:
Comment posted by LGTM.com |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Primarily a fix for #10874 but many other smaller improvements too.