Re: [PATCH 5/8] scsi: scsi-multipath: Add basic ALUA support

[John Garry <john.g.garry@oracle.com>]

On 14/03/2026 04:48, Benjamin Marzinski wrote:
>> +			ext_hdr_unsupp = true;
>> +			goto retry;
>> +		}
>> +		/*
>> +		 * If the array returns with 'ALUA state transition'
>> +		 * sense code here it cannot return RTPG data during
>> +		 * transition. So set the state to 'transitioning' directly.
>> +		 */
>> +		if (sense_hdr.sense_key == NOT_READY &&
>> +		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a)
>> +			goto out;
> This check is odd. First, we don't set the state to 'transitioning' like
> the comment says. We don't set alua_state at all, which ends up meaning
> that it stays as 0 (SCSI_ACCESS_STATE_OPTIMAL). It seems like we should
> explicitly set it and make the comment reflect that, if just to aid
> understanding of the logic.

Yeah, I have to admit that this is all bodged the a bit, as 
transitioning is not properly handled. Instead of "basic" ALUA support, 
this really is limited ALUA support.

I am now looking at a way to have a core scsi ALUA driver to handle all 
of this, but it is challenging as we need to continue ALUA DH support.

> 
> Nitpick: Also, this is the only place where we goto out. All the other
> checks individually free the buffer and return directly. I realize that
> all the other checks that exit early are errors, but it seems like we
> could just return a variable at the end of the function.

Sure, I can pay attention to this. However, as mentioned above, I am 
experimenting with moving any ALUA specifics into scsi core code.

> 
>> +
>> +		/*
>> +		 * Retry on any other UNIT ATTENTION occurred.
>> +		 */
>> +		if (sense_hdr.sense_key == UNIT_ATTENTION) {
>> +			scsi_print_sense_hdr(sdev, __func__, &sense_hdr);
>> +			kfree(buff);
>> +			return -EAGAIN;
>> +		}
> If we get a UNIT ATTENTION, we end up failing scsi_mpath_dev_alloc(),
> not retrying. Aside from the comment being wrong, it seems very brittle
> to fail here, just because we got a UNIT ATTENTION.

Ack

Thanks,
John