On Sat, 2015-03-21 at 22:26 +0200, Ilias Tsitsimpis wrote:
> On Fri, Mar 20, 2015 at 08:00PM, Nicholas A. Bellinger wrote:
<SNIP>
>
> > > @@ -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);
> >
> > For register_and_move code this check is redundant btw, because an
> > ALL_REG check already exists immediately after this one. I'd prefer to
> > keep the existing comment + check for this special case. Please drop
> > this part too.
>
> I don't believe this check is redundant because it changes the warning
> message that the user sees. Without this patch, if one tries the
> REGISTER AND MOVE service action while holding an ALL_REG reservation
> (but is not the same one with dev->dev_pr_res_holder) then the user will
> see the warning message "Nexus is not reservation holder" which is
> wrong. With the above change, the user will see the correct warning
> message "Unable to move reservation for type ALL_REG". Should I leave
> this part as is?
Ah, yes. You're most certainly correct.
Please respin -v2 with the register_and_move() conversion as well..
Thanks Ilias!
--nab
--
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
