[stgt] [PATCH] fix bugs in persistent group reservations

[Lee Duncan]

On 12/22/2012 01:47 PM, Alexander Nezhinsky wrote:
>> +static inline int list_is_last(const struct list_head *list,
>> +                              const struct list_head *head)
>> +{
>> +       return list->next == head;
>> +}
> 
> Can't we just use list_add() instead of list_add_tail() in spc_pr_register():
>       list_add_tail(&reg->registration_siblings,
>                         &cmd->dev->registration_list);
> then we don't need to introduce list_is_last() and manage it with
> list_first_entry() ?

I believe I implemented this incorrectly. What I really wanted to know
was "is this the only entry in the last", not "is this the last entry
in the list of possibly many entries".

I will rework the patch. Right now, my patch looks to see if the registration
that is about to be deleted is the last one. But I believe it would be
better to delete the registration, then check to see if the list is
empty. Then I can use the existing list_empty() function.

I don't believe using list_add() vs list_add_tail() is relevant given
these facts.

> 
> 
>> +static void __unregister_and_clean(struct scsi_cmd *cmd,
>> +                                  struct registration *reg)
>> +{
> . . .
>> +       if (!is_pr_type_all_registrants(holder->pr_type) ||
>> +            list_is_last(&reg->registration_siblings,
>> +                &reg->registration_siblings)) {
> 
> Isn't it supposed to be:
>> +            list_is_last(&reg->registration_siblings,
>> +                &cmd->dev->registration_list)) {
> ?

Yes, I believe you are right, but that code will change with an
updated patch.

> 
> Another thing, i'd suggest making a separate patch for
> is_pr_type_all_registrants()
> because it is a refactoring-like stuff and quite separate from other additions
> and fixes.
> 
> Alexander


I will remove the is_pr_type_all_registrants() function, obviating
the need for a second patch. I thought it made the code more readable,
but I can understand your dislike for "refactoring-like stuff".

I will submit an updated patch soon.
-- 
Lee Duncan
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html