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

[Marius Strobl]

The following reply was made to PR kern/127605; it has been noted by GNATS.

From: Marius Strobl <marius at alchemy.franken.de>
To: bug-followup at FreeBSD.org, rea-fbsd at codelabs.ru, ken at FreeBSD.org
Cc:  
Subject: Re: bin/127605: [patch] properly initialise ccb_h.path_id in cam_open_btl (lib/libcam)
Date: Wed, 24 Sep 2008 21:55:49 +0200

 On Wed, Sep 24, 2008 at 03:50:14PM +0400, Eygene Ryabinkin wrote:
 > 
 > >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.
 > 
 
 I think this patch is fine as-is, at least Ken and I fixed
 basically the same bug the same way in camcontrol.c rev. 1.52.
 Ken, do you have any objections to committing it?
 
 Marius