Tomo, List
Please find attached a patch for VERIFY 10/12/16
It adds support for VERIFY12/16
It also adds verification that the arguments passed from the initiator
are correct (we do not support protection infomatiom)
and return error for invalid requests.
If BYTECHK == 1 we also verify the data with the content of the medium.
If there is a mismatch we return check-condition with key==MISCOMPARE
and the proper ASCQ
Please also find a small capture file showing TGTD returning proper
MISCOMPARE when a VERIFY10 request comes in with bad data (one byte
modified in one of the blocks)
To test VERIFY10 failure I created VERIFY10 test cases for the
libiscsi test tool to verify that a target responds GOOD when VERIFY10
matches the data on the medium and that the target responds MISMATCH
if the data is known to be bad.
regards
ronnie sahlberg
On Thu, Jan 26, 2012 at 12:00 PM, FUJITA Tomonori
<fujita.tomonori@xxxxxxxxxxxxx> wrote:
> On Thu, 26 Jan 2012 11:17:39 +1100
> ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
>
>> From dfb19f35441d137ef4b82cec924a8746cb1c040d Mon Sep 17 00:00:00 2001
>> From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> Date: Thu, 26 Jan 2012 10:35:22 +1100
>> Subject: [PATCH] SBC VERIFY: Update VERIFY to check vprotect argument
>>
>> TGTD does not support formatting protection information so make
>> TGTD fail a VERIFY command requesting vprotect != 0 with a proper
>> check condition.
>>
>> Add VERIFY12/VERIFY16. These are both identical to the already existing VERIFY10 as far as we are converned.
>>
>> See SBC 5.22 VERIFY 10 COMMAND, tables 65 and 66
>> If the media is not formatted with protection information (as in TGTD)
>> any value of vprotect other than 000b is an error condition and the device h
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> ---
>> usr/sbc.c | 25 ++++++++++++++++++++++---
>> 1 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/usr/sbc.c b/usr/sbc.c
>> index 53e785b..23b1778 100644
>> --- a/usr/sbc.c
>> +++ b/usr/sbc.c
>> @@ -265,7 +265,26 @@ sense:
>>
>> static int sbc_verify(int host_no, struct scsi_cmd *cmd)
>> {
>> - return SAM_STAT_GOOD;
>> + unsigned char key;
>> + uint16_t asc;
>> + int vprotect;
>> +
>> + vprotect = cmd->scb[1] & 0xe0;
>> + if (vprotect != 0) {
>> + /* we dont support formatting with protection information,
>> + * so all verify with vprotect!=0 is an error condition
>> + */
>> + key = ILLEGAL_REQUEST;
>> + asc = ASC_INVALID_FIELD_IN_CDB;
>> + goto sense;
>> + }
>> +
>> + return SAM_STAT_GOOD;
>
> Hmm, should we implement VERIFY (such as data comparison) instead of
> returning GOOD if we replace illegal_op?
>
>
>> +sense:
>> + scsi_set_in_resid_by_actual(cmd, 0);
>> + sense_data_build(cmd, key, asc);
>> + return SAM_STAT_CHECK_CONDITION;
>> }
>>
>> static int sbc_service_action(int host_no, struct scsi_cmd *cmd)
>> @@ -506,7 +525,7 @@ static struct device_type_template sbc_template = {
>> {spc_illegal_op,},
>> {spc_illegal_op,},
>> {spc_illegal_op,},
>> - {spc_illegal_op,},
>> + {sbc_verify, NULL, PR_EA_FA|PR_EA_FN},
>>
>> /* 0x90 */
>> {sbc_rw, NULL, PR_EA_FA|PR_EA_FN}, /*PRE_FETCH_16 */
>> @@ -544,7 +563,7 @@ static struct device_type_template sbc_template = {
>> {spc_illegal_op,},
>> {spc_illegal_op,},
>> {spc_illegal_op,},
>> - {spc_illegal_op,},
>> + {sbc_verify, NULL, PR_EA_FA|PR_EA_FN},
>>
>> [0xb0 ... 0xff] = {spc_illegal_op},
>> }
>> --
>> 1.7.3.1
>>
From 39ed0ca94bdc8ae4f90c72d56d47f962b3bc5ef5 Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
Date: Thu, 26 Jan 2012 16:21:22 +1100
Subject: [PATCH] SBC VERIFY: implement VERIFY 10/12/16 and check the arguments
TGTD does not support formatting protection information so make
TGTD fail a VERIFY command requesting vprotect != 0 with a proper
check condition.
See SBC 5.22 VERIFY 10 COMMAND, tables 65 and 66
If the media is not formatted with protection information (as in TGTD)
any value of vprotect other than 000b is an error condition and the device h
If BYTCHK==1 we must also verify the DATA-OUT buffer with the content of the media. Add a check that the data matches and return sense key:MISMATCH and the proper ASCQ if a mismatch is found.
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
---
usr/bs_rdwr.c | 27 +++++++++++++++++++++++++++
usr/sbc.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
usr/scsi.c | 6 +++---
usr/scsi.h | 5 ++++-
4 files changed, 85 insertions(+), 6 deletions(-)
diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
index acee73c..8d32f45 100644
--- a/usr/bs_rdwr.c
+++ b/usr/bs_rdwr.c
@@ -62,6 +62,7 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
int result = SAM_STAT_GOOD;
uint8_t key;
uint16_t asc;
+ char *tmpbuf;
ret = length = 0;
key = asc = 0;
@@ -121,6 +122,32 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
if (ret != 0)
set_medium_error(&result, &key, &asc);
break;
+ case VERIFY_10:
+ case VERIFY_12:
+ case VERIFY_16:
+ length = scsi_get_out_length(cmd);
+
+ tmpbuf = malloc(length);
+ if (tmpbuf == NULL) {
+ result = SAM_STAT_CHECK_CONDITION;
+ key = HARDWARE_ERROR;
+ asc = ASC_INTERNAL_TGT_FAILURE;
+ break;
+ }
+
+ ret = pread64(fd, tmpbuf, length, cmd->offset);
+
+ if (ret != length)
+ set_medium_error(&result, &key, &asc);
+
+ if (memcmp(scsi_get_out_buffer(cmd), tmpbuf, length)) {
+ result = SAM_STAT_CHECK_CONDITION;
+ key = MISCOMPARE;
+ asc = ASC_MISCOMPARE_DURING_VERIFY_OPERATION;
+ }
+
+ free(tmpbuf);
+ break;
default:
break;
}
diff --git a/usr/sbc.c b/usr/sbc.c
index 53e785b..5805a72 100644
--- a/usr/sbc.c
+++ b/usr/sbc.c
@@ -265,7 +265,56 @@ sense:
static int sbc_verify(int host_no, struct scsi_cmd *cmd)
{
+ struct scsi_lu *lu = cmd->dev;
+ unsigned char key;
+ uint16_t asc;
+ int vprotect, bytchk, ret;
+ uint64_t lba;
+ uint32_t tl;
+
+ vprotect = cmd->scb[1] & 0xe0;
+ if (vprotect != 0) {
+ /* we dont support formatting with protection information,
+ * so all verify with vprotect!=0 is an error condition
+ */
+ key = ILLEGAL_REQUEST;
+ asc = ASC_INVALID_FIELD_IN_CDB;
+ goto sense;
+ }
+
+ bytchk = cmd->scb[1] & 0x02;
+ if (!bytchk) {
+ /* no data compare with the media */
+ return SAM_STAT_GOOD;
+ }
+
+ lba = scsi_rw_offset(cmd->scb) << cmd->dev->blk_shift;
+ tl = scsi_rw_count(cmd->scb) << cmd->dev->blk_shift;
+
+ /* Verify that we are not doing i/o beyond
+ the end-of-lun */
+ if (tl && (lba + tl > lu->size)) {
+ key = ILLEGAL_REQUEST;
+ asc = ASC_LBA_OUT_OF_RANGE;
+ goto sense;
+ }
+
+ cmd->offset = lba;
+ cmd->tl = tl;
+
+ ret = cmd->dev->bst->bs_cmd_submit(cmd);
+ if (ret) {
+ key = HARDWARE_ERROR;
+ asc = ASC_INTERNAL_TGT_FAILURE;
+ goto sense;
+ }
+
return SAM_STAT_GOOD;
+
+sense:
+ scsi_set_in_resid_by_actual(cmd, 0);
+ sense_data_build(cmd, key, asc);
+ return SAM_STAT_CHECK_CONDITION;
}
static int sbc_service_action(int host_no, struct scsi_cmd *cmd)
@@ -506,7 +555,7 @@ static struct device_type_template sbc_template = {
{spc_illegal_op,},
{spc_illegal_op,},
{spc_illegal_op,},
- {spc_illegal_op,},
+ {sbc_verify, NULL, PR_EA_FA|PR_EA_FN},
/* 0x90 */
{sbc_rw, NULL, PR_EA_FA|PR_EA_FN}, /*PRE_FETCH_16 */
@@ -544,7 +593,7 @@ static struct device_type_template sbc_template = {
{spc_illegal_op,},
{spc_illegal_op,},
{spc_illegal_op,},
- {spc_illegal_op,},
+ {sbc_verify, NULL, PR_EA_FA|PR_EA_FN},
[0xb0 ... 0xff] = {spc_illegal_op},
}
diff --git a/usr/scsi.c b/usr/scsi.c
index 5f78bfd..7802ba1 100644
--- a/usr/scsi.c
+++ b/usr/scsi.c
@@ -117,7 +117,7 @@ uint64_t scsi_rw_offset(uint8_t *scb)
case READ_10:
case PRE_FETCH_10:
case WRITE_10:
- case VERIFY:
+ case VERIFY_10:
case WRITE_VERIFY:
case SYNCHRONIZE_CACHE:
case READ_12:
@@ -160,7 +160,7 @@ uint32_t scsi_rw_count(uint8_t *scb)
case READ_10:
case PRE_FETCH_10:
case WRITE_10:
- case VERIFY:
+ case VERIFY_10:
case WRITE_VERIFY:
case SYNCHRONIZE_CACHE:
cnt = (uint16_t)scb[7] << 8 | (uint16_t)scb[8];
@@ -261,7 +261,7 @@ int scsi_is_io_opcode(unsigned char op)
case WRITE_6:
case READ_10:
case WRITE_10:
- case VERIFY:
+ case VERIFY_10:
case WRITE_VERIFY:
case READ_12:
case WRITE_12:
diff --git a/usr/scsi.h b/usr/scsi.h
index 0d33e32..ca1109a 100644
--- a/usr/scsi.h
+++ b/usr/scsi.h
@@ -40,7 +40,7 @@
#define SEEK_10 0x2b
#define POSITION_TO_ELEMENT 0x2b
#define WRITE_VERIFY 0x2e
-#define VERIFY 0x2f
+#define VERIFY_10 0x2f
#define SEARCH_HIGH 0x30
#define SEARCH_EQUAL 0x31
#define SEARCH_LOW 0x32
@@ -230,6 +230,9 @@
#define ASC_WRITE_PROTECT 0x2700
#define ASC_MEDIUM_OVERWRITE_ATTEMPTED 0x300c
+/* Miscompare */
+#define ASC_MISCOMPARE_DURING_VERIFY_OPERATION 0x1d00
+
/* PERSISTENT_RESERVE_IN service action codes */
#define PR_IN_READ_KEYS 0x00
--
1.7.3.1
Attachment:
0001-SBC-VERIFY-implement-VERIFY-10-12-16-and-check-the-a.patch.gz
Description: GNU Zip compressed data
Attachment:
VERIFY10-miscompare.cap
Description: application/cap
