-
Notifications
You must be signed in to change notification settings - Fork 36
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
Verify nvme from interface instead of name #94
Conversation
Althrough most nvme disk is solid state disk, i still suggest try to check identify Namespace data structure (CNS 08h). |
Hi, thanks for the quick reply. True, there could be HDDs with NVMe but from my investigations only seagate has a prototype, but this is still not commercially available (see: https://www.tomshardware.com/news/seagate-demonstrates-hdd-with-pcie-nvme-interface)
I think this potential error of wrongly reported HDDs with NVME is much better than 100% wrong SSDs on Windows. Is the Namespace data structure (CNS 08h) even reported by smartctl? I cannot find it. If you could give me a hint and/or sample data I could do some further investigation. |
Nice contrib @BjoernWaechter , I think we should notice that the smart is enabled by default on any nvme interface rather than devices with "nvme" in it. On the other hand @jackeichen has its point on that we have to check if nvme is actually a SSD. |
After checking this in deep, I don't see an easy way to detect if an NVMe is SSD. Probably, that's why the past behaviour also was that if a disk had "nvme" on its name, it must be an SSD. So, this change keeps that "bug" (if we should say that) to nvme = SSD. @jackeichen do you have a way to detect if an NVMe is SSD? Maybe some specific call to smartctl? This, is an example of smartctl output of an nvme, tell me if you see something that points to a "ssd" device type that I'm missing please:
|
I looked a bit into where the interface information comes from. As far as I get it its read here: The second occurrence is here:
This output is created here: I'm not familiar with this C++ code but it looks like a reliable source to me. |
Thank you, but I think none of that code reveals if the underlying storage mechanism is SSD or HDD. Don't misunderstand me, for me this PR is OK, but if we can improve somehow the code SSD detection, the better. From my perspective, and, according to the data I have there it isn't (unless, maybe, accesing ourselves to the smart data...). That's why, maybe @jackeichen may bring us some light that we are missing and probably help us on this. |
I cannot say that the above method is a common type, at least two challenges here:
But it is not a bad ideas to add it to the code. |
@jackeichen Do you know if this is somehow visible in the result created by smartctl? |
Let's follow the discussion here: #95 |
Could this PR fix the nvme windows issue?
Closes #93