* [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions
2011-07-22 12:31 [Qemu-devel] [PATCH 0/3] Check for supported SCSI commands Hannes Reinecke
@ 2011-07-22 12:31 ` Hannes Reinecke
2011-07-22 14:07 ` Markus Armbruster
2011-07-22 12:31 ` [Qemu-devel] [PATCH 2/3] scsi-disk: Remove drive_kind Hannes Reinecke
2011-07-22 12:31 ` [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands Hannes Reinecke
2 siblings, 1 reply; 8+ messages in thread
From: Hannes Reinecke @ 2011-07-22 12:31 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Hannes Reinecke, Markus Armbruster, kvm,
Alexander Graf
Adding some missing command definitions and update the existing ones.
No functional change.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
hw/scsi-bus.c | 71 ++++++++++++++++++++++++++++++----------------------
hw/scsi-defs.h | 60 ++++++++++++++++++++++++++++----------------
hw/scsi-disk.c | 29 ++++++++-------------
hw/scsi-generic.c | 2 +-
4 files changed, 91 insertions(+), 71 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 8b1a412..24a1298 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -232,24 +232,24 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
case RELEASE:
case ERASE:
case ALLOW_MEDIUM_REMOVAL:
- case VERIFY:
+ case VERIFY_10:
case SEEK_10:
case SYNCHRONIZE_CACHE:
case LOCK_UNLOCK_CACHE:
case LOAD_UNLOAD:
case SET_CD_SPEED:
case SET_LIMITS:
- case WRITE_LONG:
+ case WRITE_LONG_10:
case MOVE_MEDIUM:
case UPDATE_BLOCK:
req->cmd.xfer = 0;
break;
case MODE_SENSE:
break;
- case WRITE_SAME:
+ case WRITE_SAME_10:
req->cmd.xfer = 1;
break;
- case READ_CAPACITY:
+ case READ_CAPACITY_10:
req->cmd.xfer = 8;
break;
case READ_BLOCK_LIMITS:
@@ -265,7 +265,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
req->cmd.xfer *= 8;
break;
case WRITE_10:
- case WRITE_VERIFY:
+ case WRITE_VERIFY_10:
case WRITE_6:
case WRITE_12:
case WRITE_VERIFY_12:
@@ -325,7 +325,7 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
switch (req->cmd.buf[0]) {
case WRITE_6:
case WRITE_10:
- case WRITE_VERIFY:
+ case WRITE_VERIFY_10:
case WRITE_12:
case WRITE_VERIFY_12:
case WRITE_16:
@@ -345,15 +345,13 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
case SEARCH_HIGH:
case SEARCH_LOW:
case UPDATE_BLOCK:
- case WRITE_LONG:
- case WRITE_SAME:
+ case WRITE_LONG_10:
+ case WRITE_SAME_10:
case SEARCH_HIGH_12:
case SEARCH_EQUAL_12:
case SEARCH_LOW_12:
- case SET_WINDOW:
case MEDIUM_SCAN:
case SEND_VOLUME_TAG:
- case WRITE_LONG_2:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
req->cmd.mode = SCSI_XFER_TO_DEV;
@@ -517,8 +515,7 @@ static const char *scsi_command_name(uint8_t cmd)
{
static const char *names[] = {
[ TEST_UNIT_READY ] = "TEST_UNIT_READY",
- [ REZERO_UNIT ] = "REZERO_UNIT",
- /* REWIND and REZERO_UNIT use the same operation code */
+ [ REWIND ] = "REWIND",
[ REQUEST_SENSE ] = "REQUEST_SENSE",
[ FORMAT_UNIT ] = "FORMAT_UNIT",
[ READ_BLOCK_LIMITS ] = "READ_BLOCK_LIMITS",
@@ -543,14 +540,13 @@ static const char *scsi_command_name(uint8_t cmd)
[ RECEIVE_DIAGNOSTIC ] = "RECEIVE_DIAGNOSTIC",
[ SEND_DIAGNOSTIC ] = "SEND_DIAGNOSTIC",
[ ALLOW_MEDIUM_REMOVAL ] = "ALLOW_MEDIUM_REMOVAL",
-
[ SET_WINDOW ] = "SET_WINDOW",
- [ READ_CAPACITY ] = "READ_CAPACITY",
+ [ READ_CAPACITY_10 ] = "READ_CAPACITY_10",
[ READ_10 ] = "READ_10",
[ WRITE_10 ] = "WRITE_10",
[ SEEK_10 ] = "SEEK_10",
- [ WRITE_VERIFY ] = "WRITE_VERIFY",
- [ VERIFY ] = "VERIFY",
+ [ WRITE_VERIFY_10 ] = "WRITE_VERIFY_10",
+ [ VERIFY_10 ] = "VERIFY_10",
[ SEARCH_HIGH ] = "SEARCH_HIGH",
[ SEARCH_EQUAL ] = "SEARCH_EQUAL",
[ SEARCH_LOW ] = "SEARCH_LOW",
@@ -566,11 +562,14 @@ static const char *scsi_command_name(uint8_t cmd)
[ WRITE_BUFFER ] = "WRITE_BUFFER",
[ READ_BUFFER ] = "READ_BUFFER",
[ UPDATE_BLOCK ] = "UPDATE_BLOCK",
- [ READ_LONG ] = "READ_LONG",
- [ WRITE_LONG ] = "WRITE_LONG",
+ [ READ_LONG_10 ] = "READ_LONG_10",
+ [ WRITE_LONG_10 ] = "WRITE_LONG_10",
[ CHANGE_DEFINITION ] = "CHANGE_DEFINITION",
- [ WRITE_SAME ] = "WRITE_SAME",
+ [ WRITE_SAME_10 ] = "WRITE_SAME_10",
+ [ UNMAP ] = "UNMAP",
[ READ_TOC ] = "READ_TOC",
+ [ REPORT_DENSITY_SUPPORT ] = "REPORT_DENSITY_SUPPORT",
+ [ GET_CONFIGURATION ] = "GET_CONFIGURATION",
[ LOG_SELECT ] = "LOG_SELECT",
[ LOG_SENSE ] = "LOG_SENSE",
[ MODE_SELECT_10 ] = "MODE_SELECT_10",
@@ -579,27 +578,39 @@ static const char *scsi_command_name(uint8_t cmd)
[ MODE_SENSE_10 ] = "MODE_SENSE_10",
[ PERSISTENT_RESERVE_IN ] = "PERSISTENT_RESERVE_IN",
[ PERSISTENT_RESERVE_OUT ] = "PERSISTENT_RESERVE_OUT",
+ [ WRITE_FILEMARKS_16 ] = "WRITE_FILEMARKS_16",
+ [ EXTENDED_COPY ] = "EXTENDED_COPY",
+ [ ATA_PASSTHROUGH ] = "ATA_PASSTHROUGH",
+ [ ACCESS_CONTROL_IN ] = "ACCESS_CONTROL_IN",
+ [ ACCESS_CONTROL_OUT ] = "ACCESS_CONTROL_OUT",
+ [ READ_16 ] = "READ_16",
+ [ COMPARE_AND_WRITE ] = "COMPARE_AND_WRITE",
+ [ WRITE_16 ] = "WRITE_16",
+ [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
+ [ VERIFY_16 ] = "VERIFY_16",
+ [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16",
+ [ LOCATE_16 ] = "LOCATE_16",
+ [ WRITE_SAME_16 ] = "WRITE_SAME_16",
+ [ ERASE_16 ] = "ERASE_16",
+ [ SERVICE_ACTION_IN ] = "SERVICE_ACTION_IN",
+ [ WRITE_LONG_16 ] = "WRITE_LONG_16",
+ [ REPORT_LUNS ] = "REPORT_LUNS",
+ [ BLANK ] = "BLANK",
+ [ MAINTENANCE_IN ] = "MAINTENANCE_IN",
+ [ MAINTENANCE_OUT ] = "MAINTENANCE_OUT",
[ MOVE_MEDIUM ] = "MOVE_MEDIUM",
+ [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
[ READ_12 ] = "READ_12",
[ WRITE_12 ] = "WRITE_12",
[ WRITE_VERIFY_12 ] = "WRITE_VERIFY_12",
+ [ VERIFY_12 ] = "VERIFY_12",
[ SEARCH_HIGH_12 ] = "SEARCH_HIGH_12",
[ SEARCH_EQUAL_12 ] = "SEARCH_EQUAL_12",
[ SEARCH_LOW_12 ] = "SEARCH_LOW_12",
[ READ_ELEMENT_STATUS ] = "READ_ELEMENT_STATUS",
[ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG",
- [ WRITE_LONG_2 ] = "WRITE_LONG_2",
-
- [ REPORT_DENSITY_SUPPORT ] = "REPORT_DENSITY_SUPPORT",
- [ GET_CONFIGURATION ] = "GET_CONFIGURATION",
- [ READ_16 ] = "READ_16",
- [ WRITE_16 ] = "WRITE_16",
- [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
- [ SERVICE_ACTION_IN ] = "SERVICE_ACTION_IN",
- [ REPORT_LUNS ] = "REPORT_LUNS",
- [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
+ [ READ_DEFECT_DATA ] = "READ_DEFECT_DATA",
[ SET_CD_SPEED ] = "SET_CD_SPEED",
- [ BLANK ] = "BLANK",
};
if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL)
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 413cce0..458a797 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -26,6 +26,7 @@
#define TEST_UNIT_READY 0x00
#define REZERO_UNIT 0x01
+#define REWIND 0x01
#define REQUEST_SENSE 0x03
#define FORMAT_UNIT 0x04
#define READ_BLOCK_LIMITS 0x05
@@ -48,14 +49,14 @@
#define RECEIVE_DIAGNOSTIC 0x1c
#define SEND_DIAGNOSTIC 0x1d
#define ALLOW_MEDIUM_REMOVAL 0x1e
-
#define SET_WINDOW 0x24
-#define READ_CAPACITY 0x25
+#define READ_CAPACITY_10 0x25
#define READ_10 0x28
#define WRITE_10 0x2a
#define SEEK_10 0x2b
-#define WRITE_VERIFY 0x2e
-#define VERIFY 0x2f
+#define LOCATE_10 0x2b
+#define WRITE_VERIFY_10 0x2e
+#define VERIFY_10 0x2f
#define SEARCH_HIGH 0x30
#define SEARCH_EQUAL 0x31
#define SEARCH_LOW 0x32
@@ -71,11 +72,14 @@
#define WRITE_BUFFER 0x3b
#define READ_BUFFER 0x3c
#define UPDATE_BLOCK 0x3d
-#define READ_LONG 0x3e
-#define WRITE_LONG 0x3f
+#define READ_LONG_10 0x3e
+#define WRITE_LONG_10 0x3f
#define CHANGE_DEFINITION 0x40
-#define WRITE_SAME 0x41
+#define WRITE_SAME_10 0x41
+#define UNMAP 0x42
#define READ_TOC 0x43
+#define REPORT_DENSITY_SUPPORT 0x44
+#define GET_CONFIGURATION 0x46
#define LOG_SELECT 0x4c
#define LOG_SENSE 0x4d
#define MODE_SELECT_10 0x55
@@ -84,32 +88,40 @@
#define MODE_SENSE_10 0x5a
#define PERSISTENT_RESERVE_IN 0x5e
#define PERSISTENT_RESERVE_OUT 0x5f
+#define VARLENGTH_CDB 0x7f
+#define WRITE_FILEMARKS_16 0x80
+#define EXTENDED_COPY 0x83
+#define ATA_PASSTHROUGH 0x85
+#define ACCESS_CONTROL_IN 0x86
+#define ACCESS_CONTROL_OUT 0x87
+#define READ_16 0x88
+#define COMPARE_AND_WRITE 0x89
+#define WRITE_16 0x8a
+#define WRITE_VERIFY_16 0x8e
+#define VERIFY_16 0x8f
+#define SYNCHRONIZE_CACHE_16 0x91
+#define LOCATE_16 0x92
#define WRITE_SAME_16 0x93
+#define ERASE_16 0x93
+#define SERVICE_ACTION_IN 0x9e
+#define WRITE_LONG_16 0x9f
+#define REPORT_LUNS 0xa0
+#define BLANK 0xa1
#define MAINTENANCE_IN 0xa3
#define MAINTENANCE_OUT 0xa4
#define MOVE_MEDIUM 0xa5
+#define LOAD_UNLOAD 0xa6
#define READ_12 0xa8
#define WRITE_12 0xaa
#define WRITE_VERIFY_12 0xae
+#define VERIFY_12 0xaf
#define SEARCH_HIGH_12 0xb0
#define SEARCH_EQUAL_12 0xb1
#define SEARCH_LOW_12 0xb2
#define READ_ELEMENT_STATUS 0xb8
#define SEND_VOLUME_TAG 0xb6
-#define WRITE_LONG_2 0xea
-
-/* from hw/scsi-generic.c */
-#define REWIND 0x01
-#define REPORT_DENSITY_SUPPORT 0x44
-#define GET_CONFIGURATION 0x46
-#define READ_16 0x88
-#define WRITE_16 0x8a
-#define WRITE_VERIFY_16 0x8e
-#define SERVICE_ACTION_IN 0x9e
-#define REPORT_LUNS 0xa0
-#define LOAD_UNLOAD 0xa6
-#define SET_CD_SPEED 0xbb
-#define BLANK 0xa1
+#define READ_DEFECT_DATA_12 0xb7
+#define SET_CD_SPEED 0xbb
/*
* SAM Status codes
@@ -154,6 +166,7 @@
#define TYPE_DISK 0x00
#define TYPE_TAPE 0x01
+#define TYPE_PRINTER 0x02
#define TYPE_PROCESSOR 0x03 /* HP scanners use this */
#define TYPE_WORM 0x04 /* Treated as ROM by our system */
#define TYPE_ROM 0x05
@@ -161,6 +174,9 @@
#define TYPE_MOD 0x07 /* Magneto-optical disk -
* - treated as TYPE_DISK */
#define TYPE_MEDIUM_CHANGER 0x08
-#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
+#define TYPE_STORAGE_ARRAY 0x0c /* Storage array device */
+#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
+#define TYPE_RBC 0x0e /* Simplified Direct-Access Device */
+#define TYPE_OSD 0x11 /* Object-storage Device */
#define TYPE_NO_LUN 0x7f
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 05d14ab..2f7e0c9 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -836,7 +836,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
case TEST_UNIT_READY:
if (!bdrv_is_inserted(s->bs))
goto not_ready;
- break;
+ break;
case REQUEST_SENSE:
if (req->cmd.xfer < 4)
goto illegal_request;
@@ -848,7 +848,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
buflen = scsi_disk_emulate_inquiry(req, outbuf);
if (buflen < 0)
goto illegal_request;
- break;
+ break;
case MODE_SENSE:
case MODE_SENSE_10:
buflen = scsi_disk_emulate_mode_sense(req, outbuf);
@@ -881,14 +881,14 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
/* load/eject medium */
bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
}
- break;
+ break;
case ALLOW_MEDIUM_REMOVAL:
bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
- break;
+ break;
case READ_CAPACITY:
/* The normal LEN field for this command is zero. */
- memset(outbuf, 0, 8);
- bdrv_get_geometry(s->bs, &nb_sectors);
+ memset(outbuf, 0, 8);
+ bdrv_get_geometry(s->bs, &nb_sectors);
if (!nb_sectors)
goto not_ready;
nb_sectors /= s->cluster_size;
@@ -908,7 +908,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
outbuf[6] = s->cluster_size * 2;
outbuf[7] = 0;
buflen = 8;
- break;
+ break;
case SYNCHRONIZE_CACHE:
ret = bdrv_flush(s->bs);
if (ret < 0) {
@@ -970,13 +970,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
outbuf[3] = 8;
buflen = 16;
break;
- case VERIFY:
- break;
- case REZERO_UNIT:
- DPRINTF("Rezero Unit\n");
- if (!bdrv_is_inserted(s->bs)) {
- goto not_ready;
- }
+ case VERIFY_10:
break;
default:
scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
@@ -1052,14 +1046,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
case RELEASE_10:
case START_STOP:
case ALLOW_MEDIUM_REMOVAL:
- case READ_CAPACITY:
+ case READ_CAPACITY_10:
case SYNCHRONIZE_CACHE:
case READ_TOC:
case GET_CONFIGURATION:
case SERVICE_ACTION_IN:
case REPORT_LUNS:
- case VERIFY:
- case REZERO_UNIT:
+ case VERIFY_10:
rc = scsi_disk_emulate_command(r, outbuf);
if (rc < 0) {
return 0;
@@ -1082,7 +1075,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
case WRITE_10:
case WRITE_12:
case WRITE_16:
- case WRITE_VERIFY:
+ case WRITE_VERIFY_10:
case WRITE_VERIFY_12:
case WRITE_VERIFY_16:
len = r->req.cmd.xfer / s->qdev.blocksize;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 90345a7..17e83c7 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -406,7 +406,7 @@ static int get_blocksize(BlockDriverState *bdrv)
memset(cmd, 0, sizeof(cmd));
memset(buf, 0, sizeof(buf));
- cmd[0] = READ_CAPACITY;
+ cmd[0] = READ_CAPACITY_10;
memset(&io_header, 0, sizeof(io_header));
io_header.interface_id = 'S';
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions
2011-07-22 12:31 ` [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions Hannes Reinecke
@ 2011-07-22 14:07 ` Markus Armbruster
2011-07-22 14:14 ` Hannes Reinecke
0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2011-07-22 14:07 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Kevin Wolf, qemu-devel, kvm, Alexander Graf
Hannes Reinecke <hare@suse.de> writes:
> Adding some missing command definitions and update the existing ones.
> No functional change.
Add: LOCATE_10, UNMAP, VARLENGTH_CDB, WRITE_FILEMARKS_16, EXTENDED_COPY,
ATA_PASSTHROUGH, ACCESS_CONTROL_IN, ACCESS_CONTROL_OUT,
COMPARE_AND_WRITE, VERIFY_16, SYNCHRONIZE_CACHE_16, LOCATE_16, ERASE_16,
WRITE_LONG_16, VERIFY_12, READ_DEFECT_DATA_12.
Rename: READ_CAPACITY, WRITE_VERIFY, VERIFY, READ_LONG, WRITE_LONG,
WRITE_SAME to &_10.
Delete: WRITE_LONG_2
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> hw/scsi-bus.c | 71 ++++++++++++++++++++++++++++++----------------------
> hw/scsi-defs.h | 60 ++++++++++++++++++++++++++++----------------
> hw/scsi-disk.c | 29 ++++++++-------------
> hw/scsi-generic.c | 2 +-
> 4 files changed, 91 insertions(+), 71 deletions(-)
>
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 8b1a412..24a1298 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -232,24 +232,24 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
> case RELEASE:
> case ERASE:
> case ALLOW_MEDIUM_REMOVAL:
> - case VERIFY:
> + case VERIFY_10:
> case SEEK_10:
> case SYNCHRONIZE_CACHE:
> case LOCK_UNLOCK_CACHE:
> case LOAD_UNLOAD:
> case SET_CD_SPEED:
> case SET_LIMITS:
> - case WRITE_LONG:
> + case WRITE_LONG_10:
> case MOVE_MEDIUM:
> case UPDATE_BLOCK:
> req->cmd.xfer = 0;
> break;
> case MODE_SENSE:
> break;
> - case WRITE_SAME:
> + case WRITE_SAME_10:
> req->cmd.xfer = 1;
> break;
> - case READ_CAPACITY:
> + case READ_CAPACITY_10:
> req->cmd.xfer = 8;
> break;
> case READ_BLOCK_LIMITS:
> @@ -265,7 +265,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
> req->cmd.xfer *= 8;
> break;
> case WRITE_10:
> - case WRITE_VERIFY:
> + case WRITE_VERIFY_10:
> case WRITE_6:
> case WRITE_12:
> case WRITE_VERIFY_12:
> @@ -325,7 +325,7 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
> switch (req->cmd.buf[0]) {
> case WRITE_6:
> case WRITE_10:
> - case WRITE_VERIFY:
> + case WRITE_VERIFY_10:
> case WRITE_12:
> case WRITE_VERIFY_12:
> case WRITE_16:
> @@ -345,15 +345,13 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
> case SEARCH_HIGH:
> case SEARCH_LOW:
> case UPDATE_BLOCK:
> - case WRITE_LONG:
> - case WRITE_SAME:
> + case WRITE_LONG_10:
> + case WRITE_SAME_10:
> case SEARCH_HIGH_12:
> case SEARCH_EQUAL_12:
> case SEARCH_LOW_12:
> - case SET_WINDOW:
I figure this is "no functional change" because all users of this
function reject SET_WINDOW elsewhere.
> case MEDIUM_SCAN:
> case SEND_VOLUME_TAG:
> - case WRITE_LONG_2:
Same here.
> case PERSISTENT_RESERVE_OUT:
> case MAINTENANCE_OUT:
> req->cmd.mode = SCSI_XFER_TO_DEV;
> @@ -517,8 +515,7 @@ static const char *scsi_command_name(uint8_t cmd)
> {
> static const char *names[] = {
> [ TEST_UNIT_READY ] = "TEST_UNIT_READY",
> - [ REZERO_UNIT ] = "REZERO_UNIT",
> - /* REWIND and REZERO_UNIT use the same operation code */
> + [ REWIND ] = "REWIND",
"No functional change" because it's used only for scsi_req_print(),
which is unused.
> [ REQUEST_SENSE ] = "REQUEST_SENSE",
> [ FORMAT_UNIT ] = "FORMAT_UNIT",
> [ READ_BLOCK_LIMITS ] = "READ_BLOCK_LIMITS",
> @@ -543,14 +540,13 @@ static const char *scsi_command_name(uint8_t cmd)
> [ RECEIVE_DIAGNOSTIC ] = "RECEIVE_DIAGNOSTIC",
> [ SEND_DIAGNOSTIC ] = "SEND_DIAGNOSTIC",
> [ ALLOW_MEDIUM_REMOVAL ] = "ALLOW_MEDIUM_REMOVAL",
> -
> [ SET_WINDOW ] = "SET_WINDOW",
> - [ READ_CAPACITY ] = "READ_CAPACITY",
> + [ READ_CAPACITY_10 ] = "READ_CAPACITY_10",
> [ READ_10 ] = "READ_10",
> [ WRITE_10 ] = "WRITE_10",
> [ SEEK_10 ] = "SEEK_10",
New LOCATE_10 missing.
> - [ WRITE_VERIFY ] = "WRITE_VERIFY",
> - [ VERIFY ] = "VERIFY",
> + [ WRITE_VERIFY_10 ] = "WRITE_VERIFY_10",
> + [ VERIFY_10 ] = "VERIFY_10",
> [ SEARCH_HIGH ] = "SEARCH_HIGH",
> [ SEARCH_EQUAL ] = "SEARCH_EQUAL",
> [ SEARCH_LOW ] = "SEARCH_LOW",
> @@ -566,11 +562,14 @@ static const char *scsi_command_name(uint8_t cmd)
> [ WRITE_BUFFER ] = "WRITE_BUFFER",
> [ READ_BUFFER ] = "READ_BUFFER",
> [ UPDATE_BLOCK ] = "UPDATE_BLOCK",
> - [ READ_LONG ] = "READ_LONG",
> - [ WRITE_LONG ] = "WRITE_LONG",
> + [ READ_LONG_10 ] = "READ_LONG_10",
> + [ WRITE_LONG_10 ] = "WRITE_LONG_10",
> [ CHANGE_DEFINITION ] = "CHANGE_DEFINITION",
> - [ WRITE_SAME ] = "WRITE_SAME",
> + [ WRITE_SAME_10 ] = "WRITE_SAME_10",
> + [ UNMAP ] = "UNMAP",
> [ READ_TOC ] = "READ_TOC",
> + [ REPORT_DENSITY_SUPPORT ] = "REPORT_DENSITY_SUPPORT",
> + [ GET_CONFIGURATION ] = "GET_CONFIGURATION",
> [ LOG_SELECT ] = "LOG_SELECT",
> [ LOG_SENSE ] = "LOG_SENSE",
> [ MODE_SELECT_10 ] = "MODE_SELECT_10",
> @@ -579,27 +578,39 @@ static const char *scsi_command_name(uint8_t cmd)
> [ MODE_SENSE_10 ] = "MODE_SENSE_10",
> [ PERSISTENT_RESERVE_IN ] = "PERSISTENT_RESERVE_IN",
> [ PERSISTENT_RESERVE_OUT ] = "PERSISTENT_RESERVE_OUT",
New VARLENGTH_CDB missing.
> + [ WRITE_FILEMARKS_16 ] = "WRITE_FILEMARKS_16",
> + [ EXTENDED_COPY ] = "EXTENDED_COPY",
> + [ ATA_PASSTHROUGH ] = "ATA_PASSTHROUGH",
> + [ ACCESS_CONTROL_IN ] = "ACCESS_CONTROL_IN",
> + [ ACCESS_CONTROL_OUT ] = "ACCESS_CONTROL_OUT",
> + [ READ_16 ] = "READ_16",
> + [ COMPARE_AND_WRITE ] = "COMPARE_AND_WRITE",
> + [ WRITE_16 ] = "WRITE_16",
> + [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
> + [ VERIFY_16 ] = "VERIFY_16",
> + [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16",
> + [ LOCATE_16 ] = "LOCATE_16",
> + [ WRITE_SAME_16 ] = "WRITE_SAME_16",
> + [ ERASE_16 ] = "ERASE_16",
> + [ SERVICE_ACTION_IN ] = "SERVICE_ACTION_IN",
> + [ WRITE_LONG_16 ] = "WRITE_LONG_16",
> + [ REPORT_LUNS ] = "REPORT_LUNS",
> + [ BLANK ] = "BLANK",
> + [ MAINTENANCE_IN ] = "MAINTENANCE_IN",
> + [ MAINTENANCE_OUT ] = "MAINTENANCE_OUT",
Fixes missing WRITE_SAME_16, MAINTENANCE_IN, MAINTENANCE_OUT.
> [ MOVE_MEDIUM ] = "MOVE_MEDIUM",
> + [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
> [ READ_12 ] = "READ_12",
> [ WRITE_12 ] = "WRITE_12",
> [ WRITE_VERIFY_12 ] = "WRITE_VERIFY_12",
> + [ VERIFY_12 ] = "VERIFY_12",
> [ SEARCH_HIGH_12 ] = "SEARCH_HIGH_12",
> [ SEARCH_EQUAL_12 ] = "SEARCH_EQUAL_12",
> [ SEARCH_LOW_12 ] = "SEARCH_LOW_12",
> [ READ_ELEMENT_STATUS ] = "READ_ELEMENT_STATUS",
> [ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG",
> - [ WRITE_LONG_2 ] = "WRITE_LONG_2",
> -
> - [ REPORT_DENSITY_SUPPORT ] = "REPORT_DENSITY_SUPPORT",
> - [ GET_CONFIGURATION ] = "GET_CONFIGURATION",
> - [ READ_16 ] = "READ_16",
> - [ WRITE_16 ] = "WRITE_16",
> - [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
> - [ SERVICE_ACTION_IN ] = "SERVICE_ACTION_IN",
> - [ REPORT_LUNS ] = "REPORT_LUNS",
> - [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
> + [ READ_DEFECT_DATA ] = "READ_DEFECT_DATA",
READ_DEFECT_DATA_12!
> [ SET_CD_SPEED ] = "SET_CD_SPEED",
> - [ BLANK ] = "BLANK",
> };
>
> if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL)
> diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
> index 413cce0..458a797 100644
> --- a/hw/scsi-defs.h
> +++ b/hw/scsi-defs.h
> @@ -26,6 +26,7 @@
>
> #define TEST_UNIT_READY 0x00
> #define REZERO_UNIT 0x01
> +#define REWIND 0x01
> #define REQUEST_SENSE 0x03
> #define FORMAT_UNIT 0x04
> #define READ_BLOCK_LIMITS 0x05
> @@ -48,14 +49,14 @@
> #define RECEIVE_DIAGNOSTIC 0x1c
> #define SEND_DIAGNOSTIC 0x1d
> #define ALLOW_MEDIUM_REMOVAL 0x1e
> -
> #define SET_WINDOW 0x24
> -#define READ_CAPACITY 0x25
> +#define READ_CAPACITY_10 0x25
> #define READ_10 0x28
> #define WRITE_10 0x2a
> #define SEEK_10 0x2b
> -#define WRITE_VERIFY 0x2e
> -#define VERIFY 0x2f
> +#define LOCATE_10 0x2b
> +#define WRITE_VERIFY_10 0x2e
> +#define VERIFY_10 0x2f
> #define SEARCH_HIGH 0x30
> #define SEARCH_EQUAL 0x31
> #define SEARCH_LOW 0x32
> @@ -71,11 +72,14 @@
> #define WRITE_BUFFER 0x3b
> #define READ_BUFFER 0x3c
> #define UPDATE_BLOCK 0x3d
> -#define READ_LONG 0x3e
> -#define WRITE_LONG 0x3f
> +#define READ_LONG_10 0x3e
> +#define WRITE_LONG_10 0x3f
> #define CHANGE_DEFINITION 0x40
> -#define WRITE_SAME 0x41
> +#define WRITE_SAME_10 0x41
> +#define UNMAP 0x42
> #define READ_TOC 0x43
> +#define REPORT_DENSITY_SUPPORT 0x44
> +#define GET_CONFIGURATION 0x46
> #define LOG_SELECT 0x4c
> #define LOG_SENSE 0x4d
> #define MODE_SELECT_10 0x55
> @@ -84,32 +88,40 @@
> #define MODE_SENSE_10 0x5a
> #define PERSISTENT_RESERVE_IN 0x5e
> #define PERSISTENT_RESERVE_OUT 0x5f
> +#define VARLENGTH_CDB 0x7f
> +#define WRITE_FILEMARKS_16 0x80
> +#define EXTENDED_COPY 0x83
> +#define ATA_PASSTHROUGH 0x85
> +#define ACCESS_CONTROL_IN 0x86
> +#define ACCESS_CONTROL_OUT 0x87
> +#define READ_16 0x88
> +#define COMPARE_AND_WRITE 0x89
> +#define WRITE_16 0x8a
> +#define WRITE_VERIFY_16 0x8e
> +#define VERIFY_16 0x8f
> +#define SYNCHRONIZE_CACHE_16 0x91
> +#define LOCATE_16 0x92
> #define WRITE_SAME_16 0x93
> +#define ERASE_16 0x93
> +#define SERVICE_ACTION_IN 0x9e
> +#define WRITE_LONG_16 0x9f
> +#define REPORT_LUNS 0xa0
> +#define BLANK 0xa1
> #define MAINTENANCE_IN 0xa3
> #define MAINTENANCE_OUT 0xa4
> #define MOVE_MEDIUM 0xa5
> +#define LOAD_UNLOAD 0xa6
> #define READ_12 0xa8
> #define WRITE_12 0xaa
> #define WRITE_VERIFY_12 0xae
> +#define VERIFY_12 0xaf
> #define SEARCH_HIGH_12 0xb0
> #define SEARCH_EQUAL_12 0xb1
> #define SEARCH_LOW_12 0xb2
> #define READ_ELEMENT_STATUS 0xb8
> #define SEND_VOLUME_TAG 0xb6
> -#define WRITE_LONG_2 0xea
> -
> -/* from hw/scsi-generic.c */
> -#define REWIND 0x01
> -#define REPORT_DENSITY_SUPPORT 0x44
> -#define GET_CONFIGURATION 0x46
> -#define READ_16 0x88
> -#define WRITE_16 0x8a
> -#define WRITE_VERIFY_16 0x8e
> -#define SERVICE_ACTION_IN 0x9e
> -#define REPORT_LUNS 0xa0
> -#define LOAD_UNLOAD 0xa6
> -#define SET_CD_SPEED 0xbb
> -#define BLANK 0xa1
> +#define READ_DEFECT_DATA_12 0xb7
> +#define SET_CD_SPEED 0xbb
>
> /*
> * SAM Status codes
> @@ -154,6 +166,7 @@
>
> #define TYPE_DISK 0x00
> #define TYPE_TAPE 0x01
> +#define TYPE_PRINTER 0x02
> #define TYPE_PROCESSOR 0x03 /* HP scanners use this */
> #define TYPE_WORM 0x04 /* Treated as ROM by our system */
> #define TYPE_ROM 0x05
> @@ -161,6 +174,9 @@
> #define TYPE_MOD 0x07 /* Magneto-optical disk -
> * - treated as TYPE_DISK */
> #define TYPE_MEDIUM_CHANGER 0x08
> -#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
> +#define TYPE_STORAGE_ARRAY 0x0c /* Storage array device */
> +#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
> +#define TYPE_RBC 0x0e /* Simplified Direct-Access Device */
> +#define TYPE_OSD 0x11 /* Object-storage Device */
> #define TYPE_NO_LUN 0x7f
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 05d14ab..2f7e0c9 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -836,7 +836,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
> case TEST_UNIT_READY:
> if (!bdrv_is_inserted(s->bs))
> goto not_ready;
> - break;
> + break;
> case REQUEST_SENSE:
> if (req->cmd.xfer < 4)
> goto illegal_request;
> @@ -848,7 +848,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
> buflen = scsi_disk_emulate_inquiry(req, outbuf);
> if (buflen < 0)
> goto illegal_request;
> - break;
> + break;
> case MODE_SENSE:
> case MODE_SENSE_10:
> buflen = scsi_disk_emulate_mode_sense(req, outbuf);
> @@ -881,14 +881,14 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
> /* load/eject medium */
> bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
> }
> - break;
> + break;
> case ALLOW_MEDIUM_REMOVAL:
> bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
> - break;
> + break;
> case READ_CAPACITY:
> /* The normal LEN field for this command is zero. */
> - memset(outbuf, 0, 8);
> - bdrv_get_geometry(s->bs, &nb_sectors);
> + memset(outbuf, 0, 8);
> + bdrv_get_geometry(s->bs, &nb_sectors);
> if (!nb_sectors)
> goto not_ready;
> nb_sectors /= s->cluster_size;
> @@ -908,7 +908,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
> outbuf[6] = s->cluster_size * 2;
> outbuf[7] = 0;
> buflen = 8;
> - break;
> + break;
> case SYNCHRONIZE_CACHE:
> ret = bdrv_flush(s->bs);
> if (ret < 0) {
> @@ -970,13 +970,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
> outbuf[3] = 8;
> buflen = 16;
> break;
> - case VERIFY:
> - break;
> - case REZERO_UNIT:
> - DPRINTF("Rezero Unit\n");
> - if (!bdrv_is_inserted(s->bs)) {
> - goto not_ready;
> - }
Functional change, the commit message lies. Separate patch?
Matching update to scsi_cmd_table[] missing.
Looks like we could purge REZERO_UNIT elsewhere, too.
> + case VERIFY_10:
> break;
> default:
> scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
> @@ -1052,14 +1046,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
> case RELEASE_10:
> case START_STOP:
> case ALLOW_MEDIUM_REMOVAL:
> - case READ_CAPACITY:
> + case READ_CAPACITY_10:
> case SYNCHRONIZE_CACHE:
> case READ_TOC:
> case GET_CONFIGURATION:
> case SERVICE_ACTION_IN:
> case REPORT_LUNS:
> - case VERIFY:
> - case REZERO_UNIT:
> + case VERIFY_10:
> rc = scsi_disk_emulate_command(r, outbuf);
> if (rc < 0) {
> return 0;
> @@ -1082,7 +1075,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
> case WRITE_10:
> case WRITE_12:
> case WRITE_16:
> - case WRITE_VERIFY:
> + case WRITE_VERIFY_10:
> case WRITE_VERIFY_12:
> case WRITE_VERIFY_16:
> len = r->req.cmd.xfer / s->qdev.blocksize;
> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> index 90345a7..17e83c7 100644
> --- a/hw/scsi-generic.c
> +++ b/hw/scsi-generic.c
> @@ -406,7 +406,7 @@ static int get_blocksize(BlockDriverState *bdrv)
>
> memset(cmd, 0, sizeof(cmd));
> memset(buf, 0, sizeof(buf));
> - cmd[0] = READ_CAPACITY;
> + cmd[0] = READ_CAPACITY_10;
>
> memset(&io_header, 0, sizeof(io_header));
> io_header.interface_id = 'S';
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions
2011-07-22 14:07 ` Markus Armbruster
@ 2011-07-22 14:14 ` Hannes Reinecke
0 siblings, 0 replies; 8+ messages in thread
From: Hannes Reinecke @ 2011-07-22 14:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Kevin Wolf, qemu-devel, kvm, Alexander Graf
On 07/22/2011 04:07 PM, Markus Armbruster wrote:
> Hannes Reinecke<hare@suse.de> writes:
>
>> Adding some missing command definitions and update the existing ones.
>> No functional change.
>
> Add: LOCATE_10, UNMAP, VARLENGTH_CDB, WRITE_FILEMARKS_16, EXTENDED_COPY,
> ATA_PASSTHROUGH, ACCESS_CONTROL_IN, ACCESS_CONTROL_OUT,
> COMPARE_AND_WRITE, VERIFY_16, SYNCHRONIZE_CACHE_16, LOCATE_16, ERASE_16,
> WRITE_LONG_16, VERIFY_12, READ_DEFECT_DATA_12.
>
> Rename: READ_CAPACITY, WRITE_VERIFY, VERIFY, READ_LONG, WRITE_LONG,
> WRITE_SAME to&_10.
>
> Delete: WRITE_LONG_2
>
OK.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> ---
>> hw/scsi-bus.c | 71 ++++++++++++++++++++++++++++++----------------------
>> hw/scsi-defs.h | 60 ++++++++++++++++++++++++++++----------------
>> hw/scsi-disk.c | 29 ++++++++-------------
>> hw/scsi-generic.c | 2 +-
>> 4 files changed, 91 insertions(+), 71 deletions(-)
>>
>> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
>> index 8b1a412..24a1298 100644
>> --- a/hw/scsi-bus.c
>> +++ b/hw/scsi-bus.c
>> @@ -232,24 +232,24 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
>> case RELEASE:
>> case ERASE:
>> case ALLOW_MEDIUM_REMOVAL:
>> - case VERIFY:
>> + case VERIFY_10:
>> case SEEK_10:
>> case SYNCHRONIZE_CACHE:
>> case LOCK_UNLOCK_CACHE:
>> case LOAD_UNLOAD:
>> case SET_CD_SPEED:
>> case SET_LIMITS:
>> - case WRITE_LONG:
>> + case WRITE_LONG_10:
>> case MOVE_MEDIUM:
>> case UPDATE_BLOCK:
>> req->cmd.xfer = 0;
>> break;
>> case MODE_SENSE:
>> break;
>> - case WRITE_SAME:
>> + case WRITE_SAME_10:
>> req->cmd.xfer = 1;
>> break;
>> - case READ_CAPACITY:
>> + case READ_CAPACITY_10:
>> req->cmd.xfer = 8;
>> break;
>> case READ_BLOCK_LIMITS:
>> @@ -265,7 +265,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
>> req->cmd.xfer *= 8;
>> break;
>> case WRITE_10:
>> - case WRITE_VERIFY:
>> + case WRITE_VERIFY_10:
>> case WRITE_6:
>> case WRITE_12:
>> case WRITE_VERIFY_12:
>> @@ -325,7 +325,7 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
>> switch (req->cmd.buf[0]) {
>> case WRITE_6:
>> case WRITE_10:
>> - case WRITE_VERIFY:
>> + case WRITE_VERIFY_10:
>> case WRITE_12:
>> case WRITE_VERIFY_12:
>> case WRITE_16:
>> @@ -345,15 +345,13 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
>> case SEARCH_HIGH:
>> case SEARCH_LOW:
>> case UPDATE_BLOCK:
>> - case WRITE_LONG:
>> - case WRITE_SAME:
>> + case WRITE_LONG_10:
>> + case WRITE_SAME_10:
>> case SEARCH_HIGH_12:
>> case SEARCH_EQUAL_12:
>> case SEARCH_LOW_12:
>> - case SET_WINDOW:
>
> I figure this is "no functional change" because all users of this
> function reject SET_WINDOW elsewhere.
>
Yes, you are correct. Will be doing another patch with removing
references to obsolete commands SET_WINDOW and WRITE_LONG_2.
>> case MEDIUM_SCAN:
>> case SEND_VOLUME_TAG:
>> - case WRITE_LONG_2:
>
> Same here.
>
Indeed.
>> case PERSISTENT_RESERVE_OUT:
>> case MAINTENANCE_OUT:
>> req->cmd.mode = SCSI_XFER_TO_DEV;
>> @@ -517,8 +515,7 @@ static const char *scsi_command_name(uint8_t cmd)
>> {
>> static const char *names[] = {
>> [ TEST_UNIT_READY ] = "TEST_UNIT_READY",
>> - [ REZERO_UNIT ] = "REZERO_UNIT",
>> - /* REWIND and REZERO_UNIT use the same operation code */
>> + [ REWIND ] = "REWIND",
>
> "No functional change" because it's used only for scsi_req_print(),
> which is unused.
>
See above.
>> [ REQUEST_SENSE ] = "REQUEST_SENSE",
>> [ FORMAT_UNIT ] = "FORMAT_UNIT",
>> [ READ_BLOCK_LIMITS ] = "READ_BLOCK_LIMITS",
>> @@ -543,14 +540,13 @@ static const char *scsi_command_name(uint8_t cmd)
>> [ RECEIVE_DIAGNOSTIC ] = "RECEIVE_DIAGNOSTIC",
>> [ SEND_DIAGNOSTIC ] = "SEND_DIAGNOSTIC",
>> [ ALLOW_MEDIUM_REMOVAL ] = "ALLOW_MEDIUM_REMOVAL",
>> -
>> [ SET_WINDOW ] = "SET_WINDOW",
>> - [ READ_CAPACITY ] = "READ_CAPACITY",
>> + [ READ_CAPACITY_10 ] = "READ_CAPACITY_10",
>> [ READ_10 ] = "READ_10",
>> [ WRITE_10 ] = "WRITE_10",
>> [ SEEK_10 ] = "SEEK_10",
>
> New LOCATE_10 missing.
>
OK.
>> - [ WRITE_VERIFY ] = "WRITE_VERIFY",
>> - [ VERIFY ] = "VERIFY",
>> + [ WRITE_VERIFY_10 ] = "WRITE_VERIFY_10",
>> + [ VERIFY_10 ] = "VERIFY_10",
>> [ SEARCH_HIGH ] = "SEARCH_HIGH",
>> [ SEARCH_EQUAL ] = "SEARCH_EQUAL",
>> [ SEARCH_LOW ] = "SEARCH_LOW",
>> @@ -566,11 +562,14 @@ static const char *scsi_command_name(uint8_t cmd)
>> [ WRITE_BUFFER ] = "WRITE_BUFFER",
>> [ READ_BUFFER ] = "READ_BUFFER",
>> [ UPDATE_BLOCK ] = "UPDATE_BLOCK",
>> - [ READ_LONG ] = "READ_LONG",
>> - [ WRITE_LONG ] = "WRITE_LONG",
>> + [ READ_LONG_10 ] = "READ_LONG_10",
>> + [ WRITE_LONG_10 ] = "WRITE_LONG_10",
>> [ CHANGE_DEFINITION ] = "CHANGE_DEFINITION",
>> - [ WRITE_SAME ] = "WRITE_SAME",
>> + [ WRITE_SAME_10 ] = "WRITE_SAME_10",
>> + [ UNMAP ] = "UNMAP",
>> [ READ_TOC ] = "READ_TOC",
>> + [ REPORT_DENSITY_SUPPORT ] = "REPORT_DENSITY_SUPPORT",
>> + [ GET_CONFIGURATION ] = "GET_CONFIGURATION",
>> [ LOG_SELECT ] = "LOG_SELECT",
>> [ LOG_SENSE ] = "LOG_SENSE",
>> [ MODE_SELECT_10 ] = "MODE_SELECT_10",
>> @@ -579,27 +578,39 @@ static const char *scsi_command_name(uint8_t cmd)
>> [ MODE_SENSE_10 ] = "MODE_SENSE_10",
>> [ PERSISTENT_RESERVE_IN ] = "PERSISTENT_RESERVE_IN",
>> [ PERSISTENT_RESERVE_OUT ] = "PERSISTENT_RESERVE_OUT",
>
> New VARLENGTH_CDB missing.
>
Intentionally. VARLENGHT_CDB is used for all sorts of things, so
I think we should be printing that one out separately.
Same goes for MAINTENANCE_(IN|OUT) and SERVICE_ACTION_(IN|OUT).
>> + [ WRITE_FILEMARKS_16 ] = "WRITE_FILEMARKS_16",
>> + [ EXTENDED_COPY ] = "EXTENDED_COPY",
>> + [ ATA_PASSTHROUGH ] = "ATA_PASSTHROUGH",
>> + [ ACCESS_CONTROL_IN ] = "ACCESS_CONTROL_IN",
>> + [ ACCESS_CONTROL_OUT ] = "ACCESS_CONTROL_OUT",
>> + [ READ_16 ] = "READ_16",
>> + [ COMPARE_AND_WRITE ] = "COMPARE_AND_WRITE",
>> + [ WRITE_16 ] = "WRITE_16",
>> + [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
>> + [ VERIFY_16 ] = "VERIFY_16",
>> + [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16",
>> + [ LOCATE_16 ] = "LOCATE_16",
>> + [ WRITE_SAME_16 ] = "WRITE_SAME_16",
>> + [ ERASE_16 ] = "ERASE_16",
>> + [ SERVICE_ACTION_IN ] = "SERVICE_ACTION_IN",
>> + [ WRITE_LONG_16 ] = "WRITE_LONG_16",
>> + [ REPORT_LUNS ] = "REPORT_LUNS",
>> + [ BLANK ] = "BLANK",
>> + [ MAINTENANCE_IN ] = "MAINTENANCE_IN",
>> + [ MAINTENANCE_OUT ] = "MAINTENANCE_OUT",
>
> Fixes missing WRITE_SAME_16, MAINTENANCE_IN, MAINTENANCE_OUT.
>
Yes.
>> [ MOVE_MEDIUM ] = "MOVE_MEDIUM",
>> + [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
>> [ READ_12 ] = "READ_12",
>> [ WRITE_12 ] = "WRITE_12",
>> [ WRITE_VERIFY_12 ] = "WRITE_VERIFY_12",
>> + [ VERIFY_12 ] = "VERIFY_12",
>> [ SEARCH_HIGH_12 ] = "SEARCH_HIGH_12",
>> [ SEARCH_EQUAL_12 ] = "SEARCH_EQUAL_12",
>> [ SEARCH_LOW_12 ] = "SEARCH_LOW_12",
>> [ READ_ELEMENT_STATUS ] = "READ_ELEMENT_STATUS",
>> [ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG",
>> - [ WRITE_LONG_2 ] = "WRITE_LONG_2",
>> -
>> - [ REPORT_DENSITY_SUPPORT ] = "REPORT_DENSITY_SUPPORT",
>> - [ GET_CONFIGURATION ] = "GET_CONFIGURATION",
>> - [ READ_16 ] = "READ_16",
>> - [ WRITE_16 ] = "WRITE_16",
>> - [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
>> - [ SERVICE_ACTION_IN ] = "SERVICE_ACTION_IN",
>> - [ REPORT_LUNS ] = "REPORT_LUNS",
>> - [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
>> + [ READ_DEFECT_DATA ] = "READ_DEFECT_DATA",
>
> READ_DEFECT_DATA_12!
>
Yes.
>> [ SET_CD_SPEED ] = "SET_CD_SPEED",
>> - [ BLANK ] = "BLANK",
>> };
>>
>> if (cmd>= ARRAY_SIZE(names) || names[cmd] == NULL)
>> diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
>> index 413cce0..458a797 100644
>> --- a/hw/scsi-defs.h
>> +++ b/hw/scsi-defs.h
>> @@ -26,6 +26,7 @@
>>
>> #define TEST_UNIT_READY 0x00
>> #define REZERO_UNIT 0x01
>> +#define REWIND 0x01
>> #define REQUEST_SENSE 0x03
>> #define FORMAT_UNIT 0x04
>> #define READ_BLOCK_LIMITS 0x05
>> @@ -48,14 +49,14 @@
>> #define RECEIVE_DIAGNOSTIC 0x1c
>> #define SEND_DIAGNOSTIC 0x1d
>> #define ALLOW_MEDIUM_REMOVAL 0x1e
>> -
>> #define SET_WINDOW 0x24
>> -#define READ_CAPACITY 0x25
>> +#define READ_CAPACITY_10 0x25
>> #define READ_10 0x28
>> #define WRITE_10 0x2a
>> #define SEEK_10 0x2b
>> -#define WRITE_VERIFY 0x2e
>> -#define VERIFY 0x2f
>> +#define LOCATE_10 0x2b
>> +#define WRITE_VERIFY_10 0x2e
>> +#define VERIFY_10 0x2f
>> #define SEARCH_HIGH 0x30
>> #define SEARCH_EQUAL 0x31
>> #define SEARCH_LOW 0x32
>> @@ -71,11 +72,14 @@
>> #define WRITE_BUFFER 0x3b
>> #define READ_BUFFER 0x3c
>> #define UPDATE_BLOCK 0x3d
>> -#define READ_LONG 0x3e
>> -#define WRITE_LONG 0x3f
>> +#define READ_LONG_10 0x3e
>> +#define WRITE_LONG_10 0x3f
>> #define CHANGE_DEFINITION 0x40
>> -#define WRITE_SAME 0x41
>> +#define WRITE_SAME_10 0x41
>> +#define UNMAP 0x42
>> #define READ_TOC 0x43
>> +#define REPORT_DENSITY_SUPPORT 0x44
>> +#define GET_CONFIGURATION 0x46
>> #define LOG_SELECT 0x4c
>> #define LOG_SENSE 0x4d
>> #define MODE_SELECT_10 0x55
>> @@ -84,32 +88,40 @@
>> #define MODE_SENSE_10 0x5a
>> #define PERSISTENT_RESERVE_IN 0x5e
>> #define PERSISTENT_RESERVE_OUT 0x5f
>> +#define VARLENGTH_CDB 0x7f
>> +#define WRITE_FILEMARKS_16 0x80
>> +#define EXTENDED_COPY 0x83
>> +#define ATA_PASSTHROUGH 0x85
>> +#define ACCESS_CONTROL_IN 0x86
>> +#define ACCESS_CONTROL_OUT 0x87
>> +#define READ_16 0x88
>> +#define COMPARE_AND_WRITE 0x89
>> +#define WRITE_16 0x8a
>> +#define WRITE_VERIFY_16 0x8e
>> +#define VERIFY_16 0x8f
>> +#define SYNCHRONIZE_CACHE_16 0x91
>> +#define LOCATE_16 0x92
>> #define WRITE_SAME_16 0x93
>> +#define ERASE_16 0x93
>> +#define SERVICE_ACTION_IN 0x9e
>> +#define WRITE_LONG_16 0x9f
>> +#define REPORT_LUNS 0xa0
>> +#define BLANK 0xa1
>> #define MAINTENANCE_IN 0xa3
>> #define MAINTENANCE_OUT 0xa4
>> #define MOVE_MEDIUM 0xa5
>> +#define LOAD_UNLOAD 0xa6
>> #define READ_12 0xa8
>> #define WRITE_12 0xaa
>> #define WRITE_VERIFY_12 0xae
>> +#define VERIFY_12 0xaf
>> #define SEARCH_HIGH_12 0xb0
>> #define SEARCH_EQUAL_12 0xb1
>> #define SEARCH_LOW_12 0xb2
>> #define READ_ELEMENT_STATUS 0xb8
>> #define SEND_VOLUME_TAG 0xb6
>> -#define WRITE_LONG_2 0xea
>> -
>> -/* from hw/scsi-generic.c */
>> -#define REWIND 0x01
>> -#define REPORT_DENSITY_SUPPORT 0x44
>> -#define GET_CONFIGURATION 0x46
>> -#define READ_16 0x88
>> -#define WRITE_16 0x8a
>> -#define WRITE_VERIFY_16 0x8e
>> -#define SERVICE_ACTION_IN 0x9e
>> -#define REPORT_LUNS 0xa0
>> -#define LOAD_UNLOAD 0xa6
>> -#define SET_CD_SPEED 0xbb
>> -#define BLANK 0xa1
>> +#define READ_DEFECT_DATA_12 0xb7
>> +#define SET_CD_SPEED 0xbb
>>
>> /*
>> * SAM Status codes
>> @@ -154,6 +166,7 @@
>>
>> #define TYPE_DISK 0x00
>> #define TYPE_TAPE 0x01
>> +#define TYPE_PRINTER 0x02
>> #define TYPE_PROCESSOR 0x03 /* HP scanners use this */
>> #define TYPE_WORM 0x04 /* Treated as ROM by our system */
>> #define TYPE_ROM 0x05
>> @@ -161,6 +174,9 @@
>> #define TYPE_MOD 0x07 /* Magneto-optical disk -
>> * - treated as TYPE_DISK */
>> #define TYPE_MEDIUM_CHANGER 0x08
>> -#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
>> +#define TYPE_STORAGE_ARRAY 0x0c /* Storage array device */
>> +#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
>> +#define TYPE_RBC 0x0e /* Simplified Direct-Access Device */
>> +#define TYPE_OSD 0x11 /* Object-storage Device */
>> #define TYPE_NO_LUN 0x7f
>>
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index 05d14ab..2f7e0c9 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -836,7 +836,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>> case TEST_UNIT_READY:
>> if (!bdrv_is_inserted(s->bs))
>> goto not_ready;
>> - break;
>> + break;
>> case REQUEST_SENSE:
>> if (req->cmd.xfer< 4)
>> goto illegal_request;
>> @@ -848,7 +848,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>> buflen = scsi_disk_emulate_inquiry(req, outbuf);
>> if (buflen< 0)
>> goto illegal_request;
>> - break;
>> + break;
>> case MODE_SENSE:
>> case MODE_SENSE_10:
>> buflen = scsi_disk_emulate_mode_sense(req, outbuf);
>> @@ -881,14 +881,14 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>> /* load/eject medium */
>> bdrv_eject(s->bs, !(req->cmd.buf[4]& 1));
>> }
>> - break;
>> + break;
>> case ALLOW_MEDIUM_REMOVAL:
>> bdrv_set_locked(s->bs, req->cmd.buf[4]& 1);
>> - break;
>> + break;
>> case READ_CAPACITY:
>> /* The normal LEN field for this command is zero. */
>> - memset(outbuf, 0, 8);
>> - bdrv_get_geometry(s->bs,&nb_sectors);
>> + memset(outbuf, 0, 8);
>> + bdrv_get_geometry(s->bs,&nb_sectors);
>> if (!nb_sectors)
>> goto not_ready;
>> nb_sectors /= s->cluster_size;
>> @@ -908,7 +908,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>> outbuf[6] = s->cluster_size * 2;
>> outbuf[7] = 0;
>> buflen = 8;
>> - break;
>> + break;
>> case SYNCHRONIZE_CACHE:
>> ret = bdrv_flush(s->bs);
>> if (ret< 0) {
>> @@ -970,13 +970,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
>> outbuf[3] = 8;
>> buflen = 16;
>> break;
>> - case VERIFY:
>> - break;
>> - case REZERO_UNIT:
>> - DPRINTF("Rezero Unit\n");
>> - if (!bdrv_is_inserted(s->bs)) {
>> - goto not_ready;
>> - }
>
> Functional change, the commit message lies. Separate patch?
>
> Matching update to scsi_cmd_table[] missing.
>
> Looks like we could purge REZERO_UNIT elsewhere, too.
>
>> + case VERIFY_10:
>> break;
>> default:
>> scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
>> @@ -1052,14 +1046,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
>> case RELEASE_10:
>> case START_STOP:
>> case ALLOW_MEDIUM_REMOVAL:
>> - case READ_CAPACITY:
>> + case READ_CAPACITY_10:
>> case SYNCHRONIZE_CACHE:
>> case READ_TOC:
>> case GET_CONFIGURATION:
>> case SERVICE_ACTION_IN:
>> case REPORT_LUNS:
>> - case VERIFY:
>> - case REZERO_UNIT:
>> + case VERIFY_10:
>> rc = scsi_disk_emulate_command(r, outbuf);
>> if (rc< 0) {
>> return 0;
>> @@ -1082,7 +1075,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
>> case WRITE_10:
>> case WRITE_12:
>> case WRITE_16:
>> - case WRITE_VERIFY:
>> + case WRITE_VERIFY_10:
>> case WRITE_VERIFY_12:
>> case WRITE_VERIFY_16:
>> len = r->req.cmd.xfer / s->qdev.blocksize;
>> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
>> index 90345a7..17e83c7 100644
>> --- a/hw/scsi-generic.c
>> +++ b/hw/scsi-generic.c
>> @@ -406,7 +406,7 @@ static int get_blocksize(BlockDriverState *bdrv)
>>
>> memset(cmd, 0, sizeof(cmd));
>> memset(buf, 0, sizeof(buf));
>> - cmd[0] = READ_CAPACITY;
>> + cmd[0] = READ_CAPACITY_10;
>>
>> memset(&io_header, 0, sizeof(io_header));
>> io_header.interface_id = 'S';
OK, convinced. Will be doing a separate patch for removing the
obsolete commands.
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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/3] scsi-disk: Remove drive_kind
2011-07-22 12:31 [Qemu-devel] [PATCH 0/3] Check for supported SCSI commands Hannes Reinecke
2011-07-22 12:31 ` [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions Hannes Reinecke
@ 2011-07-22 12:31 ` Hannes Reinecke
2011-07-22 12:31 ` [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands Hannes Reinecke
2 siblings, 0 replies; 8+ messages in thread
From: Hannes Reinecke @ 2011-07-22 12:31 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Hannes Reinecke, Markus Armbruster, kvm,
Alexander Graf
Instead of using our own type structure we can be using the
SCSI type from the parent device.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
hw/scsi-disk.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 129 insertions(+), 27 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2f7e0c9..8ad90c0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -74,7 +74,6 @@ struct SCSIDiskState
char *version;
char *serial;
SCSISense sense;
- SCSIDriveKind drive_kind;
};
static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -362,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;
}
@@ -382,7 +475,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
return -1;
}
- if (s->drive_kind == SCSI_CD) {
+ if (s->qdev.type == TYPE_ROM) {
outbuf[buflen++] = 5;
} else {
outbuf[buflen++] = 0;
@@ -401,7 +494,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
if (s->serial)
outbuf[buflen++] = 0x80; // unit serial number
outbuf[buflen++] = 0x83; // device identification
- if (s->drive_kind == SCSI_HD) {
+ if (s->qdev.type == TYPE_DISK) {
outbuf[buflen++] = 0xb0; // block limits
outbuf[buflen++] = 0xb2; // thin provisioning
}
@@ -460,7 +553,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
unsigned int opt_io_size =
s->qdev.conf.opt_io_size / s->qdev.blocksize;
- if (s->drive_kind == SCSI_CD) {
+ if (s->qdev.type == TYPE_ROM) {
DPRINTF("Inquiry (EVPD[%02X] not supported for CDROM\n",
page_code);
return -1;
@@ -526,16 +619,15 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
memset(outbuf, 0, buflen);
if (req->lun) {
- outbuf[0] = 0x7f; /* LUN not supported */
+ outbuf[0] = 0x7f; /* LUN not supported */
return buflen;
}
- if (s->drive_kind == SCSI_CD) {
- outbuf[0] = 5;
+ outbuf[0] = s->qdev.type & 0x1f;
+ if (s->qdev.type == TYPE_ROM) {
outbuf[1] = 0x80;
memcpy(&outbuf[16], "QEMU CD-ROM ", 16);
} else {
- outbuf[0] = 0;
outbuf[1] = s->removable ? 0x80 : 0;
memcpy(&outbuf[16], "QEMU HARDDISK ", 16);
}
@@ -661,7 +753,7 @@ static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p,
return p[1] + 2;
case 0x2a: /* CD Capabilities and Mechanical Status page. */
- if (s->drive_kind != SCSI_CD)
+ if (s->qdev.type != TYPE_ROM)
return 0;
p[0] = 0x2a;
p[1] = 0x14;
@@ -877,7 +969,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
goto illegal_request;
break;
case START_STOP:
- if (s->drive_kind == SCSI_CD && (req->cmd.buf[4] & 2)) {
+ if (s->qdev.type == TYPE_ROM && (req->cmd.buf[4] & 2)) {
/* load/eject medium */
bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
}
@@ -885,7 +977,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
case ALLOW_MEDIUM_REMOVAL:
bdrv_set_locked(s->bs, req->cmd.buf[4] & 1);
break;
- case READ_CAPACITY:
+ case READ_CAPACITY_10:
/* The normal LEN field for this command is zero. */
memset(outbuf, 0, 8);
bdrv_get_geometry(s->bs, &nb_sectors);
@@ -1034,6 +1126,14 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
return 0;
}
}
+ if (!scsi_command_supported(command, s->qdev.type)) {
+ DPRINTF("Command %02x not supported for type %02x\n",
+ command, s->qdev.type);
+ scsi_command_complete(r, CHECK_CONDITION,
+ SENSE_CODE(INVALID_OPCODE));
+ return 0;
+ }
+
switch (command) {
case TEST_UNIT_READY:
case REQUEST_SENSE:
@@ -1183,7 +1283,7 @@ static void scsi_destroy(SCSIDevice *dev)
blockdev_mark_auto_del(s->qdev.conf.bs);
}
-static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
+static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
DriveInfo *dinfo;
@@ -1193,9 +1293,8 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
return -1;
}
s->bs = s->qdev.conf.bs;
- s->drive_kind = kind;
- if (kind == SCSI_HD && !bdrv_is_inserted(s->bs)) {
+ if (scsi_type == TYPE_DISK && !bdrv_is_inserted(s->bs)) {
error_report("Device needs media, but drive is empty");
return -1;
}
@@ -1217,44 +1316,47 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
return -1;
}
- if (kind == SCSI_CD) {
+ if (scsi_type == TYPE_ROM) {
s->qdev.blocksize = 2048;
- } else {
+ } else if (scsi_type == TYPE_DISK) {
s->qdev.blocksize = s->qdev.conf.logical_block_size;
+ } else {
+ error_report("scsi-disk: Unhandled SCSI type %02x", scsi_type);
+ return -1;
}
s->cluster_size = s->qdev.blocksize / 512;
s->bs->buffer_alignment = s->qdev.blocksize;
- s->qdev.type = TYPE_DISK;
+ s->qdev.type = scsi_type;
qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
- bdrv_set_removable(s->bs, kind == SCSI_CD);
+ bdrv_set_removable(s->bs, scsi_type == TYPE_ROM);
add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, ",0");
return 0;
}
static int scsi_hd_initfn(SCSIDevice *dev)
{
- return scsi_initfn(dev, SCSI_HD);
+ return scsi_initfn(dev, TYPE_DISK);
}
static int scsi_cd_initfn(SCSIDevice *dev)
{
- return scsi_initfn(dev, SCSI_CD);
+ return scsi_initfn(dev, TYPE_ROM);
}
static int scsi_disk_initfn(SCSIDevice *dev)
{
- SCSIDriveKind kind;
DriveInfo *dinfo;
+ uint8_t scsi_type = TYPE_DISK;
- if (!dev->conf.bs) {
- kind = SCSI_HD; /* will die in scsi_initfn() */
- } else {
+ if (dev->conf.bs) {
dinfo = drive_get_by_blockdev(dev->conf.bs);
- kind = dinfo->media_cd ? SCSI_CD : SCSI_HD;
+ if (dinfo->media_cd) {
+ scsi_type = TYPE_ROM;
+ }
}
- return scsi_initfn(dev, kind);
+ return scsi_initfn(dev, scsi_type);
}
#define DEFINE_SCSI_DISK_PROPERTIES() \
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands
2011-07-22 12:31 [Qemu-devel] [PATCH 0/3] Check for supported SCSI commands Hannes Reinecke
2011-07-22 12:31 ` [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions Hannes Reinecke
2011-07-22 12:31 ` [Qemu-devel] [PATCH 2/3] scsi-disk: Remove drive_kind Hannes Reinecke
@ 2011-07-22 12:31 ` Hannes Reinecke
2011-07-22 14:09 ` Markus Armbruster
2 siblings, 1 reply; 8+ messages in thread
From: Hannes Reinecke @ 2011-07-22 12:31 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Hannes Reinecke, Markus Armbruster, kvm,
Alexander Graf
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 | 102 --------------------------------------------------------
1 files changed, 0 insertions(+), 102 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 8ad90c0..6e985b4 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -361,100 +361,6 @@ 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);
@@ -1126,14 +1032,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
return 0;
}
}
- if (!scsi_command_supported(command, s->qdev.type)) {
- DPRINTF("Command %02x not supported for type %02x\n",
- command, s->qdev.type);
- scsi_command_complete(r, CHECK_CONDITION,
- SENSE_CODE(INVALID_OPCODE));
- return 0;
- }
-
switch (command) {
case TEST_UNIT_READY:
case REQUEST_SENSE:
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands
2011-07-22 12:31 ` [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands Hannes Reinecke
@ 2011-07-22 14:09 ` Markus Armbruster
2011-07-22 14:15 ` Hannes Reinecke
0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2011-07-22 14:09 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Kevin Wolf, qemu-devel, kvm, Alexander Graf
Hannes Reinecke <hare@suse.de> writes:
> 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>
Commit message says "patch adds", patch only deletes.
Looks like something wrent wrong with 2/3 and 3/3.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands
2011-07-22 14:09 ` Markus Armbruster
@ 2011-07-22 14:15 ` Hannes Reinecke
0 siblings, 0 replies; 8+ messages in thread
From: Hannes Reinecke @ 2011-07-22 14:15 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Kevin Wolf, qemu-devel, kvm, Alexander Graf
On 07/22/2011 04:09 PM, Markus Armbruster wrote:
> Hannes Reinecke<hare@suse.de> writes:
>
>> 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>
>
> Commit message says "patch adds", patch only deletes.
>
> Looks like something wrent wrong with 2/3 and 3/3.
Argl. Yes, correct.
Will be sending an updated patchset.
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)
^ permalink raw reply [flat|nested] 8+ messages in thread