Re: problem with PERSISTENT RESERVE OUT - REGISTER

[Nicholas A. Bellinger]

Hello Gera!

(Cc'ing target-devel list)

On Mon, 2013-09-09 at 08:29 -0700, Gera Kazakov wrote:
> The PERSISTENT RESERVE OUT/REGISTER returns "LUN not ready" status
> however it does register (or unregister) the key.
>
>
> This patch fixes the problem:
>

Thanks alot for your patch.

So it looks like this bug was originally introduced during v3.9 in
commit (Viro CC'ed):

commit 0e9b10a90f1c30f25dd6f130130240745ab14010
Author: Al Viro <vi...@zeniv.linux.org.uk>
Date: Sat Feb 23 15:22:43 2013 -0500

target: writev() on single-element vector is pointless

However, given that the return of core_scsi3_update_and_write_aptpl()
was not used to determine if a command should be returned with non GOOD
status, this bug was not being triggered until v3.11-rc1 by commit
(Grover CC'ed):

commit 459f213ba162bd13e113d6f92a8fa6c780fd67ed
Author: Andy Grover <agr...@redhat.com>
Date: Thu May 16 10:41:02 2013 -0700

target: Allocate aptpl_buf inside update_and_write_aptpl()

Looking at it now, it's obvious that Grover didn't test this code with
in either APTPL=[1,0] modes. :-(

In any event, the same bug exists core_alua_write_tpg_metadata() as well
from commit 0e9b10a90f1.

I'm adding this into your patch below, and will send out a version with
the ALUA fix shortly.

>
>
> diff --git a/drivers/target/target_core_pr.c
> b/drivers/target/target_core_pr.c
> index bd78faf..adec5a8 100644
> --- a/drivers/target/target_core_pr.c
> +++ b/drivers/target/target_core_pr.c
> @@ -1949,7 +1949,7 @@ static int __core_scsi3_write_aptpl_to_file(
> pr_debug("Error writing APTPL metadata file: %s\n",
> path);
> fput(file);
>
>
> - return ret ? -EIO : 0;
> + return (ret < 0) ? -EIO : 0;
> }
>
>
>
>
> There actually another problem - if __core_scsi3_write_aptpl_to_file
> fails for real, then legitimate "not ready" status would be returned
> to the host however key registration would not be reverted.
>
>

Indeed. I need to think a bit more about what to do for this case, but
am going to merge your bugfix + extra ALUA bugfix for now.

Thank you,

--nab