All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
@ 2019-04-02 19:58 Bart Van Assche
  2019-04-08 20:04 ` Mike Christie
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Bart Van Assche @ 2019-04-02 19:58 UTC (permalink / raw)
  To: target-devel

While testing with the libiscsi tool I noticed after the tool had stopped
and hence after it had logged out that an SPC-2 reservation was still active:

$ (cd /sys/kernel/config/target/core && find -name res_holder|xargs grep -aH .)
./pscsi_0/vdev3/pr/res_holder:Passthrough
./iblock_0/vdev2/pr/res_holder:No SPC-3 Reservation holder
./fileio_1/vdev1/pr/res_holder:SPC-2 Reservation: iSCSI Initiator: iqn.2007-10.com.github:sahlberg:libiscsi:iscsi-test-2
./fileio_0/vdev0/pr/res_holder:No SPC-3 Reservation holder

This is a bug. SPC-2 reservations must be cleared when an initiator
logs out. This patch fixes that bug. A quote from SPC-2 illustrates this:
"Reservations managed using the reserve/release method do not persist
across some recovery actions (e.g., hard resets). When a target performs
one of these recovery actions, the application client(s) have to rediscover
the configuration and re-establish the required reservations."

Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/target/target_core_transport.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index e3f7e21e6614..93ef5c6362d6 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -550,6 +550,15 @@ void transport_deregister_session_configfs(struct se_session *se_sess)
 }
 EXPORT_SYMBOL(transport_deregister_session_configfs);
 
+static int target_sess_release_reservation(struct se_device *dev, void *data)
+{
+	struct se_session *sess = data;
+
+	if (dev->reservation_holder = sess)
+		target_release_reservation(dev);
+	return 0;
+}
+
 void transport_free_session(struct se_session *se_sess)
 {
 	struct se_node_acl *se_nacl = se_sess->se_node_acl;
@@ -592,6 +601,7 @@ void transport_free_session(struct se_session *se_sess)
 		sbitmap_queue_free(&se_sess->sess_tag_pool);
 		kvfree(se_sess->sess_cmd_map);
 	}
+	target_for_each_device(target_sess_release_reservation, se_sess);
 	percpu_ref_exit(&se_sess->cmd_count);
 	kmem_cache_free(se_sess_cache, se_sess);
 }
-- 
2.21.0.196.g041f5ea1cf98

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
  2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
@ 2019-04-08 20:04 ` Mike Christie
  2019-04-08 20:17 ` Bart Van Assche
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Mike Christie @ 2019-04-08 20:04 UTC (permalink / raw)
  To: target-devel

On 04/02/2019 02:58 PM, Bart Van Assche wrote:
> While testing with the libiscsi tool I noticed after the tool had stopped
> and hence after it had logged out that an SPC-2 reservation was still active:
> 
> $ (cd /sys/kernel/config/target/core && find -name res_holder|xargs grep -aH .)
> ./pscsi_0/vdev3/pr/res_holder:Passthrough
> ./iblock_0/vdev2/pr/res_holder:No SPC-3 Reservation holder
> ./fileio_1/vdev1/pr/res_holder:SPC-2 Reservation: iSCSI Initiator: iqn.2007-10.com.github:sahlberg:libiscsi:iscsi-test-2
> ./fileio_0/vdev0/pr/res_holder:No SPC-3 Reservation holder
> 
> This is a bug. SPC-2 reservations must be cleared when an initiator
> logs out. This patch fixes that bug. A quote from SPC-2 illustrates this:
> "Reservations managed using the reserve/release method do not persist
> across some recovery actions (e.g., hard resets). When a target performs
> one of these recovery actions, the application client(s) have to rediscover
> the configuration and re-establish the required reservations."
> 
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Nicholas Bellinger <nab@linux-iscsi.org>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/target/target_core_transport.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index e3f7e21e6614..93ef5c6362d6 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -550,6 +550,15 @@ void transport_deregister_session_configfs(struct se_session *se_sess)
>  }
>  EXPORT_SYMBOL(transport_deregister_session_configfs);
>  
> +static int target_sess_release_reservation(struct se_device *dev, void *data)
> +{
> +	struct se_session *sess = data;
> +
> +	if (dev->reservation_holder = sess)
> +		target_release_reservation(dev);
> +	return 0;
> +}
> +
>  void transport_free_session(struct se_session *se_sess)
>  {
>  	struct se_node_acl *se_nacl = se_sess->se_node_acl;
> @@ -592,6 +601,7 @@ void transport_free_session(struct se_session *se_sess)
>  		sbitmap_queue_free(&se_sess->sess_tag_pool);
>  		kvfree(se_sess->sess_cmd_map);
>  	}
> +	target_for_each_device(target_sess_release_reservation, se_sess);
>  	percpu_ref_exit(&se_sess->cmd_count);
>  	kmem_cache_free(se_sess_cache, se_sess);
>  }
> 

Was the original code done for iscsi? We have this in
https://tools.ietf.org/html/rfc7143:

4.4.3.2. Reservations

....

In contrast, [SPC2] does not specify detailed persistence
requirements for reserve/release reservation state after an I_T nexus
failure. Nonetheless, when reserve/release reservations are
supported by an iSCSI target, the preferred implementation approach
is to preserve reserve/release reservation state for iSCSI session
reinstatement (see Section 6.3.5) or session continuation (see
Section 6.3.6).


So for example for session reinstatement if you pull a network cable
then plug it back in the iscsi target can do a
iscsit_close_session->transport_deregister_session ->
transport_free_session. Above we will now release the reservation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
  2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
  2019-04-08 20:04 ` Mike Christie
