| Message ID | 1430743343-47174-3-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/04/15 14:42, Hannes Reinecke wrote: > Non-disk devices should be ignored when detecting > ALUA capabilities. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/scsi/device_handler/scsi_dh_alua.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index e418849..834e80f 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -320,6 +320,23 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h) > { > int err = SCSI_DH_OK; > > + if (scsi_is_wlun(sdev->lun)) { > + h->tpgs = TPGS_MODE_NONE; > + sdev_printk(KERN_INFO, sdev, > + "%s: disable for WLUN\n", > + ALUA_DH_NAME); > + return SCSI_DH_DEV_UNSUPP; > + } > + if (sdev->type != TYPE_DISK && > + sdev->type != TYPE_RBC && > + sdev->type != TYPE_OSD) { > + h->tpgs = TPGS_MODE_NONE; > + sdev_printk(KERN_INFO, sdev, > + "%s: disable for non-disk devices\n", > + ALUA_DH_NAME); > + return SCSI_DH_DEV_UNSUPP; > + } > + > h->tpgs = scsi_device_tpgs(sdev); > switch (h->tpgs) { > case TPGS_MODE_EXPLICIT|TPGS_MODE_IMPLICIT: > Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 04, 2015 at 02:42:08PM +0200, Hannes Reinecke wrote: > Non-disk devices should be ignored when detecting > ALUA capabilities. Hmm. I don't think we should even attach the alua handler in this case, e.g. refine the check in scsi_dh_find_driver. But then again I don't remember how the spec wording actually disallows this, and a quick grep of SPC-4 can't find anything either. Can you put a reference to the spec section that disallows attachment to non-disk devices into the code? > + if (sdev->type != TYPE_DISK && > + sdev->type != TYPE_RBC && > + sdev->type != TYPE_OSD) { And does it really allow RBC (but not ZBC)? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/11/2015 08:46 AM, Christoph Hellwig wrote: > On Mon, May 04, 2015 at 02:42:08PM +0200, Hannes Reinecke wrote: >> Non-disk devices should be ignored when detecting >> ALUA capabilities. > > Hmm. I don't think we should even attach the alua handler in this case, > e.g. refine the check in scsi_dh_find_driver. But then again I don't > remember how the spec wording actually disallows this, and a quick grep > of SPC-4 can't find anything either. Can you put a reference to the spec > section that disallows attachment to non-disk devices into the code? > The spec wording doesn't explicitly disallowing you from doing so (unless you're using referrals). But in realiter we're using the ALUA device handler only when multipathing is used, which in turn runs on disk devices only. So we can limit ourselves to disk devices, too. Everything else (like tapes or tape changers) will have to implement their own logic anyway; tape support in multipathing with or without ALUA is too horrible to contemplate ... >> + if (sdev->type != TYPE_DISK && >> + sdev->type != TYPE_RBC && >> + sdev->type != TYPE_OSD) { > > And does it really allow RBC (but not ZBC)? > Thought so, but I'll have to cross-check. Maybe it's time to implement a 'scsi_is_disk_type()' macro... Cheers, Hannes
On Mon, May 11, 2015 at 12:25:51PM +0200, Hannes Reinecke wrote: > The spec wording doesn't explicitly disallowing you from doing so > (unless you're using referrals). > But in realiter we're using the ALUA device handler only when > multipathing is used, which in turn runs on disk devices only. > So we can limit ourselves to disk devices, too. > > Everything else (like tapes or tape changers) will have to > implement their own logic anyway; tape support in multipathing > with or without ALUA is too horrible to contemplate ... So what is the problem you're trying to solve here? We were attaching to other nodes and so far I haven't heard about that being a problem. Basically we'll disallow I/O for offlines paths to non-disk devices once the ALUA handler is attached, which actually seems useful even without a multipath handler on top. > >> + if (sdev->type != TYPE_DISK && > >> + sdev->type != TYPE_RBC && > >> + sdev->type != TYPE_OSD) { > > > > And does it really allow RBC (but not ZBC)? > > > Thought so, but I'll have to cross-check. > Maybe it's time to implement a 'scsi_is_disk_type()' macro... If the reason to disallow anything that we can't use dm-mpath on both OSD and ZBC in it's current form are excluded. RBC would be included, although I doubt there is a multi ported RBC device available. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/11/2015 01:34 PM, Christoph Hellwig wrote: > On Mon, May 11, 2015 at 12:25:51PM +0200, Hannes Reinecke wrote: >> The spec wording doesn't explicitly disallowing you from doing so >> (unless you're using referrals). >> But in realiter we're using the ALUA device handler only when >> multipathing is used, which in turn runs on disk devices only. >> So we can limit ourselves to disk devices, too. >> >> Everything else (like tapes or tape changers) will have to >> implement their own logic anyway; tape support in multipathing >> with or without ALUA is too horrible to contemplate ... > > So what is the problem you're trying to solve here? We were attaching > to other nodes and so far I haven't heard about that being a problem. > > Basically we'll disallow I/O for offlines paths to non-disk devices > once the ALUA handler is attached, which actually seems useful > even without a multipath handler on top. > I've had far too many issues with ALUA attaching to eg enclosure devices; some enclosures have the habit of returning 'NOT READY, CAUSE NOT REPORTABLE' when sending an RTPG to it. Other (broken) devices set the 'TPGS' bit in the inquiry data, but fail to implement support for everything else. Currently we're just doing a retry for these cases, and would have to wait for the timeout to kick in. We _could_ implement a dedicated error handling here, but then we're pretty much guaranteed to miss the odd case. So I've decided to just skip them altogether. If you insist we could remove this patch, and straighten out error handling here. >>>> + if (sdev->type != TYPE_DISK && >>>> + sdev->type != TYPE_RBC && >>>> + sdev->type != TYPE_OSD) { >>> >>> And does it really allow RBC (but not ZBC)? >>> >> Thought so, but I'll have to cross-check. >> Maybe it's time to implement a 'scsi_is_disk_type()' macro... > > If the reason to disallow anything that we can't use dm-mpath on > both OSD and ZBC in it's current form are excluded. RBC would > be included, although I doubt there is a multi ported RBC > device available. > As said above; we _could_ drop this patch and update error handling, but then we'll risk of having more bug reports due to invalid ALUA implementations. Might be worth it, though, as then we could get in touch with the respective vendors. But then, maybe not, as not every vendor is willing to listen to us... I don't really have a fixed opinion here; either way is fine with me. Cheers, Hannes
On Mon, May 11, 2015 at 01:55:15PM +0200, Hannes Reinecke wrote: > As said above; we _could_ drop this patch and update error > handling, but then we'll risk of having more bug reports > due to invalid ALUA implementations. > Might be worth it, though, as then we could get in touch > with the respective vendors. But then, maybe not, as not > every vendor is willing to listen to us... > > I don't really have a fixed opinion here; either way is > fine with me. For now mayeb reject a everything but TYPE_SBC and add a comment explaining why, i.e. a short form of the reasons in this mail. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c index e418849..834e80f 100644 --- a/drivers/scsi/device_handler/scsi_dh_alua.c +++ b/drivers/scsi/device_handler/scsi_dh_alua.c @@ -320,6 +320,23 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h) { int err = SCSI_DH_OK; + if (scsi_is_wlun(sdev->lun)) { + h->tpgs = TPGS_MODE_NONE; + sdev_printk(KERN_INFO, sdev, + "%s: disable for WLUN\n", + ALUA_DH_NAME); + return SCSI_DH_DEV_UNSUPP; + } + if (sdev->type != TYPE_DISK && + sdev->type != TYPE_RBC && + sdev->type != TYPE_OSD) { + h->tpgs = TPGS_MODE_NONE; + sdev_printk(KERN_INFO, sdev, + "%s: disable for non-disk devices\n", + ALUA_DH_NAME); + return SCSI_DH_DEV_UNSUPP; + } + h->tpgs = scsi_device_tpgs(sdev); switch (h->tpgs) { case TPGS_MODE_EXPLICIT|TPGS_MODE_IMPLICIT:
Non-disk devices should be ignored when detecting ALUA capabilities. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/device_handler/scsi_dh_alua.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)