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

fix: use --scan-open instead of --scan of smartctl to provide correct… #15724

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

TTTPOB
Copy link
Contributor

@TTTPOB TTTPOB commented Aug 9, 2024

… device type info

Summary

see #15723 , address incorrect device type information by smartctl --scan

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #15723

@telegraf-tiger
Copy link
Contributor

telegraf-tiger bot commented Aug 9, 2024

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Aug 9, 2024
@TTTPOB
Copy link
Contributor Author

TTTPOB commented Aug 9, 2024

!signed-cla

use smartctl --scan-open instead of smartctl --scan, see
https://www.smartmontools.org/ticket/811
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

My understanding of scan open is it also tries to open the device. I think this is fine given we already need elevated privileges.

Please also update the README and sample.conf files to say we use scan-open instead of scan as well:

  1. readme: configuration (or update the sample config and run make docs)
  2. readme: debugging issues section
  3. sample.conf: above devices_include/devices_exclude

@powersj powersj added the waiting for response waiting for response from contributor label Aug 9, 2024
TTTPOB and others added 2 commits August 12, 2024 14:45
Reflect the parameter change of smartctl when doing device scan.
@powersj powersj added ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. and removed waiting for response waiting for response from contributor labels Aug 12, 2024
@telegraf-tiger
Copy link
Contributor

@powersj powersj merged commit 706e922 into influxdata:master Aug 12, 2024
27 checks passed
@github-actions github-actions bot added this to the v1.31.3 milestone Aug 12, 2024
powersj pushed a commit that referenced this pull request Aug 12, 2024
…rrect device type info (#15724)

(cherry picked from commit 706e922)
@ilyam8
Copy link

ilyam8 commented Aug 12, 2024

Keep in mind that devices in SLEEP and STANDBY modes may spin up when using --scan-open (due to commands issued during device type autodetection).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug report] smartctl input plugin reports incorrect device type
5 participants