Re: [PATCH] sg.c to set direction more reliably (was Re: [PATCH] fusion update to current APIs)

[Jeff Garzik <jgarzik@pobox.com>]

Jeremy Higdon wrote:
> +static int
> +sg_direction(char *cmnd)
> +{
> +	switch (cmnd[0]) {
> +	/*  _DATA_OUT commands	*/
> +	case WRITE_6:		case WRITE_10:		case WRITE_12:
> +	case WRITE_16:
> +	case WRITE_LONG:	case WRITE_SAME:	case WRITE_BUFFER:
> +	case WRITE_VERIFY:	case WRITE_VERIFY_12:
> +	case COMPARE:		case COPY:		case COPY_VERIFY:
> +	case SEARCH_EQUAL:	case SEARCH_HIGH:	case SEARCH_LOW:
> +	case SEARCH_EQUAL_12:	case SEARCH_HIGH_12:	case SEARCH_LOW_12:
> +	case MODE_SELECT:	case MODE_SELECT_10:	case LOG_SELECT:
> +	case SEND_DIAGNOSTIC:	case CHANGE_DEFINITION: case UPDATE_BLOCK:
> +	case SET_WINDOW:	case MEDIUM_SCAN:	case SEND_VOLUME_TAG:
> +	case REASSIGN_BLOCKS:
> +	case PERSISTENT_RESERVE_OUT:
> +	case 0xea:
> +	case 0xa3:
> +		return SG_DXFER_TO_DEV;
> +
> +	/*  No data transfer commands  */
> +	case SEEK_6:		case SEEK_10:
> +	case RESERVE:		case RELEASE:
> +	case TEST_UNIT_READY:
> +	case START_STOP:
> +	case ALLOW_MEDIUM_REMOVAL:
> +		return SG_DXFER_NONE;
> +
> +	/*  Conditional data transfer commands	*/
> +	case FORMAT_UNIT:
> +		if (cmnd[1] & 0x10)	/* FmtData (data out phase)? */
> +			return SG_DXFER_TO_DEV;
> +		else
> +			return SG_DXFER_NONE;
> +
> +	case VERIFY:
> +		if (cmnd[1] & 0x02)	/* VERIFY:BYTCHK (data out phase)? */
> +			return SG_DXFER_TO_DEV;
> +		else
> +			return SG_DXFER_NONE;
> +
> +	case RESERVE_10:
> +		if (cmnd[1] & 0x03)  /* RESERVE:{LongID|Extent} (data out phase)? */
> +			return SG_DXFER_TO_DEV;
> +		else
> +			return SG_DXFER_NONE;
> +
> +	/*  Must be data _IN!  */
> +	default:
> +		return SG_DXFER_FROM_DEV;
> +	}
> +}


This is definitely moving backwards from the direction we want to go.

The entity creating the SCSI command needs to specify data direction, 
_not_ tables hardcoded into the kernel.  Bart recently removed such a 
table from IDE.

These tables are fundamentally broken anyway, due to vendor-reserved 
commands where the data directions are not specified, but simply "known" 
by the submittor.

If /dev/sg's userland interface does not permit userland to provide the 
data direction, then let's just consider it broken and move to Axboe's 
bsg post-haste.

	Jeff