Re: SCSI eats error from flush failure during hot plug

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2014-06-26 at 08:00 -0700, Christoph Hellwig wrote:
> On Thu, Jun 26, 2014 at 10:02:47AM -0400, James Bottomley wrote:
> > Yes, but think what you're proposing.  Every block command with a
> > special setup goes via scsi_setup_blk_pc_cmnd() because they usually
> > have to translate to something special.
> 
> That logic is backwards.  The "special" commands use
> scsi_setup_blk_pc_cmnd because that seemed the easiest we to do it
> when they were introduce.  Looking back they should have just called
> scsi_init_io directly instead of doing enough setup to pretend to be
> BLOCK_PC enough to use scsi_setup_blk_pc_cmnd().
> 
> > If we did what you propose,
> > every time we add one, we'd have to modify these five places in the code
> > plus the setup ... it's a bit insane plus a maintenance nightmare.
> 
> It's not.  They need to be treated special because they _are_ special.

If there's a problem outside the normal processing then it needs to be
solved once, not once for every new special block command type.

> If you look at the command types there's a pretty clear difference
> between BLOCK_PC and everything else:
> 
> 			BLOCK_PC	read/write	discard/flush/write_same
> need to generate CDB	no		yes		yes
> calls into the ULD	no		yes		yes
> needs error handling	no		yes		yes
> passes sense data up	yes		no		no

Right, but look what we do for the specials.  The only difference is the
ULD has a hook to generate the CDB.  For everything else we follow the
BLOCK_PC path, including error handling and sense data processing.

> so they surely aren;t like BLOCK_PC, and treating them like BLOCK_PC
> would require major changes to our architecture.  Not that I'm overly
> happy with them being _FS requests either - they probably should have
> a cmd_type assigned for each of them and come down from the block
> layer set up that way.  I'll look into that once I get a little bit of
> time.

OK, we'll us this as a fix for the bug.  If you come up with something
more elegant we can replace it.

James