On Wed, 2008-11-26 at 15:03 -0500, Alan Stern wrote: > On Wed, 26 Nov 2008, James Bottomley wrote: > > > On Mon, 2008-11-03 at 15:56 -0500, Alan Stern wrote: > > > This patch (as1142b) consolidates a lot of repetitious code in > > > scsi_io_completion(). It also fixes a few comments. Most > > > importantly, however, it clearly distinguishes among the three sorts > > > of retries that can be done when a command fails to complete: > > > OK, how about this as an update to the patch. It corrects several > > things: > > > > 1. For several error conditions, we would now print the sense twice > > in slightly different ways, so unify the location of sense > > printing. > > 2. I added more descriptions to actual failure conditions for > > better debugging > > 3. according to spec, ABORTED_COMMAND is supposed to be retried > > (except on DIF failure). Our old behaviour of erroring it looks > > to be a bug. > > 4. I'd prefer not to default initialise the action variable because > > that ensures that every leg of the error handler has an > > associated action and the compiler will warn if someone later > > accidentally misses one or removes one. > > This looks very good. I'm pleased you didn't find anything actually > wrong with the original patch aside from the ABORTED COMMAND handling. > > I was going to suggest adding a description to the ILLEGAL REQUEST > case. But that case arises normally under various circumstances, so > perhaps it wouldn't be appropriate. In fact, do you really want to > print out the result and sense data every time that case occurs? I think it's OK. I thought some of the CD probing routines triggered illegal requests, but I can't seem to see it on my test machines (it could be I have the wrong type of CD, though). > > It also looks like > > ... ? it also looks like I didn't finish my sentence. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
