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>
index | next in thread | raw e-mail
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? Mariushome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809242000.m8OK0C2p050262>