From 16d0b30bc89abe353b8021296243eb032a7a338e Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 29 Jun 2012 22:31:13 +0000 Subject: [PATCH] Cleanup: let's have a dedicate CDB flag to mark CDBs without LBA instead of relying on lba_off = 0. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@4392 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst_const.h | 43 +++++++++++++++++++----------- scst/src/dev_handlers/scst_vdisk.c | 3 +-- scst/src/scst_lib.c | 38 ++++++++++++++++++++++++-- scst/src/scst_targ.c | 3 ++- 4 files changed, 66 insertions(+), 21 deletions(-) diff --git a/scst/include/scst_const.h b/scst/include/scst_const.h index dae86acf7..24814e2eb 100644 --- a/scst/include/scst_const.h +++ b/scst/include/scst_const.h @@ -176,36 +176,47 @@ enum scst_cmd_queue_type { SCST_CMD_QUEUE_ACA }; -/************************************************************* - ** CDB flags - *************************************************************/ +/*************************************************************** + ** CDB flags. All must be single bit fit in int32. Bit fields + ** approach (unsigned int x:1) was not used, because those + ** flags supposed to be passed to the user space where another + ** compiler with another bitfields layout can be used. + ***************************************************************/ enum scst_cdb_flags { SCST_TRANSFER_LEN_TYPE_FIXED = 0x0001, SCST_SMALL_TIMEOUT = 0x0002, SCST_LONG_TIMEOUT = 0x0004, SCST_UNKNOWN_LBA = 0x0008, SCST_UNKNOWN_LENGTH = 0x0010, - SCST_INFO_VALID = 0x0020, /* must be single bit */ - SCST_IMPLICIT_HQ = 0x0040, - SCST_SKIP_UA = 0x0080, - SCST_WRITE_MEDIUM = 0x0100, - SCST_LOCAL_CMD = 0x0200, + SCST_INFO_VALID = 0x0020, + + /* + * Set if LBA not defined for this CDB. The "NOT" approach + * was used to make sure that all dev handlers either init + * cmd->lba or set this flag (for backward compatibility) + */ + SCST_LBA_NOT_VALID = 0x0040, + + SCST_IMPLICIT_HQ = 0x0080, + SCST_SKIP_UA = 0x0100, + SCST_WRITE_MEDIUM = 0x0200, + SCST_LOCAL_CMD = 0x0400, /* * Set if CDB is fully locally handled by SCST. Dev handlers * parse() and dev_done() not called for such commands */ - SCST_FULLY_LOCAL_CMD = 0x0400, + SCST_FULLY_LOCAL_CMD = 0x0800, - SCST_REG_RESERVE_ALLOWED = 0x0800, - SCST_WRITE_EXCL_ALLOWED = 0x1000, - SCST_EXCL_ACCESS_ALLOWED = 0x2000, + SCST_REG_RESERVE_ALLOWED = 0x1000, + SCST_WRITE_EXCL_ALLOWED = 0x4000, + SCST_EXCL_ACCESS_ALLOWED = 0x4000, #ifdef CONFIG_SCST_TEST_IO_IN_SIRQ - SCST_TEST_IO_IN_SIRQ_ALLOWED = 0x4000, + SCST_TEST_IO_IN_SIRQ_ALLOWED = 0x8000, #endif - SCST_SERIALIZED = 0x8000, - SCST_STRICTLY_SERIALIZED = 0x10000|SCST_SERIALIZED, - SCST_DESCRIPTORS_BASED = 0x20000, + SCST_SERIALIZED = 0x10000, + SCST_STRICTLY_SERIALIZED = 0x20000|SCST_SERIALIZED, + SCST_DESCRIPTORS_BASED = 0x40000, }; /************************************************************* diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index cd83a5ff7..55fcd03d5 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -1195,9 +1195,8 @@ static bool vdisk_parse_offset(struct vdisk_cmd_params *p, struct scst_cmd *cmd) EXTRACHECKS_BUG_ON((loff < 0) || unlikely(data_len < 0)); - /* If lba_len is 0, loff doesn't make sense, so this error not possible */ if (unlikely((loff + data_len) > virt_dev->file_size) && - (cmd->lba_len != 0)) { + (!(cmd->op_flags & SCST_LBA_NOT_VALID))) { PRINT_INFO("Access beyond the end of the device " "(%lld of %lld, data len %lld)", (long long unsigned int)loff, diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 1c9a456f1..18e85319f 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -6015,9 +6015,12 @@ static int get_cdb_info_len_10(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { cmd->cdb_len = 10; + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); + cmd->data_len = cmd->bufflen; return 0; } @@ -6025,6 +6028,7 @@ static int get_cdb_info_len_10(struct scst_cmd *cmd, static int get_cdb_info_block_limit(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; cmd->bufflen = 6; cmd->data_len = cmd->bufflen; @@ -6034,6 +6038,7 @@ static int get_cdb_info_block_limit(struct scst_cmd *cmd, static int get_cdb_info_read_capacity(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; cmd->bufflen = 8; cmd->data_len = cmd->bufflen; @@ -6053,7 +6058,7 @@ static int get_cdb_info_serv_act_in(struct scst_cmd *cmd, case SAI_READ_CAPACITY_16: cmd->op_name = "READ CAPACITY(16)"; cmd->bufflen = get_unaligned_be32(&cmd->cdb[10]); - cmd->op_flags |= SCST_IMPLICIT_HQ | + cmd->op_flags |= SCST_IMPLICIT_HQ | SCST_LBA_NOT_VALID | SCST_REG_RESERVE_ALLOWED | SCST_WRITE_EXCL_ALLOWED | SCST_EXCL_ACCESS_ALLOWED; @@ -6065,7 +6070,7 @@ static int get_cdb_info_serv_act_in(struct scst_cmd *cmd, cmd->op_flags |= SCST_WRITE_EXCL_ALLOWED; break; default: - cmd->op_flags |= SCST_UNKNOWN_LENGTH; + cmd->op_flags |= SCST_UNKNOWN_LENGTH | SCST_LBA_NOT_VALID; break; } @@ -6078,6 +6083,7 @@ static int get_cdb_info_serv_act_in(struct scst_cmd *cmd, static int get_cdb_info_single(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; cmd->bufflen = 1; cmd->data_len = cmd->bufflen; @@ -6089,7 +6095,9 @@ static int get_cdb_info_read_pos(struct scst_cmd *cmd, { int res = 0; + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + cmd->bufflen = get_unaligned_be16(cmd->cdb + sdbops->info_len_off); switch (cmd->cdb[1] & 0x1f) { @@ -6137,7 +6145,9 @@ out_inval: static int get_cdb_info_prevent_allow_medium_removal(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + cmd->data_len = 0; /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); @@ -6150,7 +6160,9 @@ static int get_cdb_info_prevent_allow_medium_removal(struct scst_cmd *cmd, static int get_cdb_info_start_stop(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + cmd->data_len = 0; /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); @@ -6163,7 +6175,9 @@ static int get_cdb_info_start_stop(struct scst_cmd *cmd, static int get_cdb_info_len_3_read_elem_stat(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + cmd->bufflen = get_unaligned_be24(cmd->cdb + sdbops->info_len_off); cmd->data_len = cmd->bufflen; @@ -6186,7 +6200,9 @@ static int get_cdb_info_bidi_lba_4_len_2(struct scst_cmd *cmd, static int get_cdb_info_fmt(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + if (cmd->cdb[1] & 0x10/*FMTDATA*/) { cmd->data_direction = SCST_DATA_WRITE; cmd->op_flags |= SCST_UNKNOWN_LENGTH; @@ -6213,7 +6229,9 @@ static int get_cdb_info_verify10(struct scst_cmd *cmd, static int get_cdb_info_verify6(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + if (cmd->cdb[1] & BYTCHK) { cmd->bufflen = get_unaligned_be24(cmd->cdb + sdbops->info_len_off); cmd->data_len = cmd->bufflen; @@ -6261,7 +6279,9 @@ static int get_cdb_info_verify16(struct scst_cmd *cmd, static int get_cdb_info_len_1(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + cmd->bufflen = (u32)cmd->cdb[sdbops->info_len_off]; cmd->data_len = cmd->bufflen; return 0; @@ -6290,6 +6310,7 @@ static int get_cdb_info_lba_2_len_1_256(struct scst_cmd *cmd, static int get_cdb_info_len_2(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; cmd->bufflen = get_unaligned_be16(cmd->cdb + sdbops->info_len_off); cmd->data_len = cmd->bufflen; @@ -6299,6 +6320,7 @@ static int get_cdb_info_len_2(struct scst_cmd *cmd, static int get_cdb_info_len_3(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; cmd->bufflen = get_unaligned_be24(cmd->cdb + sdbops->info_len_off); cmd->data_len = cmd->bufflen; @@ -6308,6 +6330,7 @@ static int get_cdb_info_len_3(struct scst_cmd *cmd, static int get_cdb_info_len_4(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; cmd->bufflen = get_unaligned_be32(cmd->cdb + sdbops->info_len_off); cmd->data_len = cmd->bufflen; @@ -6317,9 +6340,12 @@ static int get_cdb_info_len_4(struct scst_cmd *cmd, static int get_cdb_info_none(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { + cmd->op_flags |= SCST_LBA_NOT_VALID; cmd->lba = 0; + /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); + cmd->data_len = cmd->bufflen; return 0; } @@ -6328,8 +6354,10 @@ static int get_cdb_info_lba_2_none(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { cmd->lba = get_unaligned_be16(cmd->cdb + sdbops->info_lba_off); + /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); + cmd->data_len = cmd->bufflen; return 0; } @@ -6338,8 +6366,10 @@ static int get_cdb_info_lba_4_none(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { cmd->lba = get_unaligned_be32(cmd->cdb + sdbops->info_lba_off); + /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); + cmd->data_len = cmd->bufflen; return 0; } @@ -6348,8 +6378,10 @@ static int get_cdb_info_lba_8_none(struct scst_cmd *cmd, const struct scst_sdbops *sdbops) { cmd->lba = get_unaligned_be64(cmd->cdb + sdbops->info_lba_off); + /* It supposed to be already zeroed */ EXTRACHECKS_BUG_ON(cmd->bufflen != 0); + cmd->data_len = cmd->bufflen; return 0; } @@ -7202,6 +7234,8 @@ int scst_set_cdb_lba(struct scst_cmd *cmd, int64_t lba) TRACE_ENTRY(); + EXTRACHECKS_BUG_ON(cmd->op_flags & SCST_LBA_NOT_VALID); + scst_set_cdb_lba_fns[cmd->lba_len](cmd, lba); res = 0; diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 671cf2888..a538b6729 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -891,7 +891,8 @@ out: * at least 0 for not data transfer commands, or cmd must be * completed (with an error) and have correct state set. */ - if (unlikely(((cmd->lba == SCST_DEF_LBA_DATA_LEN) || + if (unlikely((((cmd->lba == SCST_DEF_LBA_DATA_LEN) && + !(cmd->op_flags & SCST_LBA_NOT_VALID)) || (cmd->data_len == SCST_DEF_LBA_DATA_LEN)) && (!cmd->completed || (((cmd->state < SCST_CMD_STATE_PRE_XMIT_RESP) ||