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

New SMART plugin #835

Merged
merged 19 commits into from
May 31, 2024
Merged

New SMART plugin #835

merged 19 commits into from
May 31, 2024

Conversation

tbzatek
Copy link
Member

@tbzatek tbzatek commented Jan 6, 2023

S.M.A.R.T. plugin based on smartmontools, providing ATA SMART log retrieval. Requires smartmontools-7.0 with JSON output support.

Only ATA SMART supported for the moment with possibility for SAS/SCSI monitoring.

libblockdev-3.1 material.
libblockdev-3.2 material.
Fixes #823

TODO:

  • tests based on fake smartctl command, supplying pre-grabbed JSON dumps
  • bd_smart_ata_set_enabled()
  • bd_smart_ata_device_self_test()
  • fix the low-power mode scenario (disabled by default in all tests), test it on a real spinning disk
  • dm-multipath compatibility: may need to explicitly specify device type (likely for all the calls)
  • add support for user-supplied JSON data input instead of reading from the device? (useful for tests)

@tbzatek tbzatek added this to the libblockdev 3.1 milestone Jan 6, 2023
@tbzatek tbzatek self-assigned this Jan 6, 2023
@tbzatek tbzatek force-pushed the smart-1 branch 7 times, most recently from e660940 to 4f426a4 Compare January 10, 2023 15:32
@tbzatek tbzatek force-pushed the smart-1 branch 2 times, most recently from ca48e74 to 68cea4a Compare January 25, 2023 15:02
@tbzatek tbzatek force-pushed the smart-1 branch 3 times, most recently from 30b7772 to d4cc960 Compare February 3, 2023 11:52
@tbzatek tbzatek changed the title [WIP] New SMART plugin New SMART plugin Feb 3, 2023
@tbzatek tbzatek marked this pull request as ready for review February 3, 2023 12:02
@tbzatek
Copy link
Member Author

tbzatek commented Feb 6, 2023

Hmm, just noticed that udisks actually reports precise temperature in Kelvins like 297.14999999999998 (aka 297.15). Do we want to deal with the fractions and turn all temperatures to float?

@tbzatek
Copy link
Member Author

tbzatek commented Jun 28, 2023

Rebased against git master, ready for review. Will address the multipath stuff in a separate pull request.

@tbzatek tbzatek requested a review from vojtechtrefny August 7, 2023 12:06
@tbzatek tbzatek force-pushed the smart-1 branch 2 times, most recently from 3cde901 to 6311fdd Compare August 23, 2023 12:10
@tbzatek tbzatek changed the title New SMART plugin [WIP] New SMART plugin Aug 29, 2023
@tbzatek tbzatek marked this pull request as draft August 29, 2023 13:15
@tbzatek

This comment was marked as outdated.

Copy link
Member

@vojtechtrefny vojtechtrefny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me in general, just some questions about the plugin priorities and small nitpicks.

tbzatek added 11 commits May 27, 2024 16:38
S.M.A.R.T. plugin based on smartmontools, providing ATA SMART log
retrieval. Requires smartmontools-7.0 with JSON output support.

Only ATA SMART supported for the moment with possibility for SAS/SCSI
monitoring.
Should generally work on any device type. Unfortunately smartctl
doesn't always report errors in the JSON output, taking process exit
code in account instead.
Testing locally created LIO, loop and scsi_debug devices,
none of which actually support (ATA) SMART reporting,
thus testing error cases only.
Dummy devices only, nothing useful in the JSON output either,
no need for testing through the fake smartctl command.
This never worked as intended and was generally buggy, not being
able to distinguish a real error code. By making the arguments less
restricted this will also allow SAT protocol to be used.

This commit serves as a point of git history in case someone wants
to bring this functionality back.
…lues

This introduces a level of abstraction for attribute names and
its values. As long as the smartmontools drivedb is vastly
inconsistent in attribute names, a whitelist for each well
known attributes had to be introduced. It serves as
a validation point as well and well known name is provided
only once it passes the whitelist.

Along with attribute name validation, a definition
of attribute value unit is carried in the table. So besides
the well known name a parsed value and its unit is provided
as well.
@tbzatek tbzatek force-pushed the smart-1 branch 2 times, most recently from 34cece9 to 6dec261 Compare May 27, 2024 15:28
tbzatek added 8 commits May 27, 2024 18:24
Refactoring in preparation for a second implementation based
on libatasmart.
Designated as a primary SMART plugin implementation, based
on libatasmart code that has proven stable for a decade.
Reads the SMART data from a supplied blob instead from
a disk. Provides testing functionality that has been
present in UDisks for years (and likely ends up
deprecated anyway).

Most of the supplied skdump blobs were obtained from
libatasmart bugzilla.
Since we already have a whitelist/conversion table between
smartmontools drivedb.h-style attributes to a libatasmart
well-known attribute names, an idea of additional validation
against drivedb.h came up.

This commit implements a very basic drivedb.h parser for
a built-in include. This needs to be supplied externally
from smartmontools (or downloaded separately for the build).

When enabled, a drive model and firmware is matched against
the drivedb regexes and non-matching attribute definitions
are reported as 'unutrusted'/'unknown'.
When the commandline lvm plugin is disabled in build time
yet lvm_dbus is enabled, the lvm.h header file wouldn't
get installed.
Finally an example of a boiling temperature reported.
Libatasmart' temperature_cb() as iterated via sk_disk_smart_get_temperature()
goes through the table and only the last available value
of four matched attributes is reported back to the caller.
This is different from smartmontools which defines matching
priority.

While there are four attributes to be matched, the latter two
are actually overrides from the standard set. This is where
drivedb comes into play, however for the moment we don't have
forward checking. Thus only match storaged-project#194 and storaged-project#190, which are
typically safe.
@vojtechtrefny
Copy link
Member

Jenkins, test this please.

2 similar comments
@vojtechtrefny
Copy link
Member

Jenkins, test this please.

@vojtechtrefny
Copy link
Member

Jenkins, test this please.

Copy link
Member

@vojtechtrefny vojtechtrefny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now, thank you.

@vojtechtrefny vojtechtrefny merged commit 7e3790d into storaged-project:master May 31, 2024
33 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ATA/SAS S.M.A.R.T. plugin
3 participants