storage: SMART support #19103
storage: SMART support #19103
Conversation
|
For now I added a new card to the disk detail which shows basic information and allows to run/abort self test. Note that for now only (S)ATA devices are supported by Udisks, NVMe support will be added in the latest release https://github.com/storaged-project/udisks/releases/tag/udisks-2.10.0 @garrett I believe you wanted to work on mockups for this, any chance you could look into proper design? |
Initial SMART self test support
tomasmatus
force-pushed
the
disks-show-smart
branch
from
f2b0ca0
to
00eb709
Compare
| <StackItem> | ||
| <SmartDetails smartInfo={drive_ata} /> | ||
| </StackItem> |
There was a problem hiding this comment.
These 3 added lines are not executed by any test. Details
| const SmartActions = ({ smartInfo }) => { | ||
| const [isKebabOpen, setKebabOpen] = useState(false); | ||
| const smartSelftestStatus = smartInfo.SmartSelftestStatus; |
There was a problem hiding this comment.
These 3 added lines are not executed by any test. Details
| const runSmartTest = async (type) => { | ||
| await smartInfo.SmartSelftestStart(type, {}); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test. Details
| const abortSmartTest = async () => { | ||
| await smartInfo.SmartSelftestAbort({}); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test. Details
| const actions = [ | ||
| <DropdownItem key="run-short-test" | ||
| isDisabled={smartSelftestStatus === "inprogress"} | ||
| onClick={() => { setKebabOpen(false); runSmartTest('short') }}> | ||
| {_("Run short test")} | ||
| </DropdownItem>, | ||
| <DropdownItem key="run-extended-test" | ||
| isDisabled={smartSelftestStatus === "inprogress"} | ||
| onClick={() => { setKebabOpen(false); runSmartTest('extended') }}> | ||
| {_("Run extended test")} | ||
| </DropdownItem>, | ||
| <DropdownItem key="run-conveyance-test" | ||
| isDisabled={smartSelftestStatus === "inprogress"} | ||
| onClick={() => { setKebabOpen(false); runSmartTest('conveyance') }}> | ||
| {_("Run conveyance test")} | ||
| </DropdownItem>, |
There was a problem hiding this comment.
These 16 added lines are not executed by any test. Details
| if (smartInfo.SmartSelftestStatus === "inprogress") { | ||
| actions.push( | ||
| <DropdownItem key="abort-smart-test" | ||
| onClick={() => { setKebabOpen(false); abortSmartTest('conveyance') }}> | ||
| {_("Abort test")} | ||
| </DropdownItem>, |
There was a problem hiding this comment.
These 6 added lines are not executed by any test. Details
| return ( | ||
| <Dropdown toggle={<KebabToggle onToggle={(_, isOpen) => setKebabOpen(isOpen)} />} | ||
| isPlain | ||
| isOpen={isKebabOpen} | ||
| position="right" | ||
| id="smart-actions" | ||
| dropdownItems={actions} /> |
There was a problem hiding this comment.
These 7 added lines are not executed by any test. Details
| export const SmartDetails = ({ smartInfo }) => { | ||
| const SmartDetailRow = ({ title, value }) => { | ||
| if (value === undefined) | ||
| return null; |
There was a problem hiding this comment.
These 4 added lines are not executed by any test. Details
| return ( | ||
| <DescriptionListGroup> | ||
| <DescriptionListTerm>{title}</DescriptionListTerm> | ||
| <DescriptionListDescription>{value}</DescriptionListDescription> | ||
| </DescriptionListGroup> |
There was a problem hiding this comment.
These 5 added lines are not executed by any test. Details
| return ( | ||
| <Card> | ||
| <CardHeader actions={{ actions: <SmartActions smartInfo={smartInfo} /> }}> | ||
| <CardTitle component="h2">{_("S.M.A.R.T")}</CardTitle> | ||
| </CardHeader> | ||
| <CardBody> | ||
| <DescriptionList isHorizontal horizontalTermWidthModifier={{ default: '20ch' }}> | ||
| <SmartDetailRow title={_("Power on hours")} value={cockpit.format(_("$0 hours"), Math.round(smartInfo.SmartPowerOnSeconds / 3600))} /> | ||
| <SmartDetailRow title={_("Last updated")} value={timeformat.dateTime(new Date(smartInfo.SmartUpdated * 1000))} /> | ||
| <SmartDetailRow title={_("Smart selftest status")} value={selftestStatusDescription[smartInfo.SmartSelftestStatus]} /> | ||
| <SmartDetailRow title={_("Number of bad sectors")} value={smartInfo.SmartNumBadSectors} /> | ||
| <SmartDetailRow title={_("Atributes failing")} value={smartInfo.SmartNumAttributesFailing} /> | ||
| </DescriptionList> | ||
| </CardBody> | ||
| </Card> |
There was a problem hiding this comment.
These 15 added lines are not executed by any test. Details
| /* | ||
| * This file is part of Cockpit. | ||
| * | ||
| * Copyright (C) 2017 Red Hat, Inc. |
There was a problem hiding this comment.
correct, we live in ${current_year}
| return ( | ||
| <Card> | ||
| <CardHeader actions={{ actions: <SmartActions smartInfo={smartInfo} /> }}> | ||
| <CardTitle component="h2">{_("S.M.A.R.T")}</CardTitle> |
There was a problem hiding this comment.
I don't think that's really something one can translate?
There was a problem hiding this comment.
Maybe in non-latin languages? Like chinese or russian or so?
There was a problem hiding this comment.
Yeah, if in doubt, keep it translatable. I can't say I like the many dots, this looks a bit like a 90's superhero comic 😁 So please either "SMART", or at least make it consistent and add a period to the T as well.
Yes, it does. SmartUpdate() has an |
|
I am not sure how to test this. In order to run |
Neither does udisks test it (only if they find SMART disks), maybe because of the same reason? Random googling https://stackoverflow.com/questions/48351096/how-to-emulate-a-sata-disk-drive-in-qemu So my suggestion would be to yolo hack |
|
FYI udiskctl has a smart simulate options: |
|
@tomasmatus Are you still interested in working on this? It's quite a nice feature. |
|
Wow its been a year already... yes, this is something i should revive @martinpitt |
|
If i recall correctly the issue was tests, so trying to experiment with |
|
So, I think having SMART info is good. I remember I suggested it a long time ago in some issue and/or mockup. I don't remember where or what though. Where would this go? It's usually stylized as SMART, not S.M.A.R.T. (which is just awkward). At least in English, acronyms and initialisms usually do not include periods. https://en.wikipedia.org/wiki/Acronym — and acronyms that are proper names (including SMART) or commonly used are generally not supposed to even have periods... and they're usually (but not always) all in uppercase. It's kind of weird with all kinds of special-casing, however; Wikipedia's acronym page actually has a "nomenclature" section with some examples: https://en.wikipedia.org/wiki/Acronym#Nomenclature — and it even covers pronunciation a bit (when some are pronounced and some are spelled out... and even some that are a mixture, like "JPEG" and "MS-DOS"). As far as being detailed; I think we should show an overview and use a "progressive disclosure" concept. (That is, show the important, common stuff first and then click somewhere to see everything.) Since it looks like you're picking this up, it would be great to talk about the design of it. 👍 I know we had the concept of SMART errors propagating all the way up to the health card on the overview too (which would then link to the details). |
|
I am interested in this feature. Is there anything that can be done to progress the PR? |
|
@SchoolGuy: A recent update related to this PR: @tomasmatus is back and is planning on picking this up (as you can see in an above comment). I will be working with him on the design. If you'd like to help out, that'd be fine as well. In what ways are you interested in SMART? What data are you looking for, specifically? And when do you consult SMART data, usually? (From time to time or only when there are strange things going on as a diagnosis tool?) |
|
@garrett I am no designer by trade as such I am not sure on what part of the design I may be of assistance. I should be able to dig into the code and help out with implementing an idea that somebody else had. If that is not available I may be able to design things on my own but they may be hard to use... I am interested in SMART in two ways:
The idea for this specific project is to have Cockpit running as a WebUI for a single host with an mdadm array running and that resulting device is exposed as an NFS share that other hosts will use. In short: I want to use the host as a NAS. I will grab the SMART values additionally with the smartd exporter and will let Prometheus grab that but since that will be hosted by one of the other hosts I want to be able to know if a failure there is related to the mdadm array being down or by a configuration error. I am a quite paranoid person (regarding my infrastructure) and as such I am checking my monitoring quite regularly but it is not a fixed schedule I have. |
|
I'm not a developer but I'm a Fedora packager and would be very interested in seeing this in Fedora/RHEL. |
|
I can try to find some time to get back around to this to make some mockups. But the gist is that we need it in two places:
FWIW, here are the design ideas for disk issues (from a long time ago): #8787 (comment) SMART is mentioned, with regard to showing it in the health card:
We probably want to have some kind of status API where the overview page can query the storage page to get the SMART info and show it on the health card if there is a problem (and only if there is a problem), similar to the updates being on the health card when there are updates. (The software updates page is queried for this.) |
|
Perfect. This is already a great amount of detail. @tomasmatus how to you want to organize so we don't run into conflicts working on the same files? |
That fails into the scope of Cockpit!
What metrics are you interested in? UDisks provides some basic health metrics as seen in the screenshot on top. For other attributes we have to use SmartGetAttributes, so if you can give us a list of interesting attributes and maybe a
Cockpit will only support showing SMART values for a single host, the host you connect too. We don't want to support anything outside of the UDisks API |
|
For reference, udisks reports only this information from SMART: It's not much, but it's probably enough in most cases? If this is all, then we can't do the expand idea, unless we get more information, like from |
|
Is it worth looking to see how gnome-disk-utility looks up the extra data? |
See my comment:
|
|
I've looked into testing this in a test virtual machine, the main issue is that even if I add a sata disk, udisks does not seem to identify this as a ATA drive: On my nas Attaching a SATA disk does allow me to use smartctl on it. |
|
I like the data that is presented here, including the ability to start a SMART test: |
Right, that's the extended data that we're definitely not going to show by default. It's way too verbose and misleading. We could, in a third stage, add all the details similar to that, possibly in a modal dialog similar to that. But, we should handle SMART in these steps:
This first PR should implement step # 1. Then we need to do a follow-up for step # 2 (overview). Then eventually add a step # 3 for more details. We should not do this all in one go, nor should we just show all the data points SMART can give us. (Many are very misleading, scary, or not relevant depending on the circumstance. And it varies per manufacturer and storage medium (spinning disks of different types, SSD, NVMe, etc.). It's a mess, really.) (I think @tomasmatus and I will talk about this more either later this week or next week.) |
|
For testing SMART in CI, there is a udisks issue now https://issues.redhat.com/browse/STORAGECFG-801 |
Please check this release out: https://github.com/storaged-project/udisks/releases/tag/udisks-2.10.90
This works now in UDisks, however it's still an However I've found this hidden workaround in Let me know if it helps your testing use case. |
Initial SMART self test support
Uses data available in UDisks http://storaged.org/doc/udisks2-api/latest/gdbus-org.freedesktop.UDisks2.Drive.Ata.html
https://issues.redhat.com/browse/COCKPIT-935