Date: Wed, 24 Sep 2008 20:00:12 GMT From: Marius Strobl <marius@alchemy.franken.de> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/127605: [patch] properly initialise ccb_h.path_id in cam_open_btl (lib/libcam) Message-ID: <200809242000.m8OK0C2p050262@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/127605; it has been noted by GNATS. From: Marius Strobl <marius@alchemy.franken.de> To: bug-followup@FreeBSD.org, rea-fbsd@codelabs.ru, ken@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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809242000.m8OK0C2p050262>