On Thu, Aug 21, 2025 at 07:16:49PM +1000, Donald Douwsma wrote:
>
>
> On 19/8/25 06:45, Darrick J. Wong wrote:
> > On Mon, Aug 18, 2025 at 03:22:02PM -0500, Eric Sandeen wrote:
> >> We had a report that a failing scsi disk was oopsing XFS when an xattr
> >> read encountered a media error. This is because the media error returned
> >> -ENODATA, which we map in xattr code to -ENOATTR and treat specially.
scsi_debug can be configured to return a MEDIUM error which if I follow
the discussion properly, would result in the block layer converting it
to ENODATA.
Carlos
P.S.
I'm pretty sure I heard somebody suggesting scsi_debug before,
I just don't remember who and where.
> >>
> >> In this particular case, it looked like:
> >>
> >> xfs_attr_leaf_get()
> >> error = xfs_attr_leaf_hasname(args, &bp);
> >> // here bp is NULL, error == -ENODATA from disk failure
> >> // but we define ENOATTR as ENODATA, so ...
> >> if (error == -ENOATTR) {
> >> // whoops, surprise! bp is NULL, OOPS here
> >> xfs_trans_brelse(args->trans, bp);
> >> return error;
> >> } ...
> >>
> >> To avoid whack-a-mole "test for null bp" or "which -ENODATA do we really
> >> mean in this function?" throughout the xattr code, my first thought is
> >> that we should simply map -ENODATA in lower level read functions back to
> >> -EIO, which is unambiguous, even if we lose the nuance of the underlying
> >> error code. (The block device probably already squawked.) Thoughts?
> >
> > Uhhhh where does this ENODATA come from? Is it the block layer?
> >
> > $ git grep -w ENODATA block/
> > block/blk-core.c:146: [BLK_STS_MEDIUM] = { -ENODATA, "critical medium" },
> >
>
> I had been working on a test case for this based on dmerror, but It
> never successfully triggered this, since dmerror returned EIO.
>
> At least it didn't until Eric got creative mapping the the error back to
> ENODATA.
>
> I'll was in the process of turning this into an xfstest based on your
> tests/xfs/556, I'll reply here with it in case its useful to anyone, but
> it would need to be modified to somehow inject ENODATA into the return
> path.
>
> Don
>
>
>
> It should work with a systemtap to map the error, though I think Eric
> was considering alternatives.
>
>
>
>
>
>
