Re: [PATCH] st: Do not rewind for SG_IO

[Hannes Reinecke <hare@suse.de>]

On 02/02/2014 08:15 PM, "Kai Mäkisara (Kolumbus)" wrote:
> 
> On 2.2.2014, at 13.42, Hannes Reinecke <hare@suse.de> wrote:
> 
> [part of explanation snipped from reply]
> 
>> But we're now trying to deprecate the original (and unmaintained)
>> scsi_id program and replace it with the standard 'sg_inq' program.
>> Which is a standard program which just issues the respective SCSI
>> command; most of the post-processing will be done by udev rules.
>> And implementing the same workaround here is really a bit hackish.
>> Hence this proposal to allow 'sg_inq' (or any program from sg3_utils)
>> to be called without interrupting normal operations on a tape device.
>>
> OK. After your explanation I think adding the new MTIOCTOP operation is the
> least ugly solution :-)
> 
Ok.

>>> If you absolutely have to do this, then do it. But new ioctls are deprecated and
>>> also it is a bad habit to change the kernel to make things easier for a single
>>> program.
>>>
>> Well, the actual problem here is that the 'st' driver is not designed for
>> multi-initiator environments. The original design for the driver
assumed
>> that a single program had control over the 'st' driver, and there
is only
>> one instance talking to the hardware.
>> Which simply doesn't fit well with the modern, asynchronous, setup.
>>
> The basic problem is that a sequential access device does not fit well the
> asynchronous setup. I admit that some simplifications have been
made because of this.
> 
>> And it's not just udev which suffers here; try to setup multipath on a tape device ...
>>
>>>> I'll be drafting up a patch.
>>>>
>>> If you do, don’t forget to update the documentation.
>>>
>> Okay. New patch attached.
>>
> I think you have not compiled the patched st.c:
> 
> @@ -2263,6 +2265,8 @@ static int st_set_options(struct scsi_tape *STp, long options)
>  			STm->sysv = value;
>  		if ((options & MT_ST_SILI) != 0)
>  			STp->sili = value;
> +		if (value && (options & MT_ST_NOREWIND) != 0)
> +			Stp->rew_at_close = 0;
>                          ^^^
> Should be STp.
> 
> However, looking at the code already suggests that this operation does not belong to MT_ST_*.
> These bits set/clear options that persist. The operations you need
is transient so that the
> effect disappears when the device is closed. I think it would be
better to define it as and
> ordinary MTIOCTOP operation code, e.g., in mtio.h:
> 
> #define MTNOAUTOREWIND 36  /* suppress possible pending automatic rewind */
> 
> or something like that?
>
Ok, can do.

Let's draft up a new version.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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