Arne Redlich wrote: > Hi Hannes, > > hare@xxxxxxx (Hannes Reinecke) writes: > >> This adds a new SPC-3 ALUA hardware handler for multipathing. >> >> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> > > <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@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel