2002-11-29 22:46:51

by Christoph Hellwig

[permalink] [raw]
Subject: [RFC] remove IDESCSI_SG_TRANSFORM (compile fix)

In the current 2.5 scsi-misc tree Scsi_Device_Template.tag is gone.

ide-scsi is still using it in should_transform() to decide whether
to test IDESCSI_SG_TRANSFORM or IDESCSI_TRANSFORM.

The obvious compile fix is to just kill that check, which makes
IDESCSI_SG_TRANSFORM superflous. Of course the question remains
what the actual point of IDESCSI_SG_TRANSFORM was and if we still
need it.


--- 1.17/drivers/scsi/ide-scsi.c Mon Nov 18 08:10:40 2002
+++ edited/drivers/scsi/ide-scsi.c Fri Nov 29 23:14:21 2002
@@ -82,7 +82,6 @@
* SCSI command transformation layer
*/
#define IDESCSI_TRANSFORM 0 /* Enable/Disable transformation */
-#define IDESCSI_SG_TRANSFORM 1 /* /dev/sg transformation */

/*
* Log flags
@@ -532,7 +531,6 @@
if (drive->id && (drive->id->config & 0x0060) == 0x20)
set_bit (IDESCSI_DRQ_INTERRUPT, &scsi->flags);
set_bit(IDESCSI_TRANSFORM, &scsi->transform);
- clear_bit(IDESCSI_SG_TRANSFORM, &scsi->transform);
#if IDESCSI_DEBUG_LOG
set_bit(IDESCSI_LOG_CMD, &scsi->log);
#endif /* IDESCSI_DEBUG_LOG */
@@ -666,22 +664,6 @@
return "SCSI host adapter emulation for IDE ATAPI devices";
}

-int idescsi_ioctl (Scsi_Device *dev, int cmd, void *arg)
-{
- ide_drive_t *drive = idescsi_drives[dev->id];
- idescsi_scsi_t *scsi = drive->driver_data;
-
- if (cmd == SG_SET_TRANSFORM) {
- if (arg)
- set_bit(IDESCSI_SG_TRANSFORM, &scsi->transform);
- else
- clear_bit(IDESCSI_SG_TRANSFORM, &scsi->transform);
- return 0;
- } else if (cmd == SG_GET_TRANSFORM)
- return put_user(test_bit(IDESCSI_SG_TRANSFORM, &scsi->transform), (int *) arg);
- return -EINVAL;
-}
-
static inline struct bio *idescsi_kmalloc_bio (int count)
{
struct bio *bh, *bhp, *first_bh;
@@ -760,13 +742,7 @@
static inline int should_transform(ide_drive_t *drive, Scsi_Cmnd *cmd)
{
idescsi_scsi_t *scsi = drive->driver_data;
- struct gendisk *disk = cmd->request->rq_disk;

- if (disk) {
- struct Scsi_Device_Template **p = disk->private_data;
- if (strcmp((*p)->tag, "sg") == 0)
- return test_bit(IDESCSI_SG_TRANSFORM, &scsi->transform);
- }
return test_bit(IDESCSI_TRANSFORM, &scsi->transform);
}

@@ -864,7 +840,6 @@
.detect = idescsi_detect,
.release = idescsi_release,
.info = idescsi_info,
- .ioctl = idescsi_ioctl,
.queuecommand = idescsi_queue,
.bios_param = idescsi_bios,
.can_queue = 10,


2002-11-30 00:35:47

by Mike Anderson

[permalink] [raw]
Subject: Re: [RFC] remove IDESCSI_SG_TRANSFORM (compile fix)

Christoph Hellwig [[email protected]] wrote:
> In the current 2.5 scsi-misc tree Scsi_Device_Template.tag is gone.
>
> ide-scsi is still using it in should_transform() to decide whether
> to test IDESCSI_SG_TRANSFORM or IDESCSI_TRANSFORM.
>
> The obvious compile fix is to just kill that check, which makes
> IDESCSI_SG_TRANSFORM superflous. Of course the question remains
> what the actual point of IDESCSI_SG_TRANSFORM was and if we still
> need it.

Thanks for catching this Christoph I thought the only use was inside
SCSI. I could make a patch to scsi-misc to add tag back in. Another
option if it is still needed is to switch to "->name == "generic").

Though I have not used this interface I thought if one was using an sg
device to a ide-scsi device and the flag was set that sg commands that
where not 100% the same as ATAP commands where translated.

-andmike
--
Michael Anderson
[email protected]

2002-12-02 17:14:20

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC] remove IDESCSI_SG_TRANSFORM (compile fix)

On Fri, Nov 29, 2002 at 04:44:35PM -0800, Mike Anderson wrote:
> Thanks for catching this Christoph I thought the only use was inside
> SCSI. I could make a patch to scsi-misc to add tag back in. Another
> option if it is still needed is to switch to "->name == "generic").
>
> Though I have not used this interface I thought if one was using an sg
> device to a ide-scsi device and the flag was set that sg commands that
> where not 100% the same as ATAP commands where translated.

Well, imho IDESCSI_SG_TRANSFORM is broken in 2.5. Now that ever block
driver implements the sg ioctls a sg request can come from sd or sr
aswell.

2002-12-02 21:40:59

by Alan

[permalink] [raw]
Subject: Re: [RFC] remove IDESCSI_SG_TRANSFORM (compile fix)

