Re: [PATCH 6/6] scsi-disk: Check for supported commands

[Hannes Reinecke <hare@suse.de>]

On 07/26/2011 03:07 PM, Kevin Wolf wrote:
> Am 22.07.2011 16:51, schrieb Hannes Reinecke:
>> Not every command is support for any device type. This patch adds
>> a check for rejecting unsupported commands.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> ---
>>   hw/scsi-disk.c |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 103 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index ae2c157..8ad90c0 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -361,13 +361,107 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
>>       return scsi_build_sense(s->sense, outbuf, len, len>  14);
>>   }
>>
>> +#define GENERIC_CMD (uint32_t)0xFFFFFFFF
>> +#define DISK_CMD (1u<<  TYPE_DISK)
>> +#define TAPE_CMD (1u<<  TYPE_TAPE)
>> +#define PRINTER_CMD (1u<<  TYPE_PRINTER)
>> +#define PROCESSOR_CMD (1u<<  TYPE_PROCESSOR)
>> +#define WORM_CMD (1u<<  TYPE_WORM)
>> +#define ROM_CMD (1u<<  TYPE_ROM)
>> +#define SCANNER_CMD (1u<<  TYPE_SCANNER)
>> +#define MOD_CMD (1u<<  TYPE_MOD)
>> +#define MEDIUM_CHANGER_CMD (1u<<  TYPE_MEDIUM_CHANGER)
>> +#define ARRAY_CMD (1u<<  TYPE_STORAGE_ARRAY)
>> +#define ENCLOSURE_CMD (1u<<  TYPE_ENCLOSURE)
>> +#define RBC_CMD (1u<<  TYPE_RBC)
>> +#define OSD_CMD (1u<<  TYPE_OSD)
>> +
>> +#define NO_ROM_CMD (GENERIC_CMD | ~ROM_CMD)
>> +
>> +uint32_t scsi_cmd_table[0x100] = {
>> +    [TEST_UNIT_READY]           = GENERIC_CMD,
>> +    [REWIND]                    = TAPE_CMD,
>> +    [REQUEST_SENSE]             = GENERIC_CMD,
>> +    [FORMAT_UNIT]               = DISK_CMD|ROM_CMD,
>> +    [READ_BLOCK_LIMITS]         = TAPE_CMD,
>> +    [REASSIGN_BLOCKS]           = DISK_CMD|WORM_CMD|MOD_CMD,
>> +    [READ_6]                    = DISK_CMD|TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [WRITE_6]                   = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD,
>> +    [READ_REVERSE]              = TAPE_CMD,
>> +    [WRITE_FILEMARKS]           = TAPE_CMD,
>> +    [SPACE]                     = TAPE_CMD,
>> +    [INQUIRY]                   = GENERIC_CMD,
>> +    [MODE_SELECT]               = GENERIC_CMD,
>> +    [RESERVE]                   = TAPE_CMD|PRINTER_CMD,
>> +    [RELEASE]                   = TAPE_CMD|PRINTER_CMD,
>> +    [ERASE]                     = TAPE_CMD,
>> +    [MODE_SENSE]                = GENERIC_CMD,
>> +    [START_STOP]                = GENERIC_CMD,
>> +    [RECEIVE_DIAGNOSTIC]        = GENERIC_CMD,
>> +    [SEND_DIAGNOSTIC]           = GENERIC_CMD,
>> +    [ALLOW_MEDIUM_REMOVAL]      = GENERIC_CMD,
>> +    [READ_CAPACITY_10]          = DISK_CMD|WORM_CMD|MOD_CMD,
>> +    [READ_10]                   = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [WRITE_10]                  = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [SEEK_10]                   = TAPE_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [WRITE_VERIFY_10]           = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [VERIFY_10]                 = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [READ_POSITION]             = TAPE_CMD,
>> +    [SYNCHRONIZE_CACHE]         = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD|RBC_CMD,
>> +    [WRITE_BUFFER]              = GENERIC_CMD,
>> +    [READ_BUFFER]               = GENERIC_CMD,
>> +    [READ_LONG_10]              = DISK_CMD|WORM_CMD|MOD_CMD,
>> +    [WRITE_LONG_10]             = DISK_CMD|WORM_CMD|MOD_CMD,
>> +    [WRITE_SAME_10]             = DISK_CMD,
>> +    [UNMAP]                     = DISK_CMD,
>> +    [READ_TOC]                  = ROM_CMD,
>> +    [REPORT_DENSITY_SUPPORT]    = TAPE_CMD,
>> +    [GET_CONFIGURATION]         = ROM_CMD,
>> +    [LOG_SELECT]                = GENERIC_CMD,
>> +    [LOG_SENSE]                 = GENERIC_CMD,
>> +    [MODE_SELECT_10]            = GENERIC_CMD,
>> +    [RESERVE_10]                = PRINTER_CMD,
>> +    [RELEASE_10]                = PRINTER_CMD,
>> +    [MODE_SENSE_10]             = GENERIC_CMD,
>> +    [PERSISTENT_RESERVE_IN]     = GENERIC_CMD,
>> +    [PERSISTENT_RESERVE_OUT]    = GENERIC_CMD,
>> +    [VARLENGTH_CDB]             = OSD_CMD,
>> +    [WRITE_FILEMARKS_16]        = TAPE_CMD,
>> +    [ATA_PASSTHROUGH]           = DISK_CMD|ROM_CMD|RBC_CMD,
>> +    [READ_16]                   = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
>> +    [WRITE_16]                  = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
>> +    [WRITE_VERIFY_16]           = DISK_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
>> +    [SYNCHRONIZE_CACHE_16]      = DISK_CMD|TAPE_CMD|WORM_CMD|MOD_CMD|RBC_CMD,
>> +    [LOCATE_16]                 = TAPE_CMD,
>> +    [WRITE_SAME_16]             = DISK_CMD|TAPE_CMD,
>> +    [SERVICE_ACTION_IN]         = GENERIC_CMD,
>> +    [REPORT_LUNS]               = NO_ROM_CMD,
>> +    [BLANK]                     = ROM_CMD,
>> +    [MAINTENANCE_IN]            = NO_ROM_CMD,
>> +    [MAINTENANCE_OUT]           = NO_ROM_CMD,
>> +    [MOVE_MEDIUM]               = MEDIUM_CHANGER_CMD,
>> +    [LOAD_UNLOAD]               = ROM_CMD|MEDIUM_CHANGER_CMD,
>> +    [READ_12]                   = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [WRITE_12]                  = DISK_CMD|WORM_CMD|ROM_CMD|MOD_CMD,
>> +    [WRITE_VERIFY_12]           = DISK_CMD|WORM_CMD|MOD_CMD,
>> +    [VERIFY_12]                 = DISK_CMD|WORM_CMD|MOD_CMD,
>> +    [READ_ELEMENT_STATUS]       = WORM_CMD|MOD_CMD,
>> +    [SET_CD_SPEED]              = ROM_CMD
>> +};
>> +
>> +static bool scsi_command_supported(uint8_t scsi_type, uint8_t cmd)
>> +{
>> +    uint32_t mask = (1u<<  scsi_type);
>> +    return scsi_cmd_table[cmd]&  mask;
>> +}
>> +
>>   static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
>>   {
>>       SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
>>       int buflen = 0;
>>
>>       if (req->cmd.buf[1]&  0x2) {
>> -        /* Command support data - optional, not implemented */
>> +        /* Command support data - obsolete */
>>           BADF("optional INQUIRY command support request not implemented\n");
>>           return -1;
>>       }
>> @@ -1032,6 +1126,14 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
>>               return 0;
>>           }
>>       }
>> +    if (!scsi_command_supported(command, s->qdev.type)) {
>
> Wrong parameter order?
>
Don't be picky :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)