Fix two issues with AllRegistrants reservations where the code didn't
handle all of the registered devices as reservation holders.
At the same time, introduce a helper function named
'is_reservation_holder()' that properly checks if a device is a
reservation holder, taking into account the reservation type. This
function cleans up the code and improves readability.
Signed-off-by: Ilias Tsitsimpis <iliastsi@xxxxxxxxxxx>
Signed-off-by: Vangelis Koukis <vkoukis@xxxxxxxxxxx>
---
Hi Nicholas,
Continuing our conversation from December 19, at thread "[PATCH 0/2]
target: Fixes for AllRegistrants reservation handling" I identified two
more cases where the code didn't correctly handle ALLREG. Since you said
that you would prefer identify any remaining ALLREG specific issues and
address them individually, I have created this patch to address the
above issues.
At the same time this patch introduces the function
'is_reservation_holder()' which properly checks if a device is a
reservation holder, correctly handling the ALLREG cases. This function
cleans up the code and improves readability.
Thanks,
Ilias
drivers/target/target_core_pr.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 2de6fb8..ca9fa61 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -78,6 +78,22 @@ enum preempt_type {
static void __core_scsi3_complete_pro_release(struct se_device *, struct se_node_acl *,
struct t10_pr_registration *, int, int);
+static inline int is_reservation_holder(
+ struct t10_pr_registration *pr_res_holder,
+ struct t10_pr_registration *pr_reg)
+{
+ int pr_res_type;
+
+ if (pr_res_holder) {
+ pr_res_type = pr_res_holder->pr_res_type;
+
+ return pr_res_holder == pr_reg ||
+ pr_res_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG ||
+ pr_res_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG;
+ }
+ return 0;
+}
+
static sense_reason_t
target_scsi2_reservation_check(struct se_cmd *cmd)
{
@@ -1825,6 +1841,7 @@ static int core_scsi3_update_aptpl_buf(
ssize_t len = 0;
int reg_count = 0;
int ret = 0;
+ struct t10_pr_registration *pr_res_holder = dev->dev_pr_res_holder;
spin_lock(&dev->dev_reservation_lock);
spin_lock(&dev->t10_pr.registration_lock);
@@ -1849,7 +1866,7 @@ static int core_scsi3_update_aptpl_buf(
* Include special metadata if the pr_reg matches the
* reservation holder.
*/
- if (dev->dev_pr_res_holder == pr_reg) {
+ if (is_reservation_holder(pr_res_holder, pr_reg)) {
snprintf(tmp, 512, "PR_REG_START: %d"
"\ninitiator_fabric=%s\n"
"initiator_node=%s\n%s"
@@ -1859,8 +1876,9 @@ static int core_scsi3_update_aptpl_buf(
"mapped_lun=%u\n", reg_count,
tpg->se_tpg_tfo->get_fabric_name(),
pr_reg->pr_reg_nacl->initiatorname, isid_buf,
- pr_reg->pr_res_key, pr_reg->pr_res_type,
- pr_reg->pr_res_scope, pr_reg->pr_reg_all_tg_pt,
+ pr_reg->pr_res_key, pr_res_holder->pr_res_type,
+ pr_res_holder->pr_res_scope,
+ pr_reg->pr_reg_all_tg_pt,
pr_reg->pr_res_mapped_lun);
} else {
snprintf(tmp, 512, "PR_REG_START: %d\n"
@@ -2287,7 +2305,6 @@ core_scsi3_pro_reserve(struct se_cmd *cmd, int type, int scope, u64 res_key)
spin_lock(&dev->dev_reservation_lock);
pr_res_holder = dev->dev_pr_res_holder;
if (pr_res_holder) {
- int pr_res_type = pr_res_holder->pr_res_type;
/*
* From spc4r17 Section 5.7.9: Reserving:
*
@@ -2298,9 +2315,7 @@ core_scsi3_pro_reserve(struct se_cmd *cmd, int type, int scope, u64 res_key)
* the logical unit, then the command shall be completed with
* RESERVATION CONFLICT status.
*/
- if ((pr_res_holder != pr_reg) &&
- (pr_res_type != PR_TYPE_WRITE_EXCLUSIVE_ALLREG) &&
- (pr_res_type != PR_TYPE_EXCLUSIVE_ACCESS_ALLREG)) {
+ if (!is_reservation_holder(pr_res_holder, pr_reg)) {
struct se_node_acl *pr_res_nacl = pr_res_holder->pr_reg_nacl;
pr_err("SPC-3 PR: Attempted RESERVE from"
" [%s]: %s while reservation already held by"
@@ -2477,7 +2492,6 @@ core_scsi3_emulate_pro_release(struct se_cmd *cmd, int type, int scope,
struct se_lun *se_lun = cmd->se_lun;
struct t10_pr_registration *pr_reg, *pr_reg_p, *pr_res_holder;
struct t10_reservation *pr_tmpl = &dev->t10_pr;
- int all_reg = 0;
sense_reason_t ret = 0;
if (!se_sess || !se_lun) {
@@ -2514,13 +2528,9 @@ core_scsi3_emulate_pro_release(struct se_cmd *cmd, int type, int scope,
spin_unlock(&dev->dev_reservation_lock);
goto out_put_pr_reg;
}
- if ((pr_res_holder->pr_res_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG) ||
- (pr_res_holder->pr_res_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG))
- all_reg = 1;
- if ((all_reg == 0) && (pr_res_holder != pr_reg)) {
+ if (!is_reservation_holder(pr_res_holder, pr_reg)) {
/*
- * Non 'All Registrants' PR Type cases..
* Release request from a registered I_T nexus that is not a
* persistent reservation holder. return GOOD status.
*/
@@ -3375,7 +3385,7 @@ after_iport_check:
* From spc4r17 section 5.7.8 Table 50 --
* Register behaviors for a REGISTER AND MOVE service action
*/
- if (pr_res_holder != pr_reg) {
+ if (!is_reservation_holder(pr_res_holder, pr_reg)) {
pr_warn("SPC-3 PR REGISTER_AND_MOVE: Calling I_T"
" Nexus is not reservation holder\n");
spin_unlock(&dev->dev_reservation_lock);
--
2.1.4
Attachment:
signature.asc
Description: Digital signature
