Re: ataraid(4) missing disk handling

[Christoph Badura <bad%bsd.de@localhost>]

On Mon, Mar 10, 2025 at 04:43:30PM +0200, Andrius V wrote:
> Taylor has a good analysis and explanation of the issue
> https://mail-index.netbsd.org/netbsd-bugs/2024/03/26/msg082202.html,
> which I also noticed by testing ATA RAID setup on VIA controllers.

> [...]

Good analysis.

> Given that three statuses are defined for adi_status
> (https://nxr.netbsd.org/xref/src/sys/dev/ata/ata_raidvar.h#75), I
> probably need to check if any of the flags are defined ((adi_status &
> (ADI_S_ONLINE | ADI_S_ASSIGNED | ADI_S_SPARE)) instead
> (https://netbsd.org/~andvar/ata_raid_fix2.diff).

The first part seems fine.  Although I didn't look at what the difference
between vn_bdev_open() vs. the deleted code is.

In the second part this bit is weird for me to read:

+		else
+			vp = NULL;
 		if (vp == NULL) {

I'd write it as:

-		vp = ata_raid_disk_vnode_find(adi);
+		vp = NULL;
+		if (adi->adi_status &
+		    (ADI_S_ONLINE | ADI_S_ASSIGNED | ADI_S_SPARE))
		if (vp == NULL) {
 			/*
 			 * XXX This is bogus.  We should just mark the

But Taylor seemed to be fine with it if I remember the discussion on ICB
correctly.  A matter of style I guess.

> Another alternative is to check that adi->adi_dev IS NULL as Taylor
> proposed in his analysis thread.

But that proposal never explained how an adv with adi->adi_dev==NULL ended
up on the ataraid_disk_vnode_list which isn't supposed to happen!

--chris