On Mon, 2002-12-02 at 17:21, Christoph Hellwig wrote:
> On Fri, Nov 29, 2002 at 04:44:35PM -0800, Mike Anderson wrote:
> > Thanks for catching this Christoph I thought the only use was inside
> > SCSI. I could make a patch to scsi-misc to add tag back in. Another
> > option if it is still needed is to switch to "->name == "generic").
> >
> > Though I have not used this interface I thought if one was using an sg
> > device to a ide-scsi device and the flag was set that sg commands that
> > where not 100% the same as ATAP commands where translated.
>
> Well, imho IDESCSI_SG_TRANSFORM is broken in 2.5. Now that ever block
> driver implements the sg ioctls a sg request can come from sd or sr
> aswell.

Quite possibly, but newer drivers that might used sd/sr via the new
API's should also know about the newer standards. Older sg users are not
always so bright.


2002-12-03 07:12:49

by Douglas Gilbert

[permalink] [raw]
Subject: Re: [RFC] remove IDESCSI_SG_TRANSFORM (compile fix)

On Mon, 2002-12-02 at 17:21, Christoph Hellwig wrote:
> On Fri, Nov 29, 2002 at 04:44:35PM -0800, Mike Anderson wrote:
> > Thanks for catching this Christoph I thought the only use was inside
> > SCSI. I could make a patch to scsi-misc to add tag back in. Another
> > option if it is still needed is to switch to "->name == "generic").
> >
> > Though I have not used this interface I thought if one was using
> > an sg
> > device to a ide-scsi device and the flag was set that sg
> > commands that
> > where not 100% the same as ATAP commands where translated.
>
> Well, imho IDESCSI_SG_TRANSFORM is broken in 2.5. Now that ever block
> driver implements the sg ioctls a sg request can come from sd or sr
> aswell.

Christoph,
Thanks for alerting me to the implementation (by Jens) of
the sg driver's SG_IO ioctl in drivers/block/scsi_ioctl.c .

It goes by the name SCSI_IOCTL_SEND_COMMAND which is the same
name as the old ioctl defined in drivers/scsi/scsi_ioctl.c .
As far as I can see if you send that ioctl to a block device
(including an ATA disk) you get the new functionality (i.e.
similar to sg's SG_IO). However if you send that ioctl to
a char device (e.g. st, osst or sg) you get the old
interface (i.e. as found in drivers/scsi/scsi_ioctl.c).
Somewhat confusing.
So old utilities like scsiinfo will now fail for disks
but continue to work for tapes.


I believe IDESCSI_SG_TRANSFORM translated 6 byte
MODE SENSE ** commands (mandatory in SCSI-2, 10 byte
MODE SENSE optional in SCSI-2) to 10 byte MODE SENSE
commands (mandatory in ATAPI, 6 byte MODE SENSE
commands optional (or not supported) in ATAPI). The
correct solution is to stop applications sending 6
byte MODE SENSE commands to CD/DVD writers. I don't
believe cdrecord uses the IDESCSI_SG_TRANSFORM
ioctl anymore. [Perhaps it comes from the cdwrite
days.]

** Equivalent translations for READ(6), WRITE(6) and
MODE_SELECT(6) command are done by the ide-scsi driver.


Doug Gilbert

2002-12-03 08:05:10

by Douglas Gilbert

[permalink] [raw]
Subject: Re: [RFC] remove IDESCSI_SG_TRANSFORM (compile fix)

Douglas Gilbert wrote:
> On Mon, 2002-12-02 at 17:21, Christoph Hellwig wrote:
> > On Fri, Nov 29, 2002 at 04:44:35PM -0800, Mike Anderson wrote:
> > > Thanks for catching this Christoph I thought the only use was inside
> > > SCSI. I could make a patch to scsi-misc to add tag back in. Another
> > > option if it is still needed is to switch to "->name == "generic").
> > >
> > > Though I have not used this interface I thought if one was using
> > > an sg
> > > device to a ide-scsi device and the flag was set that sg
> > > commands that
> > > where not 100% the same as ATAP commands where translated.
> >
> > Well, imho IDESCSI_SG_TRANSFORM is broken in 2.5. Now that ever block
> > driver implements the sg ioctls a sg request can come from sd or sr
> > aswell.
>
> Christoph,
> Thanks for alerting me to the implementation (by Jens) of
> the sg driver's SG_IO ioctl in drivers/block/scsi_ioctl.c .
>
> It goes by the name SCSI_IOCTL_SEND_COMMAND which is the same
> name as the old ioctl defined in drivers/scsi/scsi_ioctl.c .
> As far as I can see if you send that ioctl to a block device
> (including an ATA disk) you get the new functionality (i.e.
> similar to sg's SG_IO). However if you send that ioctl to
> a char device (e.g. st, osst or sg) you get the old
> interface (i.e. as found in drivers/scsi/scsi_ioctl.c).
> Somewhat confusing.
> So old utilities like scsiinfo will now fail for disks
> but continue to work for tapes.

Correcting my previous post ...

On reviewing drivers/block/scsi_ioctl.c it supports both
the SG_IO (i.e. equivalent to the sg driver's SG_IO ioctl)
and the SCSI_IOCTL_SEND_COMMAND ioctl with the old
functionality. So the SCSI_IOCTL_SEND_COMMAND ioctl is
consistent for both block and char (scsi) drivers.

The SG_IO ioctl first appeared in the lk 2.4 series
(2.4.0) while the SCSI_IOCTL_SEND_COMMAND ioctl has
been there for a long time (but has been marked as
deprecated in the source for some time).

Doug Gilbert