DAOS-16999 bio: Set LED on auto-faulty detection by tanabarr · Pull Request #17630 · daos-stack/daos

tanabarr · 2026-03-02T15:07:51Z

Centralize LED state updates within the BIO module so that when the BS
state transitions to FAULTY, the LED turns ON, and when it transitions
to NORMAL, the LED turns OFF. This consolidation simplifies testing
and maintenance by ensuring that both manual and automatic set‑faulty
workflows follow the same LED‑related code paths.

Also updates RAS event list with missing entries including LED-related.

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

github-actions · 2026-03-02T15:08:11Z

Ticket title is 'LED does not transition to "ON" after auto set-faulty eviction'
Status is 'In Review'
Labels: 'LED,SPDK,VMD,scrubbed_2.8'
Job should run at elevated priority (1)
https://daosio.atlassian.net/browse/DAOS-16999

daosbuild3 · 2026-03-02T18:17:37Z

Test stage Functional on EL 8.8 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-17630/1/display/redirect

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

daosbuild3 · 2026-03-03T04:55:13Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/2/execution/node/1343/log

sherintg

@tanabarr any plans to address the below issue aswell?
On HPE ProLiant systems when the SSD is replaced, the location indicator automatically gets turned off. This does not get reflected in “dmg storage list-devices” after “dmg storage replace nvme” command.

knard38

LGTM

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

daosbuild3 · 2026-03-05T10:43:44Z

Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/3/execution/node/1280/log

NiuYawei · 2026-03-05T13:18:34Z

src/bio/bio_recovery.c

 					DP_RC(rc));
+			send_set_led(bbs, CTL__LED_STATE__ON);
+		} else {
+			send_set_led(bbs, CTL__LED_STATE__OFF);


Set LED to off only when new state is NORMAL?

daosbuild3 · 2026-03-05T16:01:41Z

Test stage Functional Hardware Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-17630/3/testReport/

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

NiuYawei · 2026-03-06T03:12:50Z

src/bio/bio_recovery.c

+		}
+
+		uuid_copy(led_msg->dev_uuid, bbs->bb_dev->bb_uuid);
+		led_msg->xs    = bbs->bb_owner_xs;


It's not necessary (and not correct) to pass in "owner xs". The set_led() is running on "init xs".

NiuYawei · 2026-03-06T03:26:13Z

src/bio/bio_recovery.c

+
+	D_ASSERT(led_msg->xs != NULL);
+
+	rc = bio_led_manage(led_msg->xs, NULL, led_msg->dev_uuid,


This "bio_xs_context" is the "device owner xs", it's not the "init xs".

Unfortunately there is an available interface to get "init xstream" in current code, you could replace the "bd_init_thread" with "bd_init_xs" and provide a function to get the "init xs".

NiuYawei · 2026-03-06T03:29:54Z

src/bio/bio_monitor.c

 auto_faulty_detect(struct bio_blobstore *bbs)
 {
-	struct smd_dev_info	*dev_info;
+	struct smd_dev_info     *dev_info;


This change could be reverted.

NiuYawei · 2026-03-06T04:05:21Z

src/bio/bio_recovery.c

 		    bbs->bb_state != BIO_BS_STATE_SETUP)
 			rc = -DER_INVAL;
+		else
+			send_set_led(bbs, CTL__LED_STATE__ON);


It's better to move downward after the faulty state being persistent (after smd_dev_set_state() is successfully called).

NiuYawei · 2026-03-06T04:06:51Z

src/bio/bio_recovery.c

 		if (bbs->bb_state != BIO_BS_STATE_SETUP)
 			rc = -DER_INVAL;
+		else
+			send_set_led(bbs, CTL__LED_STATE__OFF);


This won't take effect. It should be called revive_dev() after the normal state being persistent. (after smd_dev_set_state() is successfully called).

NiuYawei · 2026-03-06T04:09:25Z

src/bio/bio_device.c

-			    NULL, 0);
-	if (rc != 0)
-		DL_ERROR(rc, "Reset LED on device:" DF_UUID " failed", DP_UUID(d_bdev->bb_uuid));
-


