bin/109521: [patch] 'chio return' breaks on non-voltag changers

[Joost Bekkers]

>Number:         109521
>Category:       bin
>Synopsis:       [patch] 'chio return' breaks on non-voltag changers
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun Feb 25 09:50:05 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Joost Bekkers
>Release:        FreeBSD 6.2-RELEASE amd64
>Organization:
>Environment:
System: FreeBSD bps.jodocus.org 6.2-RELEASE FreeBSD 6.2-RELEASE #1: Sun Jan 21 12:43:37 CET 2007 root@:/usr/obj/usr/src/sys/bps amd64

>Description:

Trying the `chio return` command on a changer without volume tags results
in a cry from the kernel:

(ch0:ahc0:0:4:1): READ ELEMENT STATUS. CDB: b8 30 0 1 0 1 0 0 4 0 0 0 
(ch0:ahc0:0:4:1): CAM Status: SCSI Status Error
(ch0:ahc0:0:4:1): SCSI Status: Check Condition
(ch0:ahc0:0:4:1): ILLEGAL REQUEST asc:24,0
(ch0:ahc0:0:4:1): Invalid field in CDB: Command byte 1 bit 4 is invalid
(ch0:ahc0:0:4:1): Unretryable error

byte 1 bit 4 == volume tags

This call originates from get_element_status() which is only called
from inside do_return(). The volume tag information appears to never
be used, so why ask for it?

>How-To-Repeat:

On a changer without voltag support do:

chio move slot 0 drive 0
chio return drive 0

I haven't been able to test this with any other model than the one I
have.

# camcontrol devlist
(HP C1557A U709)                   at scbus0 target 4 lun 0 (sa0,pass0)
(HP C1557A U709)                   at scbus0 target 4 lun 1 (ch0,pass1)

>Fix:


--- src/bin/chio/chio.c_6.2R    Sun Feb 25 09:15:39 2007
+++ src/bin/chio/chio.c Sun Feb 25 09:14:06 2007
@@ -1020,7 +1020,12 @@
        cesr.cesr_element_type = (uint16_t)type;
        cesr.cesr_element_base = (uint16_t)element;
        cesr.cesr_element_count = 1;            /* Only this one element */
-       cesr.cesr_flags |= CESR_VOLTAGS;        /* Grab voltags as well */
+       /* 25/02/2007 Joost Bekkers:
+        * Don't ask for voltags. The information is currently never used
+        * and it breaks the return command for changers that don't support
+        * voltags.
+        */
+       //cesr.cesr_flags |= CESR_VOLTAGS;      /* Grab voltags as well */
        cesr.cesr_element_status = ces;
 
        if (ioctl(changer_fd, CHIOGSTATUS, (char *)&cesr) == -1) {
>Release-Note:
>Audit-Trail:
>Unformatted: