some block device handling fixes#9255
Merged
keszybz merged 6 commits intosystemd:masterfrom Jun 12, 2018
Merged
Conversation
keszybz
reviewed
Jun 11, 2018
src/basic/blockdev-util.c
Outdated
| if (r <= 0) | ||
| return r; | ||
| /* For the specified block device tries to chase it down the layers, in case LUKS-style DM stacking is used, | ||
| * trying to find the next upper layer. */ |
Member
Author
There was a problem hiding this comment.
up, down, potato, potato ;-)
src/core/cgroup.c
Outdated
| return -ENODEV; | ||
| } | ||
| if (r < 0) | ||
| return log_warning_errno(r, "Failed to determine block device backing btrfs file system '%s': %m", p); |
Member
There was a problem hiding this comment.
This message assumes that p must be a btrfs device. I think it'd be better to reorder this and report the more specific case of failing to determine btrfs backing device first:
r = btrfs_get_block_device(p, ret);
if (r < 0 && r != -ENOTTY)
return log_warning_errno(r, "Failed to determine if path '%s' is backed by a btrfs file system: %m", p);
if (r == -ENOTTY) {
log_warning("'%s' is not a block device node, and file system block device cannot be determined or is not local.", p);
return -ENODEV;
}
We document the rule that return values >= 0 of functions are supposed to indicate success, and that in case of success all return parameters should be initialized. Let's actually do so. Just a tiny coding style fix-up.
… into function of its own That way we can use it in code that already acquired a dev_t from some source.
Let's chase block devices through btrfs and LUKS like we do elsewhere.
This drops needless safety checks that ensure we only reference block devices for blockio/io settings. The backing code was already able to accept regular file system paths too, in which case the backing device node of that file system would be used. Hence, let's drop the artificial restrictions and open up this underlying functionality.
Let's make sure we don't validate "char-*" and "block-*" expressions as paths.
poettering
force-pushed
the
block-dev-fixes
branch
from
b6a2132 to
bae47ba
Compare
Member
Author
|
Force pushed a new version now. PTAL. Only changes are the points you raised |
Member
|
LGTM. |
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
2 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.
I noticed these while reviewing #9247. I figure that PR should be rebased on this as soon as it is merged.