Date: Wed, 24 Sep 2008 15:50:14 +0400 (MSD) From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: FreeBSD-gnats-submit@freebsd.org Subject: bin/127605: [patch] properly initialise ccb_h.path_id in cam_open_btl (lib/libcam) Message-ID: <20080924115014.4D2731AF41C@void.codelabs.ru> Resent-Message-ID: <200809241200.m8OC056Q009867@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080924115014.4D2731AF41C>