Re: drivers/block/scsi_ioctl problem

[Brian King <brking@us.ibm.com>]

Jens Axboe wrote:
> On Wed, Dec 15 2004, Brian King wrote:
> 
>>Brian King wrote:
>>
>>>>It was added to be able to find out which opcode was rejected and thus
>>>>caused an application malfunction. It dumps the specific opcode only
>>>>once, is that such a huge problem?
>>>
>>>
>>>Ok. I didn't realize that it was only dumped once. That's not as bad. I 
>>>guess it might be better if it were only dumped if the op actually failed.
>>
>>How about something like this...
> 
> 
>>Currently if an SG_IO ioctl is issued to a block device with an
>>unknown opcode, an error is logged. This error is logged regardless
>>the device successfully processes the command or not. This results in
>>an error getting logged for each unknown opcode ever issued. This patch
>>changes this policy and only prints the unknown opcode if the command
>>fails.
> 
> 
> How about just moving it after the CAP_SYS_RAWIO check, so that it only
> logs for commands we explicitly reject? The purpose of the opcode dump
> is not to log failed commands (the app should do that itself), but
> mainly why a specific opcode was _not_ sent to the drive. This is
> something you cannot always directly gauge, unless you have the source
> for the app. And even then you have to dig :-)

I guess that would make more sense. Patch looks good.

Thanks

-Brian

> 
> ===== drivers/block/scsi_ioctl.c 1.62 vs edited =====
> --- 1.62/drivers/block/scsi_ioctl.c	2004-11-20 01:50:56 +01:00
> +++ edited/drivers/block/scsi_ioctl.c	2004-12-16 07:30:39 +01:00
> @@ -199,14 +199,14 @@
>  			return 0;
>  	}
>  
> +	/* And root can do any command.. */
> +	if (capable(CAP_SYS_RAWIO))
> +		return 0;
> +
>  	if (!(type & CMD_WARNED)) {
>  		cmd_type[cmd[0]] = CMD_WARNED;
>  		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
>  	}
> -
> -	/* And root can do any command.. */
> -	if (capable(CAP_SYS_RAWIO))
> -		return 0;
>  
>  	/* Otherwise fail it with an "Operation not permitted" */
>  	return -EPERM;
> 
> 


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center