Bug 202625 - [cam][libcam][patch] PERSISTENT RESERVE OUT needs scsi_cmd->length to be populated
Summary: [cam][libcam][patch] PERSISTENT RESERVE OUT needs scsi_cmd->length to be popu...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.2-RELEASE
Hardware: amd64 Any
: --- Affects Many People
Assignee: Sean Bruno
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-08-24 17:37 UTC by niakrisn
Modified: 2016-09-14 16:44 UTC (History)
5 users (show)

See Also:
sbruno: mfc-stable10+
sbruno: mfc-stable9-


Attachments
patch scsi_all.c and scsi_all.h (737 bytes, patch)
2015-08-24 17:37 UTC, niakrisn
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description niakrisn 2015-08-24 17:37:45 UTC
Created attachment 160309 [details]
patch scsi_all.c and scsi_all.h
Comment 1 Andrew 2016-04-12 15:03:34 UTC
Still this bug persists in the latest 10.3-RELEASE. I have no clear idea of the bug fixing policy, but I'm interested in SCSI persistent reservation support... someone can be so kind to tell me what can I do to have it committed?
Comment 2 Andrew 2016-06-10 09:33:23 UTC
Adding the freebsd-scsi list to the discussion, hoping that a committer could notice it and commit this patch. Thanks!

--Andrew
Comment 3 Sean Bruno freebsd_committer freebsd_triage 2016-06-17 21:42:41 UTC
Using this as a reference, which isn't the best, but it generally matches the data structure in question.

http://www.seagate.com/staticfiles/support/disc/manuals/scsi/100293068a.pdf 
pg 123

I can see that we *need* to populate length, but I'm unclear as why the change in array size for reserved and length is required.
Comment 4 Sean Bruno freebsd_committer freebsd_triage 2016-06-17 22:48:22 UTC
Spoke with some other developers (Scott) and its clear that the changes to scsi_all.h is incorrect.

The change to scsi_all.c should look like this instead of what is being proposed:

scsi_ulto2b(dxfer_len, dxfer_len);
Comment 5 Scott Long freebsd_committer freebsd_triage 2016-06-17 22:53:07 UTC
The diff for scsi_all.h is wrong and should be discarded.  According to SPC-2, SPC-3, SPC-4, and SPC-5, the existing code is correct.

The diff for scsi_all.c is correct.  I incorrectly stated the opposite in a different discussion
Comment 6 Sean Bruno freebsd_committer freebsd_triage 2016-06-17 23:00:03 UTC
(In reply to Scott Long from comment #5)
Ok, fair enough.
Comment 7 commit-hook freebsd_committer freebsd_triage 2016-06-28 18:09:23 UTC
A commit references this bug:

Author: sbruno
Date: Tue Jun 28 18:08:47 UTC 2016
New revision: 302253
URL: https://svnweb.freebsd.org/changeset/base/302253

Log:
  Correct PERSISTENT RESERVE OUT command and populate scsi_cmd->length.

  PR:		202625
  Submitted by:	niakrisn@gmail.com
  Reviewed by:	scottl
  Approved by:	re (hrs)
  MFC after:	2 weeks

Changes:
  head/sys/cam/scsi/scsi_all.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2016-06-28 18:32:26 UTC
A commit references this bug:

Author: sbruno
Date: Tue Jun 28 18:32:16 UTC 2016
New revision: 302254
URL: https://svnweb.freebsd.org/changeset/base/302254

Log:
  Revert svn r302253 at the request/review of Ken M.  This commit is
  incorrect.

  PR:		202625
  Approved by:	re (implicit)

Changes:
  head/sys/cam/scsi/scsi_all.c
Comment 9 Sean Bruno freebsd_committer freebsd_triage 2016-06-28 18:38:18 UTC
After speaking with Ken in email, I propose this based on his direction:

 % svn diff
Index: sys/cam/scsi/scsi_all.c
===================================================================
--- sys/cam/scsi/scsi_all.c	(revision 302254)
+++ sys/cam/scsi/scsi_all.c	(working copy)
@@ -8788,6 +8788,7 @@
 	scsi_cmd->opcode = PERSISTENT_RES_OUT;
 	scsi_cmd->action = service_action;
 	scsi_cmd->scope_type = scope | res_type;
+	scsi_ulto4b(dxfer_len, scsi_cmd->length);
 
 	cam_fill_csio(csio,
 		      retries,
Comment 10 commit-hook freebsd_committer freebsd_triage 2016-06-29 16:42:00 UTC
A commit references this bug:

Author: sbruno
Date: Wed Jun 29 16:41:37 UTC 2016
New revision: 302281
URL: https://svnweb.freebsd.org/changeset/base/302281

Log:
  Correct PERSISTENT RESERVE OUT command and populate scsi_cmd->length.

  PR:	202625
  Submitted by:	niakrisn@gmail.com
  Reviewed by:	scottl kenm
  Approved by:	re (gjb)
  MFC after:	2 weeks

Changes:
  head/sys/cam/scsi/scsi_all.c
Comment 11 commit-hook freebsd_committer freebsd_triage 2016-07-22 03:35:11 UTC
A commit references this bug:

Author: sbruno
Date: Fri Jul 22 03:34:16 UTC 2016
New revision: 303179
URL: https://svnweb.freebsd.org/changeset/base/303179

Log:
  MFC r302281
  Correct PERSISTENT RESERVE OUT command and populate scsi_cmd->length.

  PR:		202625

Changes:
  stable/10/sys/cam/scsi/scsi_all.c
Comment 12 Sean Bruno freebsd_committer freebsd_triage 2016-09-14 16:44:16 UTC
Not going to MFC to stable/9.  Closing as fixed.