Date: Sun, 15 Nov 1998 10:30:36 -0700 (MST) From: "Justin T. Gibbs" <gibbs@narnia.plutotech.com> To: dan@math.berkeley.edu (Dan Strick) Cc: scsi@FreeBSD.ORG Subject: Re: bug in cam_real_open_device() Message-ID: <199811151730.KAA13368@narnia.plutotech.com> In-Reply-To: <199811151458.GAA26354@math.berkeley.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
In article <199811151458.GAA26354@math.berkeley.edu> you wrote:
>
> In file: camlib.c,v 1.2 1998/10/12 21:54:00 ken Exp
> beginning at line 587:
>
> if ((fd = open(path, flags)) < 0) {
> sprintf(cam_errbuf, "%s: couldn't open passthrough device %s\n"
> "%s: %s", func_name, newpath, func_name,
> strerror(errno));
> goto crod_bailout;
> }
>
> The bug is that "newpath" hasn't been filled in at this point.
> Another bug is that since the contents of newpath is unknown,
> cam_errbuf might not be large enough (resulting in buffer overflow).
File a PR. I'm sure Ken will fix this later today, but it's always
a good idea to put bugs in the database.
> I feel compelled to express severe irritation with this part of the
> cam design.
The userland library is "sugar coating" on the ioctls provided by
CAM. It was written to serve the developers immediate needs to
port certain software and the interfaces it exports are by no means
cast in stone.
> The above code is trying to open one of the passthrough
> devices. It determines the passthough device name by issuing a
> CAMGETPASSTHRU ioctl() to the xpt device that maps driver name and
> unit number into a passthrough driver name and unit number. These
> driver names are not names in /dev but names defined in kernel source
> driver tables that happen (by design of course) to correspond to names
> of special files in /dev. For example, "/dev/rda0s1c" refers to
> driver "da" unit "0" and "/dev/pass1" refers to driver "pass"
> unit "1".
>
> The cam library has to make the invalid assumption that special
> file names correspond to kernel driver names and has couple of
> special rules for two well known cases that break this assumption
> ("sd" and "st"). The cam library is effectively assuming that names
> in /dev are compiled into the kernel.
>
> In general, I think it is a very serious error for the kernel to contain
> hardwired file names. The only name that the kernel really has to know
> is "/sbin/init" and that name should be passed to it as an argument
> during bootstrap.
This has been discussed on -scsi before, so you may want to look in the
archives to see all of the details of this problem.
Sure, you can kill all hard coded tables of device names in the kernel,
but you will eventually need to do some device resolution based on that
device's canonical name. Take booting as an example. We can either
look for the root device based on it's canonical name (the name the driver
for that device knows itself as), or we can have hard coded tables that
we use to perform the translation from name to major/minor number, and
use that mechanism. Since the intended goal is to get rid of major and
minor numbers once DEVFS is useful (next year probably), and major and
minor numbers are harder for the end user to deal with than canonical
names (Hmm, do I need to boot off 0x3040 or was that 0x2030?) we should
be removing as much dependence on them as possible. With DEVFS, the
device node you open can be called whatever you want since the filesystem
has recorded the information required to get to the device you want to
deal with, so major/minor number or name translation isn't a problem.
But this somewhat skirts around your real issue.
> There are a lot of little glitchy consequences of this aspect of the
> cam design. For example, cam_open_device() will fail if the required
> passthrough device has not been created in /dev, but there is currently
> no way for a program using camlib to determine the name of the required
> passthough device until after camlib has successfully opened it.
> (The camlib routines happen not to fill in this cam_device struct
> member until after the open succeeds. There is no camlib routine
> that does just the CAMGETPASSTHRU ioctl(). Since the CAMGETPASSTHRU
> ioctl() is undocumented, using it on the side is not an option.)
It is supposed to be documented. File a PR.
> This whole issue would never have come up and most of camlib would
> be obviously vacuous if SCSI ioctl()s could be issued via the
> usual device special files instead of the /dev/pass files.
> What exactly do the /dev/pass files do that could not have been
> done with the real device special files? (I.E. Why can't we just
> do SCSI passthough ioctl()s using the raw disk devices like we
> used to?)
You're missing a fundamental design concept in CAM. The pass-thru
device and the 'real' device are completely separate entities. They
know nothing about each other and don't need to in order to function
correctly. Why is this the case? Think of the requirements for the
cd driver. It can't do anything useful if it doesn't have media. So
it's open routine will fail if no media is in the drive. This simplifies
the tracking of internal state in the driver since the only place were
this check needs to occur is in the open routine. Now fold in the
'pass-thru' capability to this driver and you add unnecessary complexity.
The conditions for which the device must allow an open change. There
are also issues concerning how per-transaction resource scheduling is
performed that need to be addressed. Blech.
CAM already provideds for multiple 'peripheral drivers' to have
instances accessing the same underlying device (For instance a 'probe'
driver, CD driver and a WORM driver), so it becomes very natural
to push pass-thru functionality into its own driver. To present
your kind of world view, each peripheral driver would need to understand
that pass-thru activity is possible and likely perform a device table
lookup on each transaction to forward it to the appropriate pass-thru
instance. Blech.
If you want a reliable way to get to the pass-thru driver, have your
users specify the Bus, Target, Lun of the device they want to talk to,
and use the appropriate method in the CAM library to open the device
using that information.
--
Justin
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-scsi" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199811151730.KAA13368>