Hi Bryant, On Thu, 2017-03-30 at 09:47 -0500, Bryant G. Ly wrote: > This adds PGR support for just TCMU, since tcmu doesn't > have the necessary IT_NEXUS info to process PGR in userspace, > so have those commands be processed in kernel. I don't have a objection to adding PASSTHROUGH_PGR so TCMU can use target_core_pr.c logic, following what MNC has already done with PASSTHROUGH_ALUA. Just one comment on the patch below. > > Signed-off-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx> > --- > drivers/target/target_core_configfs.c | 10 +++++----- > drivers/target/target_core_device.c | 27 +++++++++++++++++++++++++++ > drivers/target/target_core_pr.c | 2 +- > drivers/target/target_core_pscsi.c | 3 ++- > include/target/target_core_backend.h | 1 + > 5 files changed, 36 insertions(+), 7 deletions(-) > <SNIP> > diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c > index c754ae3..83c0d77 100644 > --- a/drivers/target/target_core_device.c > +++ b/drivers/target/target_core_device.c > @@ -1076,6 +1077,32 @@ passthrough_parse_cdb(struct se_cmd *cmd, > return TCM_NO_SENSE; > } > > + /* > + * For PERSISTENT RESERVE IN/OUT, RELEASE, and RESERVE we need to > + * emulate the response, since tcmu does not have the information > + * required to process these commands. > + */ > + if (!(dev->transport->transport_flags & > + TRANSPORT_FLAG_PASSTHROUGH_PGR)) { > + if (cdb[0] == PERSISTENT_RESERVE_IN) { > + cmd->execute_cmd = target_scsi3_emulate_pr_in; > + return TCM_NO_SENSE; > + } > + if (cdb[0] == PERSISTENT_RESERVE_OUT) { > + cmd->execute_cmd = target_scsi3_emulate_pr_out; > + return TCM_NO_SENSE; > + } > + > + if (cdb[0] == RELEASE || cdb[0] == RELEASE_10) { > + cmd->execute_cmd = target_scsi2_reservation_release; > + return TCM_NO_SENSE; > + } > + if (cdb[0] == RESERVE || cdb[0] == RESERVE_10) { > + cmd->execute_cmd = target_scsi2_reservation_reserve; > + return TCM_NO_SENSE; > + } > + } > + Following what spc_parse_cdb() does wrt extracting the allocation length out of the CDB, calling target_cmd_size_check() to check for overflow + underflow + residual_count vs. fabric provided se_cmd->data_length, I'm wonder if we're getting to the point in passthrough_parse_cdb() where target_cmd_size_check() should be happening too. Historically, we've never done these checks for PSCSI, but given TCMU will be using this path for setting up full in-kernel PR, it's probably a good idea to do target_cmd_size_check() for the PASSTHROUGH_PGR special cases here. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html