Re: [Qemu-devel] [PATCH 18/55] scsi-disk: Reject CD-specific SCSI commands to disks

2011-07-29 Thread Markus Armbruster
Markus Armbruster arm...@redhat.com writes:

 Christoph Hellwig h...@lst.de writes:

 On Wed, Jul 20, 2011 at 06:23:52PM +0200, Markus Armbruster wrote:
 Use a table to declare which drive kinds accept each command.
 
 Limit READ_CAPACITY, READ_TOC, GET_CONFIGURATION to SCSI_CD, as per
 SPC-4.

 READ CAPACITY is defined in SBC, and absolutely required for proper
 operation with disks.

 Will fix.

Second thoughts: dropping this bug fix patch for now, because it clashes
with Hannes's work in progress[*], and the rest of my series doesn't
depend on it.

[*] [PATCH 6/6] scsi-disk: Check for supported commands
http://lists.gnu.org/archive/html/qemu-devel/2011-07/msg03082.html



Re: [Qemu-devel] [PATCH 18/55] scsi-disk: Reject CD-specific SCSI commands to disks

2011-07-26 Thread Christoph Hellwig
On Wed, Jul 20, 2011 at 06:23:52PM +0200, Markus Armbruster wrote:
 Use a table to declare which drive kinds accept each command.
 
 Limit READ_CAPACITY, READ_TOC, GET_CONFIGURATION to SCSI_CD, as per
 SPC-4.

READ CAPACITY is defined in SBC, and absolutely required for proper
operation with disks.




Re: [Qemu-devel] [PATCH 18/55] scsi-disk: Reject CD-specific SCSI commands to disks

2011-07-26 Thread Markus Armbruster
Christoph Hellwig h...@lst.de writes:

 On Wed, Jul 20, 2011 at 06:23:52PM +0200, Markus Armbruster wrote:
 Use a table to declare which drive kinds accept each command.
 
 Limit READ_CAPACITY, READ_TOC, GET_CONFIGURATION to SCSI_CD, as per
 SPC-4.

 READ CAPACITY is defined in SBC, and absolutely required for proper
 operation with disks.

Will fix.



[Qemu-devel] [PATCH 18/55] scsi-disk: Reject CD-specific SCSI commands to disks

2011-07-20 Thread Markus Armbruster
Use a table to declare which drive kinds accept each command.

Limit READ_CAPACITY, READ_TOC, GET_CONFIGURATION to SCSI_CD, as per
SPC-4.

It would be nice to have handler functions in the table, like commit
e1a064f9 did for ATAPI.  Left for another day.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/scsi-disk.c |   63 +++-
 1 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a8c7372..c4643d0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -991,6 +991,59 @@ illegal_request:
 return -1;
 }
 
+#define HD_OK (1u  SCSI_HD)
+#define CD_OK (1u  SCSI_CD)
+#define ALL_OK (HD_OK | CD_OK)
+
+/* See SPC-4 T10/1731-D Table D.2 Operation codes */
+uint8_t scsi_cmd_table[0x100] = {
+[TEST_UNIT_READY]   = ALL_OK,
+[REZERO_UNIT]   = ALL_OK,
+[REQUEST_SENSE] = ALL_OK,
+/* FORMAT_UNIT not implemented, SPC-4 mandatory for HD */
+[READ_6]= ALL_OK,
+[WRITE_6]   = ALL_OK,
+[SEEK_6]= ALL_OK,
+[INQUIRY]   = ALL_OK,
+[MODE_SELECT]   = ALL_OK,
+[RESERVE]   = ALL_OK,
+[RELEASE]   = ALL_OK,
+[MODE_SENSE]= ALL_OK,
+[START_STOP]= ALL_OK,
+/* SEND DIAGNOSTIC not implemented, SPC-4 mandatory for HD */
+[ALLOW_MEDIUM_REMOVAL]  = ALL_OK,
+[READ_CAPACITY] = CD_OK,
+[READ_10]   = ALL_OK,
+[WRITE_10]  = ALL_OK,
+[SEEK_10]   = ALL_OK,
+[WRITE_VERIFY]  = ALL_OK,
+[VERIFY]= ALL_OK,
+[SYNCHRONIZE_CACHE] = ALL_OK,
+[READ_TOC]  = CD_OK,
+[GET_CONFIGURATION] = CD_OK,
+/* GET EVENT STATUS NOTIFICATION not implemented, SPC-4 mandatory for CD */
+[MODE_SELECT_10]= ALL_OK,
+[RESERVE_10]= ALL_OK,
+[RELEASE_10]= ALL_OK,
+[MODE_SENSE_10] = ALL_OK,
+[READ_16]   = ALL_OK,
+[WRITE_16]  = ALL_OK,
+[WRITE_VERIFY_16]   = ALL_OK,
+/* VERIFY(16) not implemented, WRITE_VERIFY(16) is, odd */
+[WRITE_SAME_16] = ALL_OK,
+[SERVICE_ACTION_IN] = ALL_OK,
+[REPORT_LUNS]   = ALL_OK,
+[READ_12]   = ALL_OK,
+[WRITE_12]  = ALL_OK,
+[WRITE_VERIFY_12]   = ALL_OK,
+/* VERIFY(12) not implemented, WRITE_VERIFY(12) is, odd */
+};
+
+static bool scsi_cmd_permitted(SCSIDiskState *s, uint8_t cmd)
+{
+return scsi_cmd_table[cmd]  (1u  s-drive_kind);
+}
+
 /* Execute a scsi command.  Returns the length of the data expected by the
command.  This will be Positive for data transfers from the device
(eg. disk reads), negative for transfers to the device (eg. disk writes),
@@ -1033,6 +1086,13 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 return 0;
 }
 }
+
+if (!scsi_cmd_permitted(s, command)) {
+DPRINTF(SCSI command (%2.2x) not valid for device\n, command);
+scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+return 0;
+}
+
 switch (command) {
 case TEST_UNIT_READY:
 case REQUEST_SENSE:
@@ -1137,7 +1197,8 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 
 break;
 default:
-DPRINTF(Unknown SCSI command (%2.2x)\n, buf[0]);
+/* should not be reachable */
+BADF(Unknown SCSI command (%2.2x)\n, buf[0]);
 scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
 return 0;
 fail:
-- 
1.7.2.3