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

[Alexander Nezhinsky]

On Sun, Dec 23, 2012 at 3:56 AM, Lee Duncan <lduncan at suse.com> wrote:
> 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.
>

checking for "single entry in the list" can be done simply as:
(list->next == head && list->prev == head)
even without defining a special macro, by using reg->registration_siblings
and &cmd->dev->registration_list as entry and head respectively.

>> 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.
>
> 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".

no dislike for refactoring (to the contrary) - just suggested to make a separate
patch for clarity.
--
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