[Bug 202625] [cam][libcam][patch] PERSISTENT RESERVE OUT needs scsi_cmd->length to be populated

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

Andrew <andrew...@hotmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |freebs...@FreeBSD.org

--- Comment #2 from Andrew <andrew...@hotmail.com> ---
Adding the freebsd-scsi list to the discussion, hoping that a committer could
notice it and commit this patch. Thanks!

--Andrew

--
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
freebs...@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-scsi
To unsubscribe, send any mail to "freebsd-scsi...@freebsd.org"

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

Alan Somers <aso...@FreeBSD.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |aso...@FreeBSD.org
Assignee|freebs...@FreeBSD.org |k...@FreeBSD.org

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

Sean Bruno <sbr...@FreeBSD.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |sbr...@FreeBSD.org
Status|New |In Progress

--- Comment #3 from Sean Bruno <sbr...@FreeBSD.org> ---
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.

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #4 from Sean Bruno <sbr...@FreeBSD.org> ---
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);

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

Scott Long <sco...@FreeBSD.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |sco...@FreeBSD.org

--- Comment #5 from Scott Long <sco...@FreeBSD.org> ---
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

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #6 from Sean Bruno <sbr...@FreeBSD.org> ---
(In reply to Scott Long from comment #5)
Ok, fair enough.

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #7 from commi...@freebsd.org ---
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: niak...@gmail.com
Reviewed by: scottl
Approved by: re (hrs)
MFC after: 2 weeks

Changes:
head/sys/cam/scsi/scsi_all.c

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

Sean Bruno <sbr...@FreeBSD.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Flags| |mfc-stable9?, mfc-stable10?
Assignee|k...@FreeBSD.org |sbr...@FreeBSD.org

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #8 from commi...@freebsd.org ---

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

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #9 from Sean Bruno <sbr...@FreeBSD.org> ---
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,

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #10 from commi...@freebsd.org ---

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: niak...@gmail.com

Reviewed by: scottl kenm
Approved by: re (gjb)

MFC after: 2 weeks

Changes:
head/sys/cam/scsi/scsi_all.c

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

--- Comment #11 from commi...@freebsd.org ---

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

[bugzilla...@freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202625

Sean Bruno <sbr...@FreeBSD.org> changed:

What |Removed |Added
----------------------------------------------------------------------------

Flags|mfc-stable9?, mfc-stable10? |mfc-stable9-, mfc-stable10+
Resolution|--- |FIXED
Status|In Progress |Closed

--- Comment #12 from Sean Bruno <sbr...@FreeBSD.org> ---
Not going to MFC to stable/9. Closing as fixed.