| Message ID | 50b7c908-05f3-f90c-87a6-efbdc0a10956@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2017-06-02 at 11:45 +0800, Jiang Yi wrote: > Hi Nic, > > in struct se_dev_attrib, there is a field emulate_caw. > > If this field is set zero, does it mean that the corresponding struct se_device does not support the scsi cmd COMPARE_AND_WRITE? > > Do fields like emulate_tpws, emulate_tpu, emulate_tas... imply "enable support for thin provision write same", "enable support for > thin provision unmap", "enable support for task aborted status"? > > In function sbc_parse_cdb(), it rejects scsi cmd UNMAP if emulate_tpu is not set. So I think we should also reject COMPARE_AND_WRITE if emulate_caw is not set. > > I propose a patch: > > Signed-off-by: Jiang Yi <jiangyilism@gmail.com> > --- > drivers/target/target_core_sbc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c > index 4316f7b..bdea345 100644 > --- a/drivers/target/target_core_sbc.c > +++ b/drivers/target/target_core_sbc.c > @@ -1005,6 +1005,12 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) > break; > } > case COMPARE_AND_WRITE: > + if (!dev->dev_attrib.emulate_caw) { > + pr_err("se_device %s/%s (vpd_unit_serial %s) reject COMPARE_AND_WRITE\n", > + dev->se_hba->backend->ops->name, dev->dev_group.cg_item.ci_name, > + dev->t10_wwn.unit_serial); > + return TCM_UNSUPPORTED_SCSI_OPCODE; > + } > sectors = cdb[13]; > /* > * Currently enforce COMPARE_AND_WRITE for a single sector Applied to target-pending/for-next, with a slightly improved commit log. Also, all new pr_err() should be pr_err_ratelimited(). Fixed that up before applying. Wrt to your comments, I'd be open to consider a wrapper that dumps more verbose se_device specific information for these types of CDB errors. It would be a good excuse to convert all of the pr_[err,warn]() to *_ratelimited() too. ;) -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 4316f7b..bdea345 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -1005,6 +1005,12 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) break; } case COMPARE_AND_WRITE: + if (!dev->dev_attrib.emulate_caw) { + pr_err("se_device %s/%s (vpd_unit_serial %s) reject COMPARE_AND_WRITE\n", + dev->se_hba->backend->ops->name, dev->dev_group.cg_item.ci_name, + dev->t10_wwn.unit_serial); + return TCM_UNSUPPORTED_SCSI_OPCODE; + } sectors = cdb[13]; /* * Currently enforce COMPARE_AND_WRITE for a single sector
Hi Nic, in struct se_dev_attrib, there is a field emulate_caw. If this field is set zero, does it mean that the corresponding struct se_device does not support the scsi cmd COMPARE_AND_WRITE? Do fields like emulate_tpws, emulate_tpu, emulate_tas... imply "enable support for thin provision write same", "enable support for thin provision unmap", "enable support for task aborted status"? In function sbc_parse_cdb(), it rejects scsi cmd UNMAP if emulate_tpu is not set. So I think we should also reject COMPARE_AND_WRITE if emulate_caw is not set. I propose a patch: Signed-off-by: Jiang Yi <jiangyilism@gmail.com> --- drivers/target/target_core_sbc.c | 6 ++++++ 1 file changed, 6 insertions(+)