Current xs is the "init xs", the bio_led_manage() call should be kept to turn off LED.

daosbuild3 · 2026-03-06T07:57:46Z

Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/4/execution/node/1280/log

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

…function src/bio/bio_internal.h - Added init_xs_context() declaration src/bio/bio_recovery.c - Fixed LED message to use init xstream context Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

daosbuild3 · 2026-03-06T13:35:15Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/4/execution/node/1321/log

daosbuild3 · 2026-03-12T09:13:54Z

Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/12/execution/node/1363/log

daosbuild3 · 2026-03-12T13:20:39Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/12/execution/node/1322/log

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr · 2026-03-12T16:43:29Z

src/bio/bio_recovery.c

+	D_ASSERT(is_init_xstream(led_msg->xs));
+
+	bdev = lookup_dev_by_id(led_msg->dev_uuid);
+	if (bdev != NULL && bdev->bb_led_identify_active) {


The identify active flag doesn't seem to be read as intended here, the workflow identify->set_faulty results in LED ON whereas set_faulty->identify->replace results in LED QUICK_BLINK. @NiuYawei any ideas?

resolved, I had to remove the extra set LED call in drpc set-faulty handler that I had forgot

kjacque

My comments are minor and non-blocking. Once you have confirmed it's working correctly in your tests, I'll approve.

kjacque · 2026-03-12T19:54:18Z

src/bio/bio_device.c

+		/* Keep faulty and timer operations mutually exclusive */
+		if (is_faulty != NULL) {
+			/* Set is_faulty return value */
+			*is_faulty = (dev_info->bdi_flags & NVME_DEV_FL_FAULTY) ? true : false;


Just a suggestion that feels a little more intuitive in C:

Suggested change

*is_faulty = (dev_info->bdi_flags & NVME_DEV_FL_FAULTY) ? true : false;

*is_faulty = (dev_info->bdi_flags & NVME_DEV_FL_FAULTY) != 0;

kjacque · 2026-03-12T20:04:00Z

src/bio/bio_device.c

+set_timer_or_check_faulty(struct bio_xs_context *xs_ctxt, struct spdk_pci_addr pci_addr,
+			  uint64_t *expiry_time, bool *is_faulty)


If they're now mutually exclusive, I think it would be better for them to be separate functions. Using helper functions to share the boilerplate would avoid excessive duplication. I won't block on that for now, but I think it would be an improvement to a function that does completely different things based on input params.

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

daosbuild3 · 2026-03-13T11:56:47Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/14/execution/node/1362/log

daosbuild3 · 2026-03-14T03:36:48Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/15/execution/node/1363/log

daosbuild3 · 2026-03-17T00:12:53Z

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-17630/16/execution/node/429/log

knard38

Last modification LGTM

tanabarr · 2026-03-17T10:24:39Z

CI failures in the last few runs are intermittent and unrelated to the change. Features: nvme is being used to achieve appropriate coverage and probably the reason why a green run cannot be achieved.

tanabarr · 2026-03-18T10:42:47Z

@daos-stack/daos-gatekeeper ping!

mchaarawi · 2026-03-18T12:22:25Z

@daos-stack/daos-gatekeeper ping!

@tanabarr since @NiuYawei reviewed this PR, he should approve before you request gatekeeper

mchaarawi · 2026-03-18T12:22:59Z

@daos-stack/daos-gatekeeper ping!

@tanabarr since @NiuYawei reviewed this PR, he should approve before you request gatekeeper

please add gatekeeper again after Niu approves. thanks.

DAOS-16999 bio: Set LED on auto-faulty detection

c03f50a

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr requested review from NiuYawei, knard38 and sherintg March 2, 2026 15:07

tidy up

b7a224a

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

sherintg previously approved these changes Mar 3, 2026

View reviewed changes

knard38 previously approved these changes Mar 3, 2026

View reviewed changes

apply led update in bio_bs_state_set

0ea56d3

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr dismissed stale reviews from knard38 and sherintg via 0ea56d3 March 4, 2026 23:01

NiuYawei reviewed Mar 5, 2026

View reviewed changes

tanabarr added 2 commits March 5, 2026 17:02

documentation updates

4ce4116

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

only set on normal or faulty state changes

8b6134b

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr requested review from NiuYawei, knard38 and sherintg March 5, 2026 21:58

tanabarr marked this pull request as ready for review March 5, 2026 21:59

tanabarr requested review from a team as code owners March 5, 2026 21:59

NiuYawei reviewed Mar 6, 2026

View reviewed changes

tanabarr added 2 commits March 6, 2026 12:54

address review feedback

ac3c28e

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

src/bio/bio_xstream.c - Added bd_init_xs field and init_xs_context() …

4155e8d

…function src/bio/bio_internal.h - Added init_xs_context() declaration src/bio/bio_recovery.c - Fixed LED message to use init xstream context Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

correct ras event formatting in syslog by removing newlines from msg

58d304c

Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr commented Mar 12, 2026

View reviewed changes

kjacque reviewed Mar 12, 2026

View reviewed changes

log to debug when making LED state change

ee62ade

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr dismissed stale reviews from knard38 and NiuYawei via ee62ade March 13, 2026 00:54

avoid duplicate set-faulty led change in drpc handler

45dda53

Features: nvme Signed-off-by: Tom Nabarro <thomas.nabarro@hpe.com>

tanabarr requested review from NiuYawei, kjacque and knard38 March 16, 2026 11:09

tanabarr self-assigned this Mar 16, 2026

tanabarr added the forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed. label Mar 16, 2026

github-actions bot added the priority Ticket has high priority (automatically managed) label Mar 16, 2026

kjacque approved these changes Mar 16, 2026

View reviewed changes

knard38 approved these changes Mar 17, 2026

View reviewed changes

tanabarr requested a review from a team March 17, 2026 10:21

mchaarawi removed the request for review from a team March 18, 2026 12:23

NiuYawei approved these changes Mar 18, 2026

View reviewed changes

tanabarr requested a review from a team March 18, 2026 13:00

mchaarawi merged commit 4e50d3e into master Mar 18, 2026
45 of 47 checks passed

mchaarawi deleted the tanabarr/bio-autofaulty-setled branch March 18, 2026 13:15


		D_ASSERT(led_msg->xs != NULL);

		rc = bio_led_manage(led_msg->xs, NULL, led_msg->dev_uuid,

	*is_faulty = (dev_info->bdi_flags & NVME_DEV_FL_FAULTY) ? true : false;
	*is_faulty = (dev_info->bdi_flags & NVME_DEV_FL_FAULTY) != 0;

		set_timer_or_check_faulty(struct bio_xs_context *xs_ctxt, struct spdk_pci_addr pci_addr,
		uint64_t expiry_time, bool is_faulty)

Conversation

tanabarr commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daosbuild3 commented Mar 2, 2026

Uh oh!

daosbuild3 commented Mar 3, 2026

Uh oh!

sherintg left a comment

Choose a reason for hiding this comment

Uh oh!

knard38 left a comment

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Mar 6, 2026

Uh oh!

daosbuild3 commented Mar 6, 2026

Uh oh!

daosbuild3 commented Mar 12, 2026

Uh oh!

daosbuild3 commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjacque left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daosbuild3 commented Mar 13, 2026

Uh oh!

daosbuild3 commented Mar 14, 2026

Uh oh!

daosbuild3 commented Mar 17, 2026

Uh oh!

knard38 left a comment

Choose a reason for hiding this comment

Uh oh!

tanabarr commented Mar 17, 2026

Uh oh!

tanabarr commented Mar 18, 2026

Uh oh!

mchaarawi commented Mar 18, 2026

tanabarr commented Mar 2, 2026 •

edited

Loading

github-actions bot commented Mar 2, 2026 •

edited

Loading