-
Notifications
You must be signed in to change notification settings - Fork 542
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 new section fstype section under hardware devices. #3538
Conversation
5ed4fcd
to
d543a88
Compare
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
1 similar comment
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
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.
lgtm
just tested it as well and no issues with lots of fs and empty first FSTYPE, nicely dealt with
In general, I am not against maintaining centrally a next type of devices. Keeping a dict per fstype - I dont see much use case for the "per FS type" in particular (which does not mean there is a legitimate use case for it). |
The fstype will not just enumerate the fstypes but also linux_raid_member or LVM2_member. The complete list can be obtained from |
1705350
to
3ad9af4
Compare
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.
ACK, then. Thanks for explaining,
Not sure with the device key name (fstype
) as I got confused by it, but if there wont be an objection from anybody else or no better name, I am OK with 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.
Minor comments, code overall looks good.
sos/report/__init__.py
Outdated
if "ext" in helper[0]: | ||
helper[0] = 'ext2/3/4' |
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 we not be more specific here? If not, I think we should just put ext
rather than slashed version possibilities.
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.
ACK will change it to ext there is no conflict with other blkid types
blkid -k | grep ext
xfs_external_log
ext4dev
ext4
ext3
ext2
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.
Left it as ext4
3ad9af4
to
3b38f3a
Compare
Add new section fstype section under hardware devices which lists devices by filesystem, based on lsbl -nrpo output. Devices with no filesystem are placed into unknown section and Ext2/3/4 are put into section called ext4. Signed-off-by: Lukas Herbolt <[email protected]>
059587e
to
cf31367
Compare
As discussed in the #3502 adding new section under _get_hardware_devices(). Please review it.
Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines