From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59133) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlhvW-0002i4-Ol for qemu-devel@nongnu.org; Tue, 26 Jul 2011 09:44:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QlhvS-0003SC-5C for qemu-devel@nongnu.org; Tue, 26 Jul 2011 09:44:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2766) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlhvR-0003S8-U7 for qemu-devel@nongnu.org; Tue, 26 Jul 2011 09:43:58 -0400 Message-ID: <4E2EC54C.5090208@redhat.com> Date: Tue, 26 Jul 2011 15:46:52 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311346277-32329-1-git-send-email-hare@suse.de> <1311346277-32329-7-git-send-email-hare@suse.de> In-Reply-To: <1311346277-32329-7-git-send-email-hare@suse.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/6] scsi-disk: Check for supported commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke Cc: Alexander Graf , qemu-devel@nongnu.org, kvm@vger.kernel.org, Markus Armbruster 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 We do emulate SEEK (6), but it's not in your scsi_cmd_table at all. > --- > 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, My spec says that MMC doesn't support READ(6) > + [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, What's the reason for allowing this for tape, but not e.g. for disks? It's marked as obsolete for both (and optional for quite a few other types) > + [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, ROM_CMD, too > + [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, Tape doesn't have SEEK(10) in the spec. > + [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, Tape doesn't have this. > + [LOCATE_16] = TAPE_CMD, > + [WRITE_SAME_16] = DISK_CMD|TAPE_CMD, Again not for tape in my spec. Kevin