bin/127605: [patch] properly initialise ccb_h.path_id in
cam_open_btl (lib/libcam)
Marius Strobl
marius at alchemy.franken.de
Wed Sep 24 20:00:12 UTC 2008
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
More information about the freebsd-bugs
mailing list