Skip to content
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

Improve robustness of drive type auto-detection on Linux #343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

whitslack
Copy link

DtaDevOS::init(…) in linux/DtaDevOS.cpp currently detects the drive type by checking whether the user-specified device node path starts with /dev/nvme or /dev/s. This is fragile and breaks mysteriously in cases such as when the user specifies a relative path to a device node or when the user specifies a device through a udev-managed symbolic link in /dev/disk/by-id/ et al.

This Pull Request reimplements drive type auto-detection using a more robust algorithm that stats the specified device node and treats it as a SATA drive if the device node's major number is one of the SCSI_DISK*_MAJOR numbers or as an NVMe drive if the device is bound to the nvme kernel driver. (The latter check requires sysfs to be mounted at /sys.)

Example before change:

# cd /dev
# sedutil-cli --printDefaultPassword sdc
DtaDevOS::init ERROR - unknown drive type
Invalid or unsupported disk sdc

# sedutil-cli --isValidSED /dev/disk/by-id/ata-Samsung_SSD_860_EVO_2TB_S597NZFNA01928Y
DtaDevOS::init ERROR - unknown drive type

Example after change:

# cd /dev
# sedutil-cli --printDefaultPassword sdc
MSID: crucial_bymicron

# sedutil-cli --isValidSED /dev/disk/by-id/ata-Samsung_SSD_860_EVO_2TB_S597NZFNA01928Y
/dev/disk/by-id/ata-Samsung_SSD_860_EVO_2TB_S597NZFNA01928Y SED -2- Samsung SSD 860 EVO 2TB                  RVT04B6Q

DtaDevOS::init() previously detected the drive type by checking whether
the user-specified device node path starts with "/dev/nvme" or "/dev/s".
This is fragile and breaks mysteriously in cases such as when the user
specifies a relative path to a device node or when the user specifies a
device through a udev-managed symbolic link in /dev/disk/by-id/ et al.

This commit reimplements drive type auto-detection using a more robust
algorithm that stats the specified device node and treats it as a SATA
drive if the device node's major number is one of the SCSI_DISK*_MAJOR
numbers or as an NVMe drive if the device is bound to the "nvme" kernel
driver. (The latter check requires sysfs to be mounted at /sys.)
@r0m30
Copy link
Contributor

r0m30 commented Aug 9, 2021

I think the scan code is really broken. Your fix propagates the SAS/SCSI issues with the code. I agree this is better but until the SAS issue is fixed other improvements need to wait.

@henriquebecker91
Copy link

Also stumbled on this. It seems absurd to me to make any inference based on the path of the device name. Solved for now by using realpath /dev/disk/by-... (Bash) to get the usual path, save it to a variable, and use it instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants