Re: [PATCH 4/6] st: call scsi_set_medium_removal directly

[Christoph Hellwig <hch@lst.de>]

On Thu, Nov 06, 2014 at 11:54:41PM +0000, Elliott, Robert (Server Storage) wrote:
> A few comments spawned by this (and patch 5/6):
> 
> 1. Although PREVENT ALLOW MEDIUM REMOVAL was a generic
> command defined in SPC-2, In 2006 T10 proposal
> 06-248r1 changed it to be a command-set specific 
> command for SPC-3.  MMC, SSC, SBC, and SMC had slight
> differences and the groups wouldn't agree on a converged
> definition.
> 
> For example:
> * SSC and SBC only define two of the bit combinations 
>   in byte 4 bits 1:0 - allowed and prevented
> * MMC defines all four combinations of byte 4 bits 1:0 -
>   unlocked, locked, persistent allow, and persistent prevent
> * SMC follows SSC/SBC but adds a PREEMPT bit in 
>   byte 4 bit 7
> 
> So, relying on one SCSI-wide scsi_set_medium_removal
> call could theoretically be a problem (though not yet,
> since none of those extra features are used).

The scsi_set_medium_removal is fairly trivial.  For the unlikely case
that we are going to make use of these additional features we'll
need to split it.

> 2. Reviewing the callers for scsi_set_medium_removal,
> I noticed sd.c sd_release ignores the return value 
> from scsi_set_medium_removal (like many others).
> 
> Its comment says:
> *      Returns 0.
> but it is a void function so doesn't really return anything:
> static void sd_release(struct gendisk *disk, fmode_t mode)

Right.  That's actually a fairly recent change from Al, before
that the ->release method for block devices had a return value,
which we then always iŋnored higher up.

> 3. Reviewing the callers, st_release has an initialized
> result variable but does nothing else with it but return it:
> 	int result = 0;
> 	...
> 	return result;
> 
> It ignores the do_door_lock -> scsi_set_medium_removal
> result (like many others).

The st driver defintively could use an audit for error code propagation.
--
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