I created a small Persistent Group Reservations test suite, available
at git://github.com/gonzoleeman/open-iscsi-pgr-validate. It uncovered
several bugs in the SCSI target's PGR implementation. Here is a list
of the tests that failed or could not run:
Reserve Exclusive Access:
- Can release reservation: ERROR
- Unregister Releases Reservation: FAIL
Reserve Write Exclusive:
- Resvn Holder Unregister Releases Resvn: FAIL
- Non-Registrant Does Not Have Read Access: FAIL
- Non-Resvn-Holder Has Read Access: FAIL
Reserve Exclusive Access Registrants Only:
- Unregister Releases Reservation: FAIL
Reserve Write Exclusive Registrants Only:
- Resvn Holder Unregister releases resvn: FAIL
Reserve Exclusive Access All Registrants:
- Alternative Resvn Holder Can Release Resvn: FAIL
- Main Resvn Unregister Does Not Release Resvn: FAIL
Reserve Write Exclusive All Registrants:
- Alt Resvn Holder Can Release Resvn: FAIL
- Main Resvn Holder Unregister does not
release resvn: FAIL
This patch enables all PGR tests is said suite to pass.
Signed-off-by: Lee Duncan <lduncan@xxxxxxxx>
---
usr/list.h | 6 ++++
usr/spc.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/usr/list.h b/usr/list.h
index 2f80a56..819bba9 100644
--- a/usr/list.h
+++ b/usr/list.h
@@ -37,6 +37,12 @@ static inline int list_empty(const struct list_head *head)
return head->next == head;
}
+static inline int list_is_last(const struct list_head *list,
+ const struct list_head *head)
+{
+ return list->next == head;
+}
+
#define list_entry(ptr, type, member) \
container_of(ptr, type, member)
diff --git a/usr/spc.c b/usr/spc.c
index 9fb5a6d..24d449e 100644
--- a/usr/spc.c
+++ b/usr/spc.c
@@ -109,6 +109,15 @@
#define DESG_MD5 7
#define DESG_SCSI 8
+static inline int is_pr_type_all_registrants(uint8_t pr_type)
+{
+ if ((pr_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG) ||
+ (pr_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG))
+ return 1;
+ return 0;
+}
+
+
static void update_vpd_80(struct scsi_lu *lu, void *sn)
{
struct vpd *vpd_pg = lu->attrs.lu_vpd[PCODE_OFFSET(0x80)];
@@ -867,8 +876,7 @@ int spc_service_action(int host_no, struct scsi_cmd *cmd)
static int is_pr_holder(struct scsi_lu *lu, struct registration *reg)
{
- if (lu->pr_holder->pr_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG ||
- lu->pr_holder->pr_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG)
+ if (is_pr_type_all_registrants(lu->pr_holder->pr_type))
return 1;
if (lu->pr_holder == reg)
@@ -956,8 +964,7 @@ static int spc_pr_read_reservation(int host_no, struct scsi_cmd *cmd)
if (reg) {
put_unaligned_be32(16, &buf[4]);
- if (reg->pr_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG ||
- reg->pr_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG)
+ if (is_pr_type_all_registrants(reg->pr_type))
res_key = 0;
else
res_key = reg->key;
@@ -1055,6 +1062,56 @@ static void __unregister(struct scsi_lu *lu, struct registration *reg)
free(reg);
}
+static void __unregister_and_clean(struct scsi_cmd *cmd,
+ struct registration *reg)
+{
+ struct registration *holder, *sibling;
+
+ /* if reservation owner goes away then so does reservation */
+ holder = cmd->dev->pr_holder;
+ if (!holder)
+ goto unregister_and_go;
+
+ if (!is_pr_holder(cmd->dev, reg))
+ goto unregister_and_go;
+
+ if (!is_pr_type_all_registrants(holder->pr_type) ||
+ list_is_last(®->registration_siblings,
+ ®->registration_siblings)) {
+ holder->pr_scope = 0;
+ holder->pr_type = 0;
+ cmd->dev->pr_holder = NULL;
+
+ list_for_each_entry(sibling,
+ &cmd->dev->registration_list,
+ registration_siblings) {
+ /* do not send UA to self */
+ if (sibling == reg)
+ continue;
+ ua_sense_add_other_it_nexus(sibling->nexus_id,
+ cmd->dev,
+ ASC_RESERVATIONS_RELEASED);
+ }
+ cmd->dev->prgeneration++;
+ } else {
+ /* all-registrants, and not last registrant */
+ list_for_each_entry(sibling,
+ &cmd->dev->registration_list,
+ registration_siblings) {
+ if (sibling != reg) {
+ /* give resvn to any sibling */
+ cmd->dev->pr_holder = sibling;
+ sibling->pr_scope = holder->pr_scope;
+ sibling->pr_type = holder->pr_type;
+ break;
+ }
+ }
+ }
+
+unregister_and_go:
+ __unregister(cmd->dev, reg);
+}
+
static int check_pr_out_basic_parameter(struct scsi_cmd *cmd)
{
uint8_t spec_i_pt, all_tg_pt, aptpl;
@@ -1110,7 +1167,7 @@ static int spc_pr_register(int host_no, struct scsi_cmd *cmd)
if (sa_res_key)
reg->key = sa_res_key;
else
- __unregister(cmd->dev, reg);
+ __unregister_and_clean(cmd, reg);
} else
return SAM_STAT_RESERVATION_CONFLICT;
} else {
@@ -1235,7 +1292,8 @@ static int spc_pr_release(int host_no, struct scsi_cmd *cmd)
if (res_key != reg->key)
return SAM_STAT_RESERVATION_CONFLICT;
- if (reg->pr_scope != pr_scope || reg->pr_type != pr_type) {
+ if (holder->pr_scope != pr_scope ||
+ holder->pr_type != pr_type) {
asc = ASC_INVALID_RELEASE_OF_PERSISTENT_RESERVATION;
goto sense;
}
@@ -1359,8 +1417,7 @@ static int spc_pr_preempt(int host_no, struct scsi_cmd *cmd)
holder = cmd->dev->pr_holder;
if (holder) {
- if (holder->pr_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG ||
- holder->pr_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG) {
+ if (is_pr_type_all_registrants(holder->pr_type)) {
if (!sa_res_key) {
if (pr_type != holder->pr_type ||
@@ -1520,9 +1577,10 @@ int spc_access_check(struct scsi_cmd *cmd)
pr_type = cmd->dev->pr_holder->pr_type;
bits = cmd->dev->dev_type_template.ops[op].pr_conflict_bits;
- if (pr_type == PR_TYPE_WRITE_EXCLUSIVE ||
- pr_type == PR_TYPE_EXCLUSIVE_ACCESS)
- conflict = bits & (PR_WE_FA|PR_EA_FA);
+ if (pr_type == PR_TYPE_EXCLUSIVE_ACCESS)
+ conflict = bits & PR_EA_FA;
+ else if (pr_type == PR_TYPE_WRITE_EXCLUSIVE)
+ conflict = bits & PR_WE_FA;
else {
if (reg)
conflict = bits & PR_RR_FR;
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
