Re: [PATCH RFC] xfs: remap block layer ENODATA read errors to EIO

[Donald Douwsma <ddouwsma@redhat.com>]

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.
>>
>> 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.