Skip to content

Fixes #22: SMART bit string conversion is reversed#24

Closed
NiceGuyIT wants to merge 1 commit intoprometheus-community:masterfrom
NiceGuyIT:issue-22
Closed

Fixes #22: SMART bit string conversion is reversed#24
NiceGuyIT wants to merge 1 commit intoprometheus-community:masterfrom
NiceGuyIT:issue-22

Conversation

@NiceGuyIT
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown

@vagifzeynalov vagifzeynalov left a comment

Choose a reason for hiding this comment

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

Rather than reversing the correctly made bitmap better to change the code how to process it.

From https://linux.die.net/man/8/smartctl
Bit 0: Command line did not parse.
Bit 1: Device open failed, device did not return an IDENTIFY DEVICE structure, or device is in a low-power mode (see '-n' option above).
Bit 2: Some SMART or other ATA command to the disk failed, or there was a checksum error in a SMART data structure (see '-b' option above).
Bit 3: SMART status check returned "DISK FAILING".
Bit 4: We found prefail Attributes <= threshold.
Bit 5: SMART status check returned "DISK OK" but we found that some (usage or prefail) Attributes have been <= threshold at some time in the past.
Bit 6: The device error log contains records of errors.
Bit 7: The device self-test log contains records of errors. [ATA only] Failed self-tests outdated by a newer successful extended self-test are ignored.

@NiceGuyIT
Copy link
Copy Markdown
Member Author

SMARTCtlResult is correct with the LSB on the right of the number. The string representation is incorrect because the LSB is on the left of the string. In the string representation, bit 5 would be bits[2] which while technically correct, is semantically incorrect.

I have replaced the string/byte comparisons with integer/bit comparisons.

@vagifzeynalov
Copy link
Copy Markdown

Perhaps, if you want to make it perfect, all messages along with true/false results should be stored as an object in the list. So you can do something like

list = [{message, false}...{message,  true}]

finalResult = true
mask = 1
for (i = 0 to 7) {
   if (SMARTCtlResult & mask) != 0 {
           if list[i].result {
               logger.warning(list[i].message)
           } else {
               logger.error(list[i].message)
           }
           finalResult = finalResult &  list[i].result
   }
   mask << 1
}
return finalResult

@vagifzeynalov
Copy link
Copy Markdown

in fact, the whole method could be collapsed into simple check of first two bits.
like return SMARTCtlResult & 3 (not even need to create a method)
because everything else is "just for fun" :)

@SuperQ
Copy link
Copy Markdown
Contributor

SuperQ commented Jul 17, 2022

I think #37 was a simpler fix.

@SuperQ SuperQ closed this Aug 4, 2022
@NiceGuyIT NiceGuyIT deleted the issue-22 branch November 5, 2022 12:43
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