@ 2019-04-08 20:17 ` Bart Van Assche
  2019-04-08 22:41 ` Mike Christie
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2019-04-08 20:17 UTC (permalink / raw)
  To: target-devel

On Mon, 2019-04-08 at 15:04 -0500, Mike Christie wrote:
+AD4 On 04/02/2019 02:58 PM, Bart Van Assche wrote:
+AD4 +AD4 While testing with the libiscsi tool I noticed after the tool had stopped
+AD4 +AD4 and hence after it had logged out that an SPC-2 reservation was still active:
+AD4 +AD4 
+AD4 +AD4 +ACQ (cd /sys/kernel/config/target/core +ACYAJg find -name res+AF8-holder+AHw-xargs grep -aH .)
+AD4 +AD4 ./pscsi+AF8-0/vdev3/pr/res+AF8-holder:Passthrough
+AD4 +AD4 ./iblock+AF8-0/vdev2/pr/res+AF8-holder:No SPC-3 Reservation holder
+AD4 +AD4 ./fileio+AF8-1/vdev1/pr/res+AF8-holder:SPC-2 Reservation: iSCSI Initiator: iqn.2007-10.com.github:sahlberg:libiscsi:iscsi-test-2
+AD4 +AD4 ./fileio+AF8-0/vdev0/pr/res+AF8-holder:No SPC-3 Reservation holder
+AD4 +AD4 
+AD4 +AD4 This is a bug. SPC-2 reservations must be cleared when an initiator
+AD4 +AD4 logs out. This patch fixes that bug. A quote from SPC-2 illustrates this:
+AD4 +AD4 +ACI-Reservations managed using the reserve/release method do not persist
+AD4 +AD4 across some recovery actions (e.g., hard resets). When a target performs
+AD4 +AD4 one of these recovery actions, the application client(s) have to rediscover
+AD4 +AD4 the configuration and re-establish the required reservations.+ACI
+AD4 +AD4 
+AD4 +AD4 Cc: Mike Christie +ADw-mchristi+AEA-redhat.com+AD4
+AD4 +AD4 Cc: Christoph Hellwig +ADw-hch+AEA-lst.de+AD4
+AD4 +AD4 Cc: Hannes Reinecke +ADw-hare+AEA-suse.com+AD4
+AD4 +AD4 Cc: Nicholas Bellinger +ADw-nab+AEA-linux-iscsi.org+AD4
+AD4 +AD4 Signed-off-by: Bart Van Assche +ADw-bvanassche+AEA-acm.org+AD4
+AD4 +AD4 ---
+AD4 +AD4  drivers/target/target+AF8-core+AF8-transport.c +AHw 10 +-+-+-+-+-+-+-+-+-+-
+AD4 +AD4  1 file changed, 10 insertions(+-)
+AD4 +AD4 
+AD4 +AD4 diff --git a/drivers/target/target+AF8-core+AF8-transport.c b/drivers/target/target+AF8-core+AF8-transport.c
+AD4 +AD4 index e3f7e21e6614..93ef5c6362d6 100644
+AD4 +AD4 --- a/drivers/target/target+AF8-core+AF8-transport.c
+AD4 +AD4 +-+-+- b/drivers/target/target+AF8-core+AF8-transport.c
+AD4 +AD4 +AEAAQA -550,6 +-550,15 +AEAAQA void transport+AF8-deregister+AF8-session+AF8-configfs(struct se+AF8-session +ACo-se+AF8-sess)
+AD4 +AD4  +AH0
+AD4 +AD4  EXPORT+AF8-SYMBOL(transport+AF8-deregister+AF8-session+AF8-configfs)+ADs
+AD4 +AD4  
+AD4 +AD4 +-static int target+AF8-sess+AF8-release+AF8-reservation(struct se+AF8-device +ACo-dev, void +ACo-data)
+AD4 +AD4 +-+AHs
+AD4 +AD4 +-	struct se+AF8-session +ACo-sess +AD0 data+ADs
+AD4 +AD4 +-
+AD4 +AD4 +-	if (dev-+AD4-reservation+AF8-holder +AD0APQ sess)
+AD4 +AD4 +-		target+AF8-release+AF8-reservation(dev)+ADs
+AD4 +AD4 +-	return 0+ADs
+AD4 +AD4 +-+AH0
+AD4 +AD4 +-
+AD4 +AD4  void transport+AF8-free+AF8-session(struct se+AF8-session +ACo-se+AF8-sess)
+AD4 +AD4  +AHs
+AD4 +AD4  	struct se+AF8-node+AF8-acl +ACo-se+AF8-nacl +AD0 se+AF8-sess-+AD4-se+AF8-node+AF8-acl+ADs
+AD4 +AD4 +AEAAQA -592,6 +-601,7 +AEAAQA void transport+AF8-free+AF8-session(struct se+AF8-session +ACo-se+AF8-sess)
+AD4 +AD4  		sbitmap+AF8-queue+AF8-free(+ACY-se+AF8-sess-+AD4-sess+AF8-tag+AF8-pool)+ADs
+AD4 +AD4  		kvfree(se+AF8-sess-+AD4-sess+AF8-cmd+AF8-map)+ADs
+AD4 +AD4  	+AH0
+AD4 +AD4 +-	target+AF8-for+AF8-each+AF8-device(target+AF8-sess+AF8-release+AF8-reservation, se+AF8-sess)+ADs
+AD4 +AD4  	percpu+AF8-ref+AF8-exit(+ACY-se+AF8-sess-+AD4-cmd+AF8-count)+ADs
+AD4 +AD4  	kmem+AF8-cache+AF8-free(se+AF8-sess+AF8-cache, se+AF8-sess)+ADs
+AD4 +AD4  +AH0
+AD4 +AD4 
+AD4 
+AD4 Was the original code done for iscsi? We have this in
+AD4 https://tools.ietf.org/html/rfc7143:
+AD4 
+AD4 4.4.3.2. Reservations
+AD4 
+AD4 ....
+AD4 
+AD4 In contrast, +AFs-SPC2+AF0 does not specify detailed persistence
+AD4 requirements for reserve/release reservation state after an I+AF8-T nexus
+AD4 failure. Nonetheless, when reserve/release reservations are
+AD4 supported by an iSCSI target, the preferred implementation approach
+AD4 is to preserve reserve/release reservation state for iSCSI session
+AD4 reinstatement (see Section 6.3.5) or session continuation (see
+AD4 Section 6.3.6).
+AD4 
+AD4 
+AD4 So for example for session reinstatement if you pull a network cable
+AD4 then plug it back in the iscsi target can do a
+AD4 iscsit+AF8-close+AF8-session-+AD4-transport+AF8-deregister+AF8-session -+AD4
+AD4 transport+AF8-free+AF8-session. Above we will now release the reservation.

