[stgt] [PATCH] Implement error checking in VERIFY10/12/16
FUJITA Tomonori
fujita.tomonori at lab.ntt.co.jp
Thu Jan 26 18:32:21 CET 2012
On Thu, 26 Jan 2012 16:29:11 +1100
ronnie sahlberg <ronniesahlberg at gmail.com> wrote:
> From 39ed0ca94bdc8ae4f90c72d56d47f962b3bc5ef5 Mon Sep 17 00:00:00 2001
> From: Ronnie Sahlberg <ronniesahlberg at gmail.com>
> 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 at gmail.com>
> ---
> usr/bs_rdwr.c | 27 +++++++++++++++++++++++++++
> usr/sbc.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> usr/scsi.c | 6 +++---
> usr/scsi.h | 5 ++++-
> 4 files changed, 85 insertions(+), 6 deletions(-)
Thanks a lot! Looks great, some minor comments.
> 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) {
Please use the following style instead?
if (!tmpbuf)
> + 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);
In this case, we need to avoid the following memcmp to avoid the error
code overwriting (the memcmp fails in this case)?
if (ret != length)
set_medium_error(&result, &key, &asc);
else if (memcmp(scsi_get_out_buffer(cmd), tmpbuf, length)) {
?
> + 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) {
if (vprotect) {
> + /* 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;
Use cmd->tl later?
> + 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
>
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the stgt
mailing list