[RFC] sgbind

[RFC] sgbind

[Douglas Gilbert]

I have resynced the sgbind code against lk 2.5.69-bk8
but I will await a more stable baseline before releasing
it (so I know sgbind isn't causing more flakiness).


sgbind is a virtual device node (misc char device 10, 246
at the moment) that allows an existing or an unscanned
logical unit (device) to be bound by its nexus and SCSI
commands sent to it. By "nexus" I mean its topological
address <host_no, channel, id, lun>.

The sgbind code is in the sg driver which allows it
to share existing code. sgbind avoids using the sg
driver's upper level sysfs code (i.e. sg_driverfs_dev).

Here is the additional code in the sg header:

/* sgbind support, for device scanning via /dev/sgbind */
#define SGBIND_ATTACH 0x22a0
#define SGBIND_DETACH 0x22a1
#define SGBIND_ADD_LU 0x22a2
#define SGBIND_REMOVE_LU 0x22a3
 

struct sgbind_nexus {
         int descriptor_id;      /* set to 0 for now */
         unsigned int host_no;
         unsigned int channel;
         unsigned int scsi_id;
         unsigned int lun;
};

The SGBIND_ADD_LU and SGBIND_REMOVE_LU ioctls are simply
programmatic interfaces to the code behind "scsi add-single-device"
and "scsi remove-single-device". [Code was moved from scsi_proc.c
to scsi_scan.c so that these ioctls don't require procfs to be
configured.]

Multiple file descriptors may be open on /dev/sgbind, each
file descriptor has its own state (like normal sg device nodes).
The SGBIND_ATTACH ioctl will either bind:
   - to an existing logical unit (device) which matches the
     given nexus
   - create a new scsi_device instance (using scsi_alloc_dev())
     with the given nexus
The host number must match an existing host.

The SGBIND_DETACH ioctl unbinds the file descriptor from an
existing nexus (assuming a nexus was established by a prior
SGBIND_ATTACH). Thereafter an new nexus can be formed with
SGBIND_ATTACH. Closing the file descriptor also has the effect
of unbinding the nexus.

I'm not sure whether a SGBIND_PUBLISH ioctl is required
(SGBIND_ADD_LU sort of does that).

Two other sg ioctl's can be used on a sgbind file descriptor:
   - SG_GET_VERSION
   - SG_IO once there is an existing nexus

Obviously getting to the SG_IO ioctl is the point of the exercise.


To test this further I believe there should be a new
field in the Scsi_Host and Scsi_Host_Template structures
called something like 'no_scan'. Default is 0 for
existing mid level scanning. A value of 1 instructs
the scsi_scan.c to ignore this host.

Comments?

Doug Gilbert

Re: [RFC] sgbind

[Christoph Hellwig]

On Wed, May 14, 2003 at 10:33:32PM +1000, Douglas Gilbert wrote:
> The sgbind code is in the sg driver which allows it
> to share existing code. sgbind avoids using the sg
> driver's upper level sysfs code (i.e. sg_driverfs_dev).

I'd prefer to have it in the scsi midlayer as it's not really
a traditional scsi upper driver but rather a special way to access
scsi devices.

> struct sgbind_nexus {
>          int descriptor_id;      /* set to 0 for now */

Why is this signed?

>          unsigned int host_no;
>          unsigned int channel;
>          unsigned int scsi_id;
>          unsigned int lun;

Please use __u32/__s32 instead.  Any new interface should
use those explicit versions.

> The SGBIND_ADD_LU and SGBIND_REMOVE_LU ioctls are simply
> programmatic interfaces to the code behind "scsi add-single-device"
> and "scsi remove-single-device". [Code was moved from scsi_proc.c
> to scsi_scan.c so that these ioctls don't require procfs to be
> configured.]

I don't think adding more ioctls is a good thing.

> Multiple file descriptors may be open on /dev/sgbind, each
> file descriptor has its own state (like normal sg device nodes).
> The SGBIND_ATTACH ioctl will either bind:
>    - to an existing logical unit (device) which matches the
>      given nexus
>    - create a new scsi_device instance (using scsi_alloc_dev())
>      with the given nexus
> The host number must match an existing host.
> 
> The SGBIND_DETACH ioctl unbinds the file descriptor from an
> existing nexus (assuming a nexus was established by a prior
> SGBIND_ATTACH). Thereafter an new nexus can be formed with
> SGBIND_ATTACH. Closing the file descriptor also has the effect
> of unbinding the nexus.

I don't like this ioctl horror.  Can't you add a sgfs that just containß
nodes for all scsi devices?

> To test this further I believe there should be a new
> field in the Scsi_Host and Scsi_Host_Template structures
> called something like 'no_scan'. Default is 0 for
> existing mid level scanning. A value of 1 instructs
> the scsi_scan.c to ignore this host.

Good idea.

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] sgbind

[Douglas Gilbert]

Christoph Hellwig wrote:
> On Wed, May 14, 2003 at 10:33:32PM +1000, Douglas Gilbert wrote:
> 
>>The sgbind code is in the sg driver which allows it
>>to share existing code. sgbind avoids using the sg
>>driver's upper level sysfs code (i.e. sg_driverfs_dev).
> 
> 
> I'd prefer to have it in the scsi midlayer as it's not really
> a traditional scsi upper driver but rather a special way to access
> scsi devices.

The sg driver is nothing if not "a special way to
access scsi devices". Sg is a pass through to the
mid level. Note that I originally suggested this
could be an addition to the mid level and Patrick
Mansfield suggested I put it in the sg driver.
To date it is about 180 extra lines of code in
sg.

Also the SCSI_IOCTL_SEND_COMMAND and SG_IO implementations
in the block layer are sub-standard. I'd prefer not to
see that hack repeated.

>>struct sgbind_nexus {
>>         int descriptor_id;      /* set to 0 for now */
> 
> 
> Why is this signed?

It is simply a guard, allowing a different toplogical
representation (perhaps with a 64 bit, multi-level
lun).

>>         unsigned int host_no;
>>         unsigned int channel;
>>         unsigned int scsi_id;
>>         unsigned int lun;
> 
> 
> Please use __u32/__s32 instead.  Any new interface should
> use those explicit versions.

Why not int32_t and uint32_t?
Using identifiers that start with a double underscore is
a contravention of both C89 and C99.

>>The SGBIND_ADD_LU and SGBIND_REMOVE_LU ioctls are simply
>>programmatic interfaces to the code behind "scsi add-single-device"
>>and "scsi remove-single-device". [Code was moved from scsi_proc.c
>>to scsi_scan.c so that these ioctls don't require procfs to be
>>configured.]
> 
> 
> I don't think adding more ioctls is a good thing.
> 
> 
>>Multiple file descriptors may be open on /dev/sgbind, each
>>file descriptor has its own state (like normal sg device nodes).
>>The SGBIND_ATTACH ioctl will either bind:
>>   - to an existing logical unit (device) which matches the
>>     given nexus
>>   - create a new scsi_device instance (using scsi_alloc_dev())
>>     with the given nexus
>>The host number must match an existing host.
>>
>>The SGBIND_DETACH ioctl unbinds the file descriptor from an
>>existing nexus (assuming a nexus was established by a prior
>>SGBIND_ATTACH). Thereafter an new nexus can be formed with
>>SGBIND_ATTACH. Closing the file descriptor also has the effect
>>of unbinding the nexus.
> 
> 
> I don't like this ioctl horror.  Can't you add a sgfs that just containß
> nodes for all scsi devices?

That is exactly what I'm trying to avoid. The whole idea is
to not scan all SCSI devices in the kernel and
not generate some kernel mapping of them. Rather, give
the scanning job to a user application and it can pick its
way around and only make those devices that are relevant
visible.

Doug Gilbert



-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] sgbind

[Patrick Mansfield]

On Thu, May 15, 2003 at 12:02:09AM +1000, Douglas Gilbert wrote:

> >>struct sgbind_nexus {
> >>         int descriptor_id;      /* set to 0 for now */
> > 
> > 
> > Why is this signed?
> 
> It is simply a guard, allowing a different toplogical
> representation (perhaps with a 64 bit, multi-level
> lun).
> 
> >>         unsigned int host_no;
> >>         unsigned int channel;
> >>         unsigned int scsi_id;
> >>         unsigned int lun;

Why not use an 8 byte LUN now? You could still keep the descriptor.

-- Patrick Mansfield

Re: [RFC] sgbind

[Patrick Mansfield]

On Wed, May 14, 2003 at 10:33:32PM +1000, Douglas Gilbert wrote:

> Here is the additional code in the sg header:
> 
> /* sgbind support, for device scanning via /dev/sgbind */
> #define SGBIND_ATTACH 0x22a0
> #define SGBIND_DETACH 0x22a1
> #define SGBIND_ADD_LU 0x22a2
> #define SGBIND_REMOVE_LU 0x22a3

> The SGBIND_ADD_LU and SGBIND_REMOVE_LU ioctls are simply
> programmatic interfaces to the code behind "scsi add-single-device"
> and "scsi remove-single-device". [Code was moved from scsi_proc.c
> to scsi_scan.c so that these ioctls don't require procfs to be
> configured.]

Calling the current scsi_probe_and_add_lun defeats the user level scanning
- at least what I had in mind, that is, using sgbind to send the INQUIRY
and all other SCSI commands currently sent via calling
scsi_probe_and_add_lun.

I was thinking more along the lines of using scsi_add_lun or similiar
code, and/or allowing modification of appropriate scsi_device values from
user land - like borken, single_lun etc.

So in user land we could SGBIND_ATTACH (to a non-existent device), send an
INQUIRY, if the INQUIRY worked (like current scan code when we call
scsi_add_lun) then use an sgbind interface to add (SGBIND_ADD_LU or such)
this scsi_device.

The SGBIND_ADD_LU would at a minimum call scsi_device_register, so we
would then have sysfs interfaces available. If we passed in the inquiry
result, we might be able to just call scsi_add_lun, but exposing and
setting scsi_device sysfs attributes (like single_lun, borken, etc.; a
scsi_device.bflags might make this easier) would be more flexible.

Maybe this is too much to ask for in 2.5, though exposing interfaces that
are only used via sgbind would not be dangerous.

> To test this further I believe there should be a new
> field in the Scsi_Host and Scsi_Host_Template structures
> called something like 'no_scan'. Default is 0 for
> existing mid level scanning. A value of 1 instructs
> the scsi_scan.c to ignore this host.

You can always test by removing an existing device, or using scsi_debug.

If we want to allow both in kernel and user level scanning, we would need
a global scsi value, not just per host, a boot time or user settable value
would work nicely. I don't think we have anyplace for global scsi sysfs
attributes.

-- Patrick Mansfield