Skip to contentSkip to conversation

sgbind

RFC Douglas Gilbert 3 participants 5 messages May 14, 2003 SCSI

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 3 lines · Douglas Gilbert

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.

Quoted 2 lines

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

Why is this signed?

Quoted 4 lines
         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.

Quoted 5 lines

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.

Quoted 9 lines

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.

Quoted 5 lines

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?

Quoted 5 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 10 lines · Christoph Hellwig
Quoted 3 lines · Douglas Gilbert

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.

Quoted 5 lines
Quoted 2 lines

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).

Quoted 8 lines
Quoted 4 lines
        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.

Quoted 26 lines
Quoted 5 lines

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.

Quoted 9 lines

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.

Quoted 5 lines

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.

Quoted 2 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 4 lines · Douglas Gilbert
Quoted 4 lines
Quoted 2 lines

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

Quoted 10 lines
Quoted 1 line

Why is this signed?

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

Quoted 4 lines
Quoted 4 lines
        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

#4 lore raw
In reply to Douglas Gilbert
Quoted 8 lines · Douglas Gilbert

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

Quoted 5 lines

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.

Quoted 5 lines

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