bin/127605: [patch] properly initialise ccb_h.path_id in cam_open_btl (lib/libcam)

[Eygene Ryabinkin]

>Number:         127605
>Category:       bin
>Synopsis:       [patch] properly initialise ccb_h.path_id in cam_open_btl (lib/libcam)
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Sep 24 12:00:05 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Eygene Ryabinkin
>Release:        FreeBSD 7.1-PRERELEASE i386
>Organization:
Code Labs
>Environment:

System: FreeBSD XXX 7.1-PRERELEASE FreeBSD 7.1-PRERELEASE #19: Tue Sep 23 13:21:48 MSD 2008 root at XXX:/usr/src/sys/i386/compile/XXX i386

>Description:

When I use cdrecord on the fresh 7.1-PRERELEASE, it fails to open the
device (atapicam one in my case) saying
-----
cdrecord: Invalid argument. Cannot open SCSI driver.
-----

I had traced this to the calls for XPT_DEV_MATCH on /dev/xpt0 with
ioctl(CAMIOCOMMAND) inside cam_open_btl() and it turned out that the
ccb.ccb_h.path_id is not filled in.  As I see, xptioctl in
sys/cam/cam_xpt.c invokes xpt_find_bus passing path_id as an argument
and returns EINVAL in case of error.

>How-To-Repeat:

For me it was sufficient to check out yesterday's (September 24th, 2008)
7.1-PRERELEASE, and spawn cdrecord on my recorder (IDE-connected PIONEER
DVD-RW DVR-108 1.14, with atapicam emulation layer).

cdrecord also fails to perform bus scanning with '-scanbus': is has
simular code to enumerate devices via XPT_DEV_MATCH.  But with old
libcam and fixed enumeration code it successfully returns the list of
devices, but fails to open any due to the problem in cam_open_btl().

Another way to reproduce the problem is to spawn 'camcontrol eject
b:t:l' with unpatched libcam:
-----
$ camcontrol eject 1:1:0
camcontrol: cam_open_btl: CAMIOCOMMAND ioctl failed
cam_open_btl: Invalid argument
-----

>Fix:

The following patch cures the problem in the libcam:
--- libcam-add-ids-for-XPT_DEV_MATCH.patch begins here ---
CAMIOCOMMAND with argument XPT_DEV_MATCH fails with EINVAL if field
ccb_h.path_id is not set to CAM_XPT_PATH_ID.  I am additionally setting
target_id and target_lun to the wildcard values.  Had not found the
exact specifications for what is needed for XPT_DEV_MATCH, but the code
in the camcontrol.c sets all three fields.

For the atapicam(4) CD-ROM device setting only ccb_h.path_id is
sufficient to get cam_open_btl() working for cdrecord tool from
sysutils/cdrtools [1].  But setting the other fields makes no harm here,
so I really don't know if it is needed or not.

[1] Needs patching too: it has the simular code for the bus scanning and
no ccb_h.path_id was set there as well.  Opened another PR.
-- 
Eygene, rea-fbsd at codelabs.ru
--- lib/libcam/camlib.c.orig	2008-09-24 14:56:25.000000000 +0400
+++ lib/libcam/camlib.c	2008-09-24 14:58:12.000000000 +0400
@@ -346,6 +346,9 @@
 
 	bzero(&ccb, sizeof(union ccb));
 	ccb.ccb_h.func_code = XPT_DEV_MATCH;
+	ccb.ccb_h.path_id = CAM_XPT_PATH_ID;
+	ccb.ccb_h.target_id = CAM_TARGET_WILDCARD;
+	ccb.ccb_h.target_lun = CAM_LUN_WILDCARD;
 
 	/* Setup the result buffer */
 	bufsize = sizeof(struct dev_match_result);
--- libcam-add-ids-for-XPT_DEV_MATCH.patch ends here ---

As I said in the patch description, I am not completely sure that one
should initialize all three fields, but it makes no harm for my test
cases.

Attaching the patch for the cdrecord here too, but it is just for the
reference.  I will open another PR and will try to contact Joerg
Schilling: seems like this needs to be patched in upstream as well.

--- patch-libscg::scsi-bsd.c begins here ---
--- libscg/scsi-bsd.c.orig	2008-09-24 14:03:04.000000000 +0400
+++ libscg/scsi-bsd.c	2008-09-24 14:04:03.000000000 +0400
@@ -674,6 +674,9 @@
 	 * system.
 	 */
 	ccb.ccb_h.func_code = XPT_DEV_MATCH;
+	ccb.ccb_h.path_id = CAM_XPT_PATH_ID;
+	ccb.ccb_h.target_id = CAM_TARGET_WILDCARD;
+	ccb.ccb_h.target_lun = CAM_LUN_WILDCARD;
 
 	/*
 	 * Setup the result buffer.
--- patch-libscg::scsi-bsd.c begins here ---

With these two patches I have no problem in burning CDs and performing
some basic commands via camcontrol on my CD unit.

I have no idea why this popped only in 7.1-PRERELEASE: I see that the
code that checks ccb_h.path_id is present since sys/cam/cam_xpt.c 1.176
that is 17 month old.  May be the problem is somewhere else, but I don't
see where it is.
>Release-Note:
>Audit-Trail:
>Unformatted: