[dm-devel] Re: [PATCH 2/3] multipath: Add new SPC-3 ALUA hardware handler

[Hannes Reinecke]

Arne Redlich wrote:
> Hi Hannes,
> 
> hare at suse.de (Hannes Reinecke) writes:
> 
>> This adds a new SPC-3 ALUA hardware handler for multipathing.
>>
>> Signed-off-by: Hannes Reinecke <hare at suse.de>
> 
> <snip>
> 
>> +#define TPGS_STATE_OPTIMIZED		0x0
>> +#define TPGS_STATE_NONOPTIMIZED		0x1
>> +#define TPGS_STATE_STANDBY		0x2
>> +#define TPGS_STATE_UNAVAILABLE		0x3
>> +#define TPGS_STATE_OFFLINE		0xe
> 
> SPC-3 (at least the draft, rev 23 I'm looking at) doesn't know an
> 'Offline' state - I think it's a SPC-4 feature. So maybe 'Unavailable'
> should be interpreted as path failure as well / instead?
> 
It is SPC-4. And if the state is unavailable, we can try to
activate it; spc3r23 says (5.8.2.4.5):
Therefore it may not be possible to transition from this state to
either the active/optimized, active/non-optimized or standby states.

But consequently it _may_ be possible, so we should at least try.
If that fails (ie if SET TARGET PORT GROUPS returns
an error) we'll fail the path anyway.
No harm in trying.

> <snip>
> 
>> +/*
>> + * SET TARGET GROUP STATES endio handler
>> + *
>> + * We only have to test here if we should resubmit the command;
>> + * any other error is assumed as a failure.
>> + * Maybe we should analyze the sensebuffer here, too.
>> + */
>> +static void stpg_endio(struct request *req, int error)
>> +{
>> +	struct hw_handler *hwh = req->end_io_data;
>> +	struct alua_handler *h = hwh->context;
>> +
>> +	switch(host_byte(error)) {
>> +	case DID_BUS_BUSY:
>> +		if (!h->retry)
>> +			break;
>> +		h->retry--;
>> +	case DID_REQUEUE:
>> +	case DID_IMM_RETRY:
>> +		dm_enqueue_hw_workq(hwh);
>> +		goto done;
>> +	}
>> +
>> +	if (had_failures(req, error)) {
>> +		if (h->tpgs & TPGS_MODE_IMPLICIT) {
>> +			/* Ignore errors; the array will figure it out */
>> +			DMWARN("%s: stpg failed %x, disabling explicit mode",
>> +			       h->path->dev->name, error);
>> +			h->tpgs &= ~TPGS_MODE_EXPLICIT;
>> +			dm_enqueue_hw_workq(hwh);
>> +		} else {
>> +			DMWARN("%s: stpg failed %x, disable path",
>> +			       h->path->dev->name, error);
>> +			dm_pg_init_complete(h->path, MP_FAIL_PATH);
>> +		}
>> +	} else {
>> +		DMWARN("%s: port group %02x new state %c",
>> +		       h->path->dev->name, h->group_id,
>> +		       print_alua_state(h->state) );
>> +		dm_pg_init_complete(h->path, 0);
> 
> Hmmm, maybe I'm just missing something so CMIIW, but I think the PG
> state should be retrieved once more before finally calling
> dm_pg_init_complete(), because the target might return the STPG command
> before the transition has completed (SPC-3, 6.31). This could confuse
> application clients?
> 
Hmm. Spec isn't exactly clear here. One would expect that these arrays
would have set the T_SUP bit in REPORT TARGET PORT GROUPS, and set the
ALUA state to 'TRANSITIONING' accordingly. But we catch the relevant
sense codes as per SPC-3, so we should retry it properly.

And it's not that I've actually seen an array implementing this, so
it's a bit academic currently. 

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare at suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)