Hello Mike,

Was that paragraph from the iSCSI RFC perhaps written before it was made clear
in SPC-2 that reserve/release reservations do not persist? I think the name of
the successor concept, +ACI-persistent reservations+ACI, makes it clear that the
majority of the implementations of the old reserve/release mechanism did not
persist these reservations across sessions.

Bart.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
  2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
  2019-04-08 20:04 ` Mike Christie
  2019-04-08 20:17 ` Bart Van Assche
@ 2019-04-08 22:41 ` Mike Christie
  2019-04-08 23:17 ` Bart Van Assche
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Mike Christie @ 2019-04-08 22:41 UTC (permalink / raw)
  To: target-devel

On 04/08/2019 03:17 PM, Bart Van Assche wrote:
> On Mon, 2019-04-08 at 15:04 -0500, Mike Christie wrote:
>> On 04/02/2019 02:58 PM, Bart Van Assche wrote:
>>> While testing with the libiscsi tool I noticed after the tool had stopped
>>> and hence after it had logged out that an SPC-2 reservation was still active:
>>>
>>> $ (cd /sys/kernel/config/target/core && find -name res_holder|xargs grep -aH .)
>>> ./pscsi_0/vdev3/pr/res_holder:Passthrough
>>> ./iblock_0/vdev2/pr/res_holder:No SPC-3 Reservation holder
>>> ./fileio_1/vdev1/pr/res_holder:SPC-2 Reservation: iSCSI Initiator: iqn.2007-10.com.github:sahlberg:libiscsi:iscsi-test-2
>>> ./fileio_0/vdev0/pr/res_holder:No SPC-3 Reservation holder
>>>
>>> This is a bug. SPC-2 reservations must be cleared when an initiator
>>> logs out. This patch fixes that bug. A quote from SPC-2 illustrates this:
>>> "Reservations managed using the reserve/release method do not persist
>>> across some recovery actions (e.g., hard resets). When a target performs
>>> one of these recovery actions, the application client(s) have to rediscover
>>> the configuration and re-establish the required reservations."
>>>
>>> Cc: Mike Christie <mchristi@redhat.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Hannes Reinecke <hare@suse.com>
>>> Cc: Nicholas Bellinger <nab@linux-iscsi.org>
>>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>>> ---
>>>  drivers/target/target_core_transport.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
>>> index e3f7e21e6614..93ef5c6362d6 100644
>>> --- a/drivers/target/target_core_transport.c
>>> +++ b/drivers/target/target_core_transport.c
>>> @@ -550,6 +550,15 @@ void transport_deregister_session_configfs(struct se_session *se_sess)
>>>  }
>>>  EXPORT_SYMBOL(transport_deregister_session_configfs);
>>>  
>>> +static int target_sess_release_reservation(struct se_device *dev, void *data)
>>> +{
>>> +	struct se_session *sess = data;
>>> +
>>> +	if (dev->reservation_holder = sess)
>>> +		target_release_reservation(dev);
>>> +	return 0;
>>> +}
>>> +
>>>  void transport_free_session(struct se_session *se_sess)
>>>  {
>>>  	struct se_node_acl *se_nacl = se_sess->se_node_acl;
>>> @@ -592,6 +601,7 @@ void transport_free_session(struct se_session *se_sess)
>>>  		sbitmap_queue_free(&se_sess->sess_tag_pool);
>>>  		kvfree(se_sess->sess_cmd_map);
>>>  	}
>>> +	target_for_each_device(target_sess_release_reservation, se_sess);
>>>  	percpu_ref_exit(&se_sess->cmd_count);
>>>  	kmem_cache_free(se_sess_cache, se_sess);
>>>  }
>>>
>>
>> Was the original code done for iscsi? We have this in
>> https://tools.ietf.org/html/rfc7143:
>>
>> 4.4.3.2. Reservations
>>
>> ....
>>
>> In contrast, [SPC2] does not specify detailed persistence
>> requirements for reserve/release reservation state after an I_T nexus
>> failure. Nonetheless, when reserve/release reservations are
>> supported by an iSCSI target, the preferred implementation approach
>> is to preserve reserve/release reservation state for iSCSI session
>> reinstatement (see Section 6.3.5) or session continuation (see
>> Section 6.3.6).
>>
>>
>> So for example for session reinstatement if you pull a network cable
>> then plug it back in the iscsi target can do a
>> iscsit_close_session->transport_deregister_session ->
>> transport_free_session. Above we will now release the reservation.
> 
> Hello Mike,
> 
> Was that paragraph from the iSCSI RFC perhaps written before it was made clear
> in SPC-2 that reserve/release reservations do not persist? I think the name of

