-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add mount by LABEL= for ext filesystems #532
Conversation
@@ -222,6 +222,9 @@ def _add_mounts(self) -> None: | |||
# but instead a volume_id which is not case-sensitive | |||
fs_id = fs_id[:4].upper() + "-" + fs_id[4:].upper() | |||
|
|||
if fs.__type__ == "ext": |
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.
Can't comment that far up, but might be nice to refactor the code above it a bit too look like this (lose the if dev_id
):
if fs.__type__ == "xfs":
fs_id = fs.xfs.uuid
elif fs.__type__ == "ext":
fs_id = fs.extfs.uuid
fs_volume_name = fs.extfs.volume_name
last_mount = fs.extfs.last_mount
elif fs.__type__ == "btrfs":
fs_id = fs.btrfs.uuid
fs_subvol = fs.subvolume.path
fs_subvolid = fs.subvolume.objectid
elif fs.__type__ == "fat":
fs_id = fs.fatfs.volume_id
# This normalizes fs_id to comply with libblkid generated UUIDs
# This is needed because FAT filesystems don't have a real UUID,
# but instead a volume_id which is not case-sensitive
fs_id = fs_id[:4].upper() + "-" + fs_id[4:].upper()
And maybe also rename last_mount
to fs_last_mount
.
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.
Yeah, I was thinking about that too. Didn't know if i specifically wanted to do that in this ticket/PR.
Meaning, i was wondering if it might be an idea to add META
info to a filesystem, where it can fill the volume_id and information like that, where it would eliminate these checks altogether and let that meta info handle it.
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'd keep it simpler and just add different fields/properties to the base Filesystem
class. Similarly to how volume systems have a serial. But that's indeed not something for this PR.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #532 +/- ##
=======================================
Coverage 73.69% 73.69%
=======================================
Files 277 277
Lines 23056 23058 +2
=======================================
+ Hits 16990 16992 +2
Misses 6066 6066
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cfdd734
to
22be0e0
Compare
(DIS-2894)