Skip to content

Conversation

@tomasmatus
Copy link
Member

@tomasmatus tomasmatus commented Mar 21, 2025

fixes: #21728

Libblockdev changed how SmartFailing property is set. Now it is less strict with when a disk is considered as failing.

storaged-project/libblockdev#1097

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks! But I'm afraid this needs to do a libblockdev or m.image or TEST_SCENARIO or whatever dynamic check -- with this it's now failing on f42 in the opposite way.

Comment on lines 88 to 89
# latest libblockdev builds from copr have different behavior when assessing disk as failed
is_nightly_scenario = "daily" in getenv("TEST_SCENARIO", "")
Copy link
Member

Choose a reason for hiding this comment

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

This is not a robust condition. The API change can land in Fedora or updates-testing any day, or in a distro other than Fedora 41. This needs to be a feature check or a version comparison.

Comment on lines 93 to 117
if is_nightly_scenario:
check_smart_info("Disk is OK", "556 hours", "Aborted", bad_sectors="1")
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to factorize this, do the version check and set some overall_status = "..." string conditionally there, then use the string here and below.

fixes: cockpit-project#21728

Libblockdev changed how SmartFailing property is set. Now it is less
strict with when a disk is considered as failing.

storaged-project/libblockdev#1097
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thank you! How painful.. I would have liked the string factorization, but it's ok -- we'll see this again when we clean up the hack, and then we can do a simpler version check.

@martinpitt martinpitt merged commit b25fb35 into cockpit-project:main Mar 25, 2025
89 of 90 checks passed
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.

TestStorageSmart.testSmart fails in /daily

2 participants