Do you know when that was added to SPC 2?

The iscsi text above is from RFC 7143 which I think was ratified in
2014. I think it was added in that version to clarify the issue. I am
not sure though.

The original 3720 ratified in 2004 did not have that chunk and only had
limited references/info on reservations.


> the successor concept, "persistent reservations", makes it clear that the
> majority of the implementations of the old reserve/release mechanism did not
> persist these reservations across sessions.
> 
> Bart.
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
  2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
                   ` (2 preceding siblings ...)
  2019-04-08 22:41 ` Mike Christie
@ 2019-04-08 23:17 ` Bart Van Assche
  2019-04-09  5:47 ` Mike Christie
  2019-04-09 17:25 ` Bart Van Assche
  5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2019-04-08 23:17 UTC (permalink / raw)
  To: target-devel

On Mon, 2019-04-08 at 17:41 -0500, Mike Christie wrote:
+AD4 On 04/08/2019 03:17 PM, Bart Van Assche wrote:
+AD4 +AD4 On Mon, 2019-04-08 at 15:04 -0500, Mike Christie wrote:
+AD4 +AD4 +AD4 Was the original code done for iscsi? We have this in
+AD4 +AD4 +AD4 https://tools.ietf.org/html/rfc7143:
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 4.4.3.2. Reservations
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 ....
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 In contrast, +AFs-SPC2+AF0 does not specify detailed persistence
+AD4 +AD4 +AD4 requirements for reserve/release reservation state after an I+AF8-T nexus
+AD4 +AD4 +AD4 failure. Nonetheless, when reserve/release reservations are
+AD4 +AD4 +AD4 supported by an iSCSI target, the preferred implementation approach
+AD4 +AD4 +AD4 is to preserve reserve/release reservation state for iSCSI session
+AD4 +AD4 +AD4 reinstatement (see Section 6.3.5) or session continuation (see
+AD4 +AD4 +AD4 Section 6.3.6).
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 So for example for session reinstatement if you pull a network cable
+AD4 +AD4 +AD4 then plug it back in the iscsi target can do a
+AD4 +AD4 +AD4 iscsit+AF8-close+AF8-session-+AD4-transport+AF8-deregister+AF8-session -+AD4
+AD4 +AD4 +AD4 transport+AF8-free+AF8-session. Above we will now release the reservation.
+AD4 +AD4 
+AD4 +AD4 Hello Mike,
+AD4 +AD4 
+AD4 +AD4 Was that paragraph from the iSCSI RFC perhaps written before it was made clear
+AD4 +AD4 in SPC-2 that reserve/release reservations do not persist? I think the name of
+AD4 
+AD4 Do you know when that was added to SPC 2?
+AD4 
+AD4 The iscsi text above is from RFC 7143 which I think was ratified in
+AD4 2014. I think it was added in that version to clarify the issue. I am
+AD4 not sure though.
+AD4 
+AD4 The original 3720 ratified in 2004 did not have that chunk and only had
+AD4 limited references/info on reservations.

Hello Mike,

From spc2r01 (1997-11-13): +ACI-Reservations managed using the Reserve/Release
method do not persist across some recovery actions (e.g., hard resets), so
most systems require significant reinitialization after a failure that
results in a hard reset. Reserve/Release managed reservations are retained
by the device server until released or until reset by mechanisms specified
in this standard.+ACI

Although the words differ from later versions of SPC2, the intention has not
changed.

Other vendors seem to agree with what has been defined in SPC2. From
https://kb.netapp.com/app/answers/answer+AF8-view/a+AF8-id/1001463/+AH4-/what-are-scsi-reservations-and-scsi-persistent-reservations+ACU-3F-:
+ACI-Thus, a SCSI bus reset performed due to an error recovery would cause the
reservation to be released.+ACI

Does this mean that the authors of the iSCSI RFC chose behavior that
contradicts established behavior for the SCSI reserve and release commands?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
  2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
                   ` (3 preceding siblings ...)
  2019-04-08 23:17 ` Bart Van Assche
@ 2019-04-09  5:47 ` Mike Christie
  2019-04-09 17:25 ` Bart Van Assche
  5 siblings, 0 replies; 7+ messages in thread
From: Mike Christie @ 2019-04-09  5:47 UTC (permalink / raw)
  To: target-devel

On 04/08/2019 06:17 PM, Bart Van Assche wrote:
> On Mon, 2019-04-08 at 17:41 -0500, Mike Christie wrote:
>> On 04/08/2019 03:17 PM, Bart Van Assche wrote:
>>> On Mon, 2019-04-08 at 15:04 -0500, Mike Christie wrote:
>>>> Was the original code done for iscsi? We have this in
>>>> https://tools.ietf.org/html/rfc7143:
>>>>
>>>> 4.4.3.2. Reservations
>>>>
>>>> ....
>>>>
>>>> In contrast, [SPC2] does not specify detailed persistence
>>>> requirements for reserve/release reservation state after an I_T nexus
>>>> failure. Nonetheless, when reserve/release reservations are
>>>> supported by an iSCSI target, the preferred implementation approach
>>>> is to preserve reserve/release reservation state for iSCSI session
>>>> reinstatement (see Section 6.3.5) or session continuation (see
>>>> Section 6.3.6).
>>>>
>>>>
>>>> So for example for session reinstatement if you pull a network cable
>>>> then plug it back in the iscsi target can do a
>>>> iscsit_close_session->transport_deregister_session ->
>>>> transport_free_session. Above we will now release the reservation.
>>>
>>> Hello Mike,
>>>
>>> Was that paragraph from the iSCSI RFC perhaps written before it was made clear
>>> in SPC-2 that reserve/release reservations do not persist? I think the name of
>>
>> Do you know when that was added to SPC 2?
>>
>> The iscsi text above is from RFC 7143 which I think was ratified in
>> 2014. I think it was added in that version to clarify the issue. I am
>> not sure though.
>>
>> The original 3720 ratified in 2004 did not have that chunk and only had
>> limited references/info on reservations.
> 
> Hello Mike,
> 
> From spc2r01 (1997-11-13): "Reservations managed using the Reserve/Release
> method do not persist across some recovery actions (e.g., hard resets), so
> most systems require significant reinitialization after a failure that
> results in a hard reset. Reserve/Release managed reservations are retained
> by the device server until released or until reset by mechanisms specified
> in this standard."
> 
> Although the words differ from later versions of SPC2, the intention has not
> changed.
> 
> Other vendors seem to agree with what has been defined in SPC2. From
> https://kb.netapp.com/app/answers/answer_view/a_id/1001463/~/what-are-scsi-reservations-and-scsi-persistent-reservations%3F-:
> "Thus, a SCSI bus reset performed due to an error recovery would cause the
> reservation to be released."
> 
> Does this mean that the authors of the iSCSI RFC chose behavior that
> contradicts established behavior for the SCSI reserve and release commands?
> 

I don't know.

For the hard reset/reset event case, if we go by SAM 2 which iscsi RFC
7143 and 3720 say they are based on at the beginning of their
definitions then it sounds like it depends on the transport spec for
what is a hard reset. We have this from SAM 2:

5.9.6 Hard reset:

A hard reset is a SCSI target port action in response to a reset event
within the service delivery subsystem. A wakeup event (see 3.1.132) is a
reset event. The definition of additional reset events is SCSI transport
protocol specific. Each SCSI transport protocol standard that defines
reset events shall specify the SCSI target port’s action
in response to reset events.

--------

That might sound like a wake event is a hard reset defined by SAM 2, and
the transport defines others. In the iscsi RFC the only mention of reset
events is when handling the iSCSI TARGET COLD RESET TMF. However, the
RFC does define the session reinstatement case as a I_T nexus loss
event, so if that is considered a hard reset by the SAM 2 definition
then I think your patch is correct. I have no idea if other software
writers read the specs/rfcs like maybe Nick did though.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout
  2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
                   ` (4 preceding siblings ...)
  2019-04-09  5:47 ` Mike Christie
@ 2019-04-09 17:25 ` Bart Van Assche
  5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2019-04-09 17:25 UTC (permalink / raw)
  To: target-devel

On Tue, 2019-04-09 at 00:47 -0500, Mike Christie wrote:
+AD4 I don't know.
+AD4 
+AD4 For the hard reset/reset event case, if we go by SAM 2 which iscsi RFC
+AD4 7143 and 3720 say they are based on at the beginning of their
+AD4 definitions then it sounds like it depends on the transport spec for
+AD4 what is a hard reset. We have this from SAM 2:
+AD4 
+AD4 5.9.6 Hard reset:
+AD4 
+AD4 A hard reset is a SCSI target port action in response to a reset event
+AD4 within the service delivery subsystem. A wakeup event (see 3.1.132) is a
+AD4 reset event. The definition of additional reset events is SCSI transport
+AD4 protocol specific. Each SCSI transport protocol standard that defines
+AD4 reset events shall specify the SCSI target port+IBk-s action
+AD4 in response to reset events.
+AD4 
+AD4 --------
+AD4 
+AD4 That might sound like a wake event is a hard reset defined by SAM 2, and
+AD4 the transport defines others. In the iscsi RFC the only mention of reset
+AD4 events is when handling the iSCSI TARGET COLD RESET TMF. However, the
+AD4 RFC does define the session reinstatement case as a I+AF8-T nexus loss
+AD4 event, so if that is considered a hard reset by the SAM 2 definition
+AD4 then I think your patch is correct. I have no idea if other software
+AD4 writers read the specs/rfcs like maybe Nick did though.

Hi Mike,

I'm fine with leaving out patch 3/11. But even if this patch is left out
that still leaves the question open what the behavior of RESERVE/RELEASE
should be ...

Bart.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-04-09 17:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-02 19:58 [PATCH 03/11] target/core: Release SPC-2 reservation upon initiator logout Bart Van Assche
2019-04-08 20:04 ` Mike Christie
2019-04-08 20:17 ` Bart Van Assche
2019-04-08 22:41 ` Mike Christie
2019-04-08 23:17 ` Bart Van Assche
2019-04-09  5:47 ` Mike Christie
2019-04-09 17:25 ` Bart Van Assche

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.