All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] st: Do not rewind for SG_IO
@ 2014-01-31  8:46 Hannes Reinecke
  2014-01-31 16:36 ` Jeremy Linton
  2014-01-31 16:43 ` Jeremy Linton
  0 siblings, 2 replies; 25+ messages in thread
From: Hannes Reinecke @ 2014-01-31  8:46 UTC (permalink / raw)
  To: James Bottomley; +Cc: Kay Sievers, Kai Makisara, linux-scsi, Hannes Reinecke

Plain 'st' devices are defined to do a rewind on close.
This causes quite some issues when trying to read the
VPD pages to figure out the WWN of the device.
Especially for udev this means we either have to use
another (non-rewinding) device node for this
(and thereby introducing race conditions) or not
generating a persistent device node at all
(and breaking multi-tape setups).

This patch make the tape always non-rewinding
when SG_IO is used, thus allowing udev to get
a proper device id for tapes.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/st.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index dc4826c..da18ce5 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3645,6 +3645,9 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 		case SCSI_IOCTL_GET_IDLUN:
 		case SCSI_IOCTL_GET_BUS_NUMBER:
 			break;
+		case SG_IO:
+			STp->rew_at_close = 0;
+			/* Fallthrough */
 		default:
 			if ((cmd_in == SG_IO ||
 			     cmd_in == SCSI_IOCTL_SEND_COMMAND ||
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-01-31  8:46 [PATCH] st: Do not rewind for SG_IO Hannes Reinecke
@ 2014-01-31 16:36 ` Jeremy Linton
  2014-01-31 16:43 ` Jeremy Linton
  1 sibling, 0 replies; 25+ messages in thread
From: Jeremy Linton @ 2014-01-31 16:36 UTC (permalink / raw)
  To: Hannes Reinecke, James Bottomley; +Cc: Kay Sievers, Kai Makisara, linux-scsi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 1/31/2014 2:46 AM, Hannes Reinecke wrote:

> This patch make the tape always non-rewinding when SG_IO is used, thus
> allowing udev to get a proper device id for tapes.

	This is wholly bad. Just because someone fires a SG ioctl at the device
(usually to perform an operation that cannot be done with the st_ops) doesn't
mean they don't want the tape rewound on close.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS69EBAAoJEL5i86xrzcy7Ke0IAJbFISKHpJXuuWkK5EveElgG
8+Oy/ndRTSilqg5Ghn4Givr6LnVgs2hZVu6RUz3Y4WADwehxMof3iq6VhqN8bwkr
Zun40DxZAwrxAQQJ8jn+0grKbiL/GdkTr6CwVJ7AUC1odFUOXd9tCqKa8YEzsRwQ
dfoHBqU3cgGFir/l9wlvz0n+9kR4O3Y81IzCTJNAaLNRDelss6eqKEXuRI/53/5y
K5WcYSxHNvqpBLWlRRF2fouyrxiVdsYr4WGoJZf9ReMK5UV8Ztr3YFG7HsRAAyTA
b9PzWQF160U73sh6UFIjxG1UNmkBMxilLdQTJWfVHrQTeWakXRIV9gYB/0Z2l2Q=
=teWU
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-01-31  8:46 [PATCH] st: Do not rewind for SG_IO Hannes Reinecke
  2014-01-31 16:36 ` Jeremy Linton
@ 2014-01-31 16:43 ` Jeremy Linton
  2014-02-01 14:06   ` Hannes Reinecke
  1 sibling, 1 reply; 25+ messages in thread
From: Jeremy Linton @ 2014-01-31 16:43 UTC (permalink / raw)
  To: Hannes Reinecke, James Bottomley; +Cc: Kay Sievers, Kai Makisara, linux-scsi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 1/31/2014 2:46 AM, Hannes Reinecke wrote:

> This patch make the tape always non-rewinding when SG_IO is used, thus
> allowing udev to get a proper device id for tapes.

	Maybe instead of silently changing the behavior, if you just _HAVE_ to open
the st device, add an ioctl or st/mt_op that disables the rewind on close.
That way applications have to explicitly disable the rewind on close.







-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS69K7AAoJEL5i86xrzcy7aC8IAJWoag7UaFselARB6eZ4Zfvm
qi0Fho04TkqnNUJ5VEU81p05XwPJrmonrmqK55kR0PVkMT3o4Wp/KpkeN7gwrQjx
ecR1Ckpoo4Q6n3W/HY06amN6qxLHgwi8RuU9vF7gjRZP4xqW57WRZz1GcuerD94n
tF/i2Ajev6ZsdmRSCUN9DDFDR5RNKZ+XmiX3ihx4L1v27I/zMEteO66pDEIRdCoM
laJnzsEfh/VNZdLeB3wck5xnW6HVq9YgqtH/oV+2LHeHg/Ji626g5/qsjhaA3YJQ
asol8MJsbBGIcaRKEa9EJYy76GVFyCkMLywVFEyN7F9xcFD75P5p2a4siMlYzBc=
=7Viv
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-01-31 16:43 ` Jeremy Linton
@ 2014-02-01 14:06   ` Hannes Reinecke
  2014-02-01 15:23     ` "Kai Mäkisara (Kolumbus)"
  0 siblings, 1 reply; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-01 14:06 UTC (permalink / raw)
  To: Jeremy Linton, James Bottomley; +Cc: Kay Sievers, Kai Makisara, linux-scsi

On 01/31/2014 05:43 PM, Jeremy Linton wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 1/31/2014 2:46 AM, Hannes Reinecke wrote:
>
>> This patch make the tape always non-rewinding when SG_IO is used, thus
>> allowing udev to get a proper device id for tapes.
>
> 	Maybe instead of silently changing the behavior, if you just _HAVE_ to open
> the st device, add an ioctl or st/mt_op that disables the rewind on close.
> That way applications have to explicitly disable the rewind on close.
>
Okay, that sounds like a better alternative.
Point is for udev we simply _have_ to use the given device node.
And when this happens to be set to rewind on close we're doomed.

I'll be drafting up a patch.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-01 14:06   ` Hannes Reinecke
@ 2014-02-01 15:23     ` "Kai Mäkisara (Kolumbus)"
  2014-02-02 11:42       ` Hannes Reinecke
  0 siblings, 1 reply; 25+ messages in thread
From: "Kai Mäkisara (Kolumbus)" @ 2014-02-01 15:23 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Jeremy Linton, James Bottomley, Kay Sievers, linux-scsi


On 1.2.2014, at 16.06, Hannes Reinecke <hare@suse.de> wrote:

> On 01/31/2014 05:43 PM, Jeremy Linton wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>> 
>> On 1/31/2014 2:46 AM, Hannes Reinecke wrote:
>> 
>>> This patch make the tape always non-rewinding when SG_IO is used, thus
>>> allowing udev to get a proper device id for tapes.
>> 
>> 	Maybe instead of silently changing the behavior, if you just _HAVE_ to open
>> the st device, add an ioctl or st/mt_op that disables the rewind on close.
>> That way applications have to explicitly disable the rewind on close.
>> 
> Okay, that sounds like a better alternative.
> Point is for udev we simply _have_ to use the given device node.
> And when this happens to be set to rewind on close we're doomed.
> 
I don’t quite understand why it has to use the auto-rewind device instead of the
non-rewind device. From the minor number it can see what the non-rewind device
is. If the problem is that it is not guaranteed that the non-rewind device exits, you
should rather change the order the devices are created.

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.

> I'll be drafting up a patch.
> 
If you do, don’t forget to update the documentation.

Thanks,
Kai

P.S. I don’t like the auto-rewind devices at all. But the Unix systems have those
and so we also must to have those ;-)

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-01 15:23     ` "Kai Mäkisara (Kolumbus)"
@ 2014-02-02 11:42       ` Hannes Reinecke
  2014-02-02 19:15         ` "Kai Mäkisara (Kolumbus)"
  2014-02-03 14:50         ` Jeremy Linton
  0 siblings, 2 replies; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-02 11:42 UTC (permalink / raw)
  To: "Kai Mäkisara (Kolumbus)"
  Cc: Jeremy Linton, James Bottomley, Kay Sievers, linux-scsi

[-- Attachment #1: Type: text/plain, Size: 3658 bytes --]

On 02/01/2014 04:23 PM, "Kai Mäkisara (Kolumbus)" wrote:
>
> On 1.2.2014, at 16.06, Hannes Reinecke <hare@suse.de> wrote:
>
>> On 01/31/2014 05:43 PM, Jeremy Linton wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> On 1/31/2014 2:46 AM, Hannes Reinecke wrote:
>>>
>>>> This patch make the tape always non-rewinding when SG_IO is used, thus
>>>> allowing udev to get a proper device id for tapes.
>>>
>>> 	Maybe instead of silently changing the behavior, if you just _HAVE_ to open
>>> the st device, add an ioctl or st/mt_op that disables the rewind on close.
>>> That way applications have to explicitly disable the rewind on close.
>>>
>> Okay, that sounds like a better alternative.
>> Point is for udev we simply _have_ to use the given device node.
>> And when this happens to be set to rewind on close we're doomed.
>>
> I don’t quite understand why it has to use the auto-rewind device instead of the
> non-rewind device. From the minor number it can see what the non-rewind device
> is. If the problem is that it is not guaranteed that the non-rewind device exits, you
> should rather change the order the devices are created.
>
This is due to the strictly sequential design udev has.
Essentially udev spawns a worker for every device which
gets created (= udev receives a uevent for).
While udev can and does make sure that all parent devices
are created and all events for those device are processed,
it _cannot_ make any assumptions about devices at the same level.
So when udev spawns a worker for the 'st' device, it is guaranteed that 
the uevent for the scsi_device has been processed.
It is likely that the uevent for the 'nst' device has been received by 
udev, but it positively impossible to make any assumptions what the
udev worker for the 'nst' device may or may not have done.
Nor whether it has been processed at all; it might be stuck with some 
other programs for all we know.

So you cannot blindly try to import the values from the 'nst' device; 
they might not be present at that time.
And redirecting the ioctl to another node is possible;
that's actually what we did for the old 'scsi_id' program.
There we implemented a mechanism to use the 'nst' node instead of the 
'st' one for these cases.

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.

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

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.

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)

[-- Attachment #2: st-implement-MT_ST_NOREWIND.patch --]
[-- Type: text/x-patch, Size: 2348 bytes --]

>From 6d0f1d306660c645d72f8e1543ce0bebc719f10b Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@suse.de>
Date: Sun, 2 Feb 2014 12:03:09 +0100
Subject: [PATCH] st: Implement MT_ST_NOREWIND option

This patch implements the MT_ST_NOREWIND option to inhibit the
rewind on close setting even on normal 'st' devices.
With this patch programs like sg_inq can retrieve the device
identification without interrupting the current operation.

Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/Documentation/scsi/st.txt b/Documentation/scsi/st.txt
index f346abb..93906d0 100644
--- a/Documentation/scsi/st.txt
+++ b/Documentation/scsi/st.txt
@@ -399,6 +399,8 @@ MTSETDRVBUFFER
 		correctly returns transfer residuals
 	     MT_ST_DEBUGGING debugging (global; debugging must be
 		compiled into the driver)
+	     MT_ST_NOREWIND disable rewind-on-close even on autorewind
+	        tape devices
 	MT_ST_SETBOOLEANS
 	MT_ST_CLEARBOOLEANS
 	   Sets or clears the option bits.
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index a1d6986..f550d0d 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -2228,6 +2228,8 @@ static int st_set_options(struct scsi_tape *STp, long options)
 		STp->immediate_filemark = (options & MT_ST_NOWAIT_EOF) != 0;
 		STm->sysv = (options & MT_ST_SYSV) != 0;
 		STp->sili = (options & MT_ST_SILI) != 0;
+		if (options & MT_ST_NOREWIND)
+			STp->rew_at_close = 0; 
 		DEB( debugging = (options & MT_ST_DEBUGGING) != 0;
 		     st_log_options(STp, STm, name); )
 	} else if (code == MT_ST_SETBOOLEANS || code == MT_ST_CLEARBOOLEANS) {
@@ -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;
                 DEB(
 		if ((options & MT_ST_DEBUGGING) != 0)
 			debugging = value;
diff --git a/include/uapi/linux/mtio.h b/include/uapi/linux/mtio.h
index 18543e2..e8b0417 100644
--- a/include/uapi/linux/mtio.h
+++ b/include/uapi/linux/mtio.h
@@ -195,6 +195,7 @@ struct	mtpos {
 #define MT_ST_NOWAIT            0x2000
 #define MT_ST_SILI		0x4000
 #define MT_ST_NOWAIT_EOF	0x8000
+#define MT_ST_NOREWIND		0x10000
 
 /* The mode parameters to be controlled. Parameter chosen with bits 20-28 */
 #define MT_ST_CLEAR_DEFAULT	0xfffff

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-02 11:42       ` Hannes Reinecke
@ 2014-02-02 19:15         ` "Kai Mäkisara (Kolumbus)"
  2014-02-03  6:55           ` Hannes Reinecke
  2014-02-03 14:50         ` Jeremy Linton
  1 sibling, 1 reply; 25+ messages in thread
From: "Kai Mäkisara (Kolumbus)" @ 2014-02-02 19:15 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Jeremy Linton, James Bottomley, Kay Sievers, linux-scsi


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

>> 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?

Thanks,
Kai

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-02 19:15         ` "Kai Mäkisara (Kolumbus)"
@ 2014-02-03  6:55           ` Hannes Reinecke
  0 siblings, 0 replies; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-03  6:55 UTC (permalink / raw)
  To: "Kai Mäkisara (Kolumbus)"
  Cc: Jeremy Linton, James Bottomley, Kay Sievers, linux-scsi

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-02 11:42       ` Hannes Reinecke
  2014-02-02 19:15         ` "Kai Mäkisara (Kolumbus)"
@ 2014-02-03 14:50         ` Jeremy Linton
  2014-02-03 15:06           ` Hannes Reinecke
  1 sibling, 1 reply; 25+ messages in thread
From: Jeremy Linton @ 2014-02-03 14:50 UTC (permalink / raw)
  To: Hannes Reinecke, "Kai Mäkisara (Kolumbus)"
  Cc: James Bottomley, Kay Sievers, linux-scsi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/2/2014 5:42 AM, Hannes Reinecke wrote:
> This is due to the strictly sequential design udev has. Essentially udev
> spawns a worker for every device which gets created (= udev receives a
> uevent for).
	
	The part I fail to see in this explanation is why the nst/st/st*a/st*m/etc
handles are being treated as separate devices. They aren't. They are all the
same physical tape device, so why are the nst devices being handled separately
from the st ones?

	Maybe the problem is that you have to many "workers" for the tape device.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS76ykAAoJEL5i86xrzcy7MKMH/0upLuOBOJWzdOfKYq0WmIvZ
eIaaZG2vhckYeS2zZtim5uVFQDp5eTOirmjxfwSGzSTSAmNrQJwzZvBO/vA2/Kqk
wZSKXp/ZGZhw11+6Kg8f1EArQQwT/i3R6BKglLELFvZVvNOUg3KCnd6nE/4k7ysh
H3f6+6/Jb1wUA6h7a65BG7VBQlJ3HqVe01vYTrkb3eYW7IWfN0tX2FMdqYt2zon2
Yo6TRxhTE/dmqJhg8nLB+fA8rUwW7CYU/IX8nsKNn9lPaDdoJ6g22ozpJRrtEZZ+
lt/qL3VxfWu38z0GWhuKuOqY969bMlyaphY7bOgf7LY4osiC7OgarVoxSIgfP9E=
=pIPt
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 14:50         ` Jeremy Linton
@ 2014-02-03 15:06           ` Hannes Reinecke
  2014-02-03 15:08             ` Jeremy Linton
  0 siblings, 1 reply; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-03 15:06 UTC (permalink / raw)
  To: Jeremy Linton, "Kai Mäkisara (Kolumbus)"
  Cc: James Bottomley, Kay Sievers, linux-scsi, Doug Gilbert

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/03/2014 03:50 PM, Jeremy Linton wrote:
> On 2/2/2014 5:42 AM, Hannes Reinecke wrote:
>> This is due to the strictly sequential design udev has.
>> Essentially udev spawns a worker for every device which gets
>> created (= udev receives a uevent for).
>  The part I fail to see in this explanation is why the
> nst/st/st*a/st*m/etc handles are being treated as separate
> devices. They aren't. They are all the same physical tape
> device, so why are the nst devices being handled separately 
> from the st ones?
> 
That's due to udev.
Udev is getting events for each device it should create a device
node for.
So for 'st' it'll get a series of events for 'stX', and another
series of
events for 'nstX'.
Udev will treat each of these events separately, with distinct worker
programs handling them. Each of those workers run fully asynchronous
and cannot access information from other workers.
Udev cannot and should not make assumptions about any correlations
between events; if you need this then you have to explicitly gate
events via the 'collect' mechanism.
But even then you can only access information from the currently
running
process/event; you cannot modify information from prior events.

So when trying to generate unique IDs for tape devices you either
have to

a) modify the program generating the Unique ID to redirect the call
   to a different device node
or

b) to modify the driver to not rewind the tape

We tried approach a) when using the scsi_id program. But as this
program
is rather old and doesn't have a maintainer I'm working on using
'sg_inq' as a replacement. And these kind of hacks really do not
belong in a generic program.
(It would involve create a temporary device node, call SG_IO on
that, and remove the temporary device node again).
So now we're shooting for b).

Unless you have a better idea ...

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)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS77BsAAoJEGz4yi9OyKjPQz8P/1cY+k9KtEezXC2ovuzJmy9T
BjQaoxcTgHbaXykFyEoJeiNQ09XrjAl7IlVOOF9jX3XqC4kYQiBZCThUnC8O3D69
vaZrA6+VR9AoTXbfNzBUdHzEKWTlz5X2ThLzrLPogXS6v5hiw0iBcLq5mvlJxlkc
DBHJrV+ujlKOM/JibNwUNldeYAFnrrzCCH50arJ/Eqa6BcLZ2uXTksACqIVybUoI
nW1yfVzJdsxI5YwPp0N5bnuAPIg3N7zBTcFCVYqTHUiAmrS4HYkHRrC501yuopCA
PS9Gv1KznctEIGudoP5S0Lwn+g/aVSuzOTpo3uUWAa1BNyyvRMRUSzSIUfm2WdAj
+o1mswhhX6NZMM9rFrM35dbiky2Nv/pvc2zcD4pvizsLiHkoQER7GQ9FCaBNXP7d
eK0OF2jJeEPV6LSC97wxH27Vybv/c89wp0HGDaJc+kd8+WZFIvkLbvY7K3BSA2ut
YvuebI59veIYNZEJVEdAGYQ3IEUG8uukbdpqf3Kdr17PQEms/gTW+xYWPo1+hBR7
V58ojgghBxPdVwGRyiSgKgjY/vHLFunm9H83LDrHKdQsKs8JGiTCflPhJEMwwsDo
mhlmd5Ihw20Cyh46sd3Uu1onH0L5Rg0hfDqT913ShUFYi5Y+XvtHEezYn73OxYDr
JoUo9HYfT843MabEympC
=c45s
-----END PGP SIGNATURE-----
--
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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 15:06           ` Hannes Reinecke
@ 2014-02-03 15:08             ` Jeremy Linton
  2014-02-03 20:51               ` Kay Sievers
  2014-02-03 21:16               ` Douglas Gilbert
  0 siblings, 2 replies; 25+ messages in thread
From: Jeremy Linton @ 2014-02-03 15:08 UTC (permalink / raw)
  To: Hannes Reinecke, "Kai Mäkisara (Kolumbus)"
  Cc: James Bottomley, Kay Sievers, linux-scsi, Doug Gilbert

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/3/2014 9:06 AM, Hannes Reinecke wrote:

> That's due to udev. Udev is getting events for each device it should create
> a device node for. So for 'st' it'll get a series of events for 'stX', and
> another series of events for 'nstX'. Udev will treat each of these events
> separately, with distinct worker programs handling them. Each of those
> workers run fully asynchronous and cannot access information from other
> workers.

	So whats wrong with the simple solution? You throw the ones for st away, and
create the st handles from the nst worker.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS77DnAAoJEL5i86xrzcy7j+cH/24oncr+DN1yZ4ReXk3i8QHx
LpI83UgP9EAvvcHJb5Op3IQtojccda1rYmecMS8qLYV0IX33lJg6UXbhkNS/skkR
gFbPdsD/27JqJZvCU02U+ET0zfO5XH833UCRKOsqoA/GMeikLAaUKPV5t65eyCHh
Qy4CYr4Gve9AxMhV9n00IdadOL5NoH/aO+Qb916zeJ2dUng5TUDqQ3WzdlQQdhD5
ObReHBnTBXlsWQdZL2VakP6gEX2ijiZ09GOIeSf1rKz/974OAudmzLsVjF4BwqTA
5JzqQXezYO2CZ8zkiuCCuiuXEwjv3f62rCuqzi5lQByFGDpvJNMZDAU9GjTn9Z8=
=ELYk
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 15:08             ` Jeremy Linton
@ 2014-02-03 20:51               ` Kay Sievers
  2014-02-03 21:11                 ` James Bottomley
  2014-02-03 21:58                 ` Jeremy Linton
  2014-02-03 21:16               ` Douglas Gilbert
  1 sibling, 2 replies; 25+ messages in thread
From: Kay Sievers @ 2014-02-03 20:51 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: Hannes Reinecke, Kai Mäkisara (Kolumbus),
	James Bottomley, linux-scsi, Doug Gilbert

On Mon, Feb 3, 2014 at 4:08 PM, Jeremy Linton <jlinton@tributary.com> wrote:
> On 2/3/2014 9:06 AM, Hannes Reinecke wrote:
>
>> That's due to udev. Udev is getting events for each device it should create
>> a device node for. So for 'st' it'll get a series of events for 'stX', and
>> another series of events for 'nstX'. Udev will treat each of these events
>> separately, with distinct worker programs handling them. Each of those
>> workers run fully asynchronous and cannot access information from other
>> workers.
>
>         So whats wrong with the simple solution? You throw the ones for st away, and
> create the st handles from the nst worker.

This is not simple and not going to happen. Sibling devices in /sys
cannot have a relationship in udev, there are only parent/child
dependencies.

Hannes, can't you just drop the weird auto-rewinding device matches
from the persistent rules, is that really useful today?

Kay

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 20:51               ` Kay Sievers
@ 2014-02-03 21:11                 ` James Bottomley
  2014-02-03 21:58                 ` Jeremy Linton
  1 sibling, 0 replies; 25+ messages in thread
From: James Bottomley @ 2014-02-03 21:11 UTC (permalink / raw)
  To: kay; +Cc: linux-scsi, jlinton, dgilbert, hare, kai.makisara

On Mon, 2014-02-03 at 21:51 +0100, Kay Sievers wrote:
> On Mon, Feb 3, 2014 at 4:08 PM, Jeremy Linton <jlinton@tributary.com> wrote:
> > On 2/3/2014 9:06 AM, Hannes Reinecke wrote:
> >
> >> That's due to udev. Udev is getting events for each device it should create
> >> a device node for. So for 'st' it'll get a series of events for 'stX', and
> >> another series of events for 'nstX'. Udev will treat each of these events
> >> separately, with distinct worker programs handling them. Each of those
> >> workers run fully asynchronous and cannot access information from other
> >> workers.
> >
> >         So whats wrong with the simple solution? You throw the ones for st away, and
> > create the st handles from the nst worker.
> 
> This is not simple and not going to happen. Sibling devices in /sys
> cannot have a relationship in udev, there are only parent/child
> dependencies.
> 
> Hannes, can't you just drop the weird auto-rewinding device matches
> from the persistent rules, is that really useful today?

Regardless of the outcome of these discussions about adding a rewind
ioctl, we can't just kill the auto-rewind devices because that would be
an ABI break (the consequence would be people's backup scripts would
break and nothing annoys sysadmins more than failed backups).

James


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 15:08             ` Jeremy Linton
  2014-02-03 20:51               ` Kay Sievers
@ 2014-02-03 21:16               ` Douglas Gilbert
  2014-02-03 21:24                 ` Kay Sievers
  1 sibling, 1 reply; 25+ messages in thread
From: Douglas Gilbert @ 2014-02-03 21:16 UTC (permalink / raw)
  To: Jeremy Linton, Hannes Reinecke, "Kai Mäkisara (Kolumbus)"
  Cc: James Bottomley, Kay Sievers, linux-scsi

On 14-02-03 10:08 AM, Jeremy Linton wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 2/3/2014 9:06 AM, Hannes Reinecke wrote:
>
>> That's due to udev. Udev is getting events for each device it should create
>> a device node for. So for 'st' it'll get a series of events for 'stX', and
>> another series of events for 'nstX'. Udev will treat each of these events
>> separately, with distinct worker programs handling them. Each of those
>> workers run fully asynchronous and cannot access information from other
>> workers.
>
> 	So whats wrong with the simple solution? You throw the ones for st away, and
> create the st handles from the nst worker.

Doesn't seem to be any good solutions to this
problem. If you can't discovery something without
modifying its state, then what? For udev and/or
sg_inq to know about the special relationship
between st<i> nodes and nst<i> nodes seems
unreasonable IMO.

The cleanest way I can think of is for the st driver to
show this relationship via sysfs. But then why should
udev going mining for that relationship? A sysfs flag
from the st driver indicating a node is "undiscoverable"
might be a start.

A possible hack inside the st driver is to notice that
only the SG_IO ioctl is called on a file handle
between st_open(/dev/st*, O_RDONLY|O_NONBLOCK) and
st_release() then not auto-rewind it.

BTW Recent versions of sg_inq in Linux use
open(<dev>, O_RDONLY|O_NONBLOCK) unless 'sg_inq --block=1'
is given in which case open(<dev>, O_RDONLY) is used.


I just noticed that when scsi_debug makes a tape device,
the st driver silently ignores it, probably because
scsi_debug doesn't support a SSC command that st expects it
to respond to. IMO the st driver should make some noise if
it ever ignores a SCSI device with a PDT of 1 (i.e. a tape).

For example:
# lsscsi -g
[0:0:0:0]    disk    ATA      INTEL SSDSC2CW12 400i  /dev/sda   /dev/sg0
[8:0:0:0]    tape    Linux    scsi_debug       0004  -          /dev/sg1

with the st module loaded and no indication in dmesg.

Doug Gilbert

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 21:16               ` Douglas Gilbert
@ 2014-02-03 21:24                 ` Kay Sievers
  0 siblings, 0 replies; 25+ messages in thread
From: Kay Sievers @ 2014-02-03 21:24 UTC (permalink / raw)
  To: dgilbert
  Cc: Jeremy Linton, Hannes Reinecke, Kai Mäkisara (Kolumbus),
	James Bottomley, linux-scsi

On Mon, Feb 3, 2014 at 10:16 PM, Douglas Gilbert <dgilbert@interlog.com> wrote:
> On 14-02-03 10:08 AM, Jeremy Linton wrote:

>>         So whats wrong with the simple solution? You throw the ones for st
>> away, and
>> create the st handles from the nst worker.
>
>
> Doesn't seem to be any good solutions to this
> problem. If you can't discovery something without
> modifying its state, then what? For udev and/or
> sg_inq to know about the special relationship
> between st<i> nodes and nst<i> nodes seems
> unreasonable IMO.
>
> The cleanest way I can think of is for the st driver to
> show this relationship via sysfs. But then why should
> udev going mining for that relationship? A sysfs flag
> from the st driver indicating a node is "undiscoverable"
> might be a start.

Udev rules explicitely match on device names, that should be fine. It
can just not be included in the match if it does not provide the
needed functionality, or if it is not needed.

Kay

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 20:51               ` Kay Sievers
  2014-02-03 21:11                 ` James Bottomley
@ 2014-02-03 21:58                 ` Jeremy Linton
  2014-02-03 22:15                   ` Kay Sievers
  2014-02-06 13:10                   ` Hannes Reinecke
  1 sibling, 2 replies; 25+ messages in thread
From: Jeremy Linton @ 2014-02-03 21:58 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Hannes Reinecke, "Kai Mäkisara (Kolumbus)",
	James Bottomley, linux-scsi, Doug Gilbert

On 2/3/2014 2:51 PM, Kay Sievers wrote:
> This is not simple and not going to happen. Sibling devices in /sys cannot
> have a relationship in udev, there are only parent/child dependencies.

	Ok.. so if we can't ignore all the "spare" device nodes in a given /sys entry
for a given device. Why open the device to scan it?

	I've often wondered why the serial number isn't part of the data in /sys
along with the manufacture/model. The last tape drive I saw that failed to
respond to inquiry page 0x80 was over a decade ago (probably manufactured in
the early 90s). So enabling it just for tape is pretty safe.


	Matching Manufacturer/model/serial is going to be better than anything your
going to get out of 0x83 anyway. That data is guaranteed to be there, but its
also guaranteed to be unreliable (every device, and every port has a slightly
different set of descriptors they choose to support).

	Plus, your not going to have issues accidentally rewinding a device, or
resetting a tape density, or accidentally turning compression off if you don't
open the device.



> Hannes, can't you just drop the weird auto-rewinding device matches from
> the persistent rules, is that really useful today?

	The relationship between the st and nst devices is leveraged by a large
number of backup applications in the field. If you change it, its likely lots
of breakage will ensue.








^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 21:58                 ` Jeremy Linton
@ 2014-02-03 22:15                   ` Kay Sievers
  2014-02-03 22:26                     ` Jeremy Linton
  2014-02-06 13:10                   ` Hannes Reinecke
  1 sibling, 1 reply; 25+ messages in thread
From: Kay Sievers @ 2014-02-03 22:15 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: Hannes Reinecke, Kai Mäkisara (Kolumbus),
	James Bottomley, linux-scsi, Doug Gilbert

On Mon, Feb 3, 2014 at 10:58 PM, Jeremy Linton <jlinton@tributary.com> wrote:
> On 2/3/2014 2:51 PM, Kay Sievers wrote:
>> This is not simple and not going to happen. Sibling devices in /sys cannot
>> have a relationship in udev, there are only parent/child dependencies.
>
>         Ok.. so if we can't ignore all the "spare" device nodes in a given /sys entry
> for a given device. Why open the device to scan it?
>
>         I've often wondered why the serial number isn't part of the data in /sys
> along with the manufacture/model. The last tape drive I saw that failed to
> respond to inquiry page 0x80 was over a decade ago (probably manufactured in
> the early 90s). So enabling it just for tape is pretty safe.
>
>
>         Matching Manufacturer/model/serial is going to be better than anything your
> going to get out of 0x83 anyway. That data is guaranteed to be there, but its
> also guaranteed to be unreliable (every device, and every port has a slightly
> different set of descriptors they choose to support).
>
>         Plus, your not going to have issues accidentally rewinding a device, or
> resetting a tape density, or accidentally turning compression off if you don't
> open the device.

Well, the predictable symlink for tapes stuff is all pretty old, and
got through quite a few changes over the years. Not sure how much the
/dev/tape/by-*/ links are really used in the field, and what people
really expect here.

(Mis-)using one of the various open() flags which do not make sense
for SG_IO or tapes, which would prevent any state changes still sounds
like the most convincing option to me. In the end it's always the
nicer interface if the device node itself can be used to identify the
device, instead of jumping through /sys or adding more proping caches
to the kernel.

>> Hannes, can't you just drop the weird auto-rewinding device matches from
>> the persistent rules, is that really useful today?
>
>         The relationship between the st and nst devices is leveraged by a large
> number of backup applications in the field. If you change it, its likely lots
> of breakage will ensue.

Sure, but do they really expect the additional symlinks udev creates?
We are not talking about the primary device nodes, only about the
additional /dev/tape/by-*/ links.

Kay

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 22:15                   ` Kay Sievers
@ 2014-02-03 22:26                     ` Jeremy Linton
  0 siblings, 0 replies; 25+ messages in thread
From: Jeremy Linton @ 2014-02-03 22:26 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Hannes Reinecke, "Kai Mäkisara (Kolumbus)",
	James Bottomley, linux-scsi, Doug Gilbert

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/3/2014 4:15 PM, Kay Sievers wrote:
> got through quite a few changes over the years. Not sure how much the 
> /dev/tape/by-*/ links are really used in the field, and what people really
> expect here.
	I'm not sure they actually work that much either (random opensuse 12.3 machine).

lrwxrwxrwx 1 root root  9 Feb  3 15:31 /dev/tape/by-id/scsi-3201034d7b4000007
- -> ../../st3
lrwxrwxrwx 1 root root 10 Feb  3 15:31
/dev/tape/by-id/scsi-3201034d7b4000007-nst -> ../../nst1

That is what shows up on my machine with 4 tape drives /dev/st0->st4. Its
because there are 4 devices with the same I_T and different luns. The /dev/st*
and /dev/nst* are correct.





> Sure, but do they really expect the additional symlinks udev creates? We
> are not talking about the primary device nodes, only about the additional
> /dev/tape/by-*/ links.

	Oh... I suspect your right. If anyone is using it is probably one off scripts.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS8BeoAAoJEL5i86xrzcy7wBkH/jn9wewZIU7DrVRLc0OiHTDh
Cm7Fo4wir01i8t29P+qL3EgsMzoe7w3xfCJ74rRks5UO5Vl13NmI0ck3rJ+ElTf7
9MJl40cgQgYB+ATCMy91mcObr6KTwkRI1n1oZZKV8YAd20mGSDIEv9FeqHGEWOC4
c3THh+6C0Fb63cYQ6X3oyK8twHNJl8HAJPRbnZ0oKdvSFdoAGBXoBxQ7mbsaWBcI
HkTF6ArHdIMGMeI4F1PUROKMsSeEhwzudvJY3+fb7pQO3pmG6aFTCTVlkSZiDVNX
uqhbB9cdz6c0GYHVg/3jCEsmdzMmlT/qY0jy6NChBPgdFpsS1CAfbnYW2eKtpS4=
=ANj0
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-03 21:58                 ` Jeremy Linton
  2014-02-03 22:15                   ` Kay Sievers
@ 2014-02-06 13:10                   ` Hannes Reinecke
  2014-02-06 13:21                     ` Martin K. Petersen
  1 sibling, 1 reply; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-06 13:10 UTC (permalink / raw)
  To: Jeremy Linton, Kay Sievers
  Cc: "Kai Mäkisara (Kolumbus)",
	James Bottomley, linux-scsi, Doug Gilbert

On 02/03/2014 10:58 PM, Jeremy Linton wrote:
> On 2/3/2014 2:51 PM, Kay Sievers wrote:
>> This is not simple and not going to happen. Sibling devices in /sys cannot
>> have a relationship in udev, there are only parent/child dependencies.
> 
> 	Ok.. so if we can't ignore all the "spare" device nodes in a given /sys entry
> for a given device. Why open the device to scan it?
> 
> 	I've often wondered why the serial number isn't part of the data in /sys
> along with the manufacture/model. The last tape drive I saw that failed to
> respond to inquiry page 0x80 was over a decade ago (probably manufactured in
> the early 90s). So enabling it just for tape is pretty safe.
> 
> 
> 	Matching Manufacturer/model/serial is going to be better than anything your
> going to get out of 0x83 anyway. That data is guaranteed to be there, but its
> also guaranteed to be unreliable (every device, and every port has a slightly
> different set of descriptors they choose to support).
> 
> 	Plus, your not going to have issues accidentally rewinding a device, or
> resetting a tape density, or accidentally turning compression off if you don't
> open the device.
> 
And indeed, I have been wondering about this, too.
And (again) it is something which has been on my To-Do list for a
long time.

Moving EVPD page 0x83 (and maybe 0x80, too) into sysfs will save
quite a lot of headache we have currently; udev won't have to
call 'sg_inq', information will be present even though the
device itself might be temporarily unavailable yadda yadda.

So I've decided to bite the bullet and sent out a patch,
check for 'Add EVPD page 0x83 entries to sysfs'.

Reviews are welcome.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-06 13:10                   ` Hannes Reinecke
@ 2014-02-06 13:21                     ` Martin K. Petersen
  2014-02-06 13:26                       ` Hannes Reinecke
  0 siblings, 1 reply; 25+ messages in thread
From: Martin K. Petersen @ 2014-02-06 13:21 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Jeremy Linton, Kay Sievers, Kai Mäkisara (Kolumbus),
	James Bottomley, linux-scsi, Doug Gilbert

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes> Moving EVPD page 0x83 (and maybe 0x80, too) into sysfs will save
Hannes> quite a lot of headache we have currently; udev won't have to
Hannes> call 'sg_inq', information will be present even though the
Hannes> device itself might be temporarily unavailable yadda yadda.

Hannes> So I've decided to bite the bullet and sent out a patch, check
Hannes> for 'Add EVPD page 0x83 entries to sysfs'.

I'm already doing something similar since I need it for xcopy.

My patch provides both the original VPD 0x83 and 0x80 bits as well as a
handle identical to /sbin/scsi_id.

I'll take a look at your patch later today...

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-06 13:21                     ` Martin K. Petersen
@ 2014-02-06 13:26                       ` Hannes Reinecke
  2014-02-06 13:50                         ` Martin K. Petersen
  0 siblings, 1 reply; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-06 13:26 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Jeremy Linton, Kay Sievers,
	"Kai Mäkisara (Kolumbus)",
	James Bottomley, linux-scsi, Doug Gilbert

On 02/06/2014 02:21 PM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
> 
> Hannes> Moving EVPD page 0x83 (and maybe 0x80, too) into sysfs will save
> Hannes> quite a lot of headache we have currently; udev won't have to
> Hannes> call 'sg_inq', information will be present even though the
> Hannes> device itself might be temporarily unavailable yadda yadda.
> 
> Hannes> So I've decided to bite the bullet and sent out a patch, check
> Hannes> for 'Add EVPD page 0x83 entries to sysfs'.
> 
> I'm already doing something similar since I need it for xcopy.
> 
Hehe. Beat you to it.

> My patch provides both the original VPD 0x83 and 0x80 bits as well as a
> handle identical to /sbin/scsi_id.
> 
Bah, don't do that.
That should better be handled by udev rules.
I've got a set of patches moving from scsi_id to sg_inq, which can
be easily adapted to using sysfs directly.

Once we figure out if that's the direction we want to go ...

> I'll take a look at your patch later today...
> 
Thanks.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-06 13:26                       ` Hannes Reinecke
@ 2014-02-06 13:50                         ` Martin K. Petersen
  2014-02-06 14:38                           ` James Bottomley
  0 siblings, 1 reply; 25+ messages in thread
From: Martin K. Petersen @ 2014-02-06 13:50 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Martin K. Petersen, Jeremy Linton, Kay Sievers,
	Kai Mäkisara (Kolumbus),
	James Bottomley, linux-scsi, Doug Gilbert

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

>> My patch provides both the original VPD 0x83 and 0x80 bits as well as
>> a handle identical to /sbin/scsi_id.
>> 
Hannes> Bah, don't do that.  That should better be handled by udev
Hannes> rules.  I've got a set of patches moving from scsi_id to sg_inq,
Hannes> which can be easily adapted to using sysfs directly.

I just want to get out of the "userland sending random SCSI commands"
business. That is a world of pain right now.

I wanted to provide a handle that was guaranteed to be compatible with
existing tooling. If you have worked around that in udev rules I guess
that's OK with me.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-06 13:50                         ` Martin K. Petersen
@ 2014-02-06 14:38                           ` James Bottomley
  2014-02-06 15:13                             ` Hannes Reinecke
  0 siblings, 1 reply; 25+ messages in thread
From: James Bottomley @ 2014-02-06 14:38 UTC (permalink / raw)
  To: martin.petersen; +Cc: linux-scsi, jlinton, kay, hare, dgilbert, kai.makisara

On Thu, 2014-02-06 at 08:50 -0500, Martin K. Petersen wrote:
> >>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
> 
> >> My patch provides both the original VPD 0x83 and 0x80 bits as well as
> >> a handle identical to /sbin/scsi_id.
> >> 
> Hannes> Bah, don't do that.  That should better be handled by udev
> Hannes> rules.  I've got a set of patches moving from scsi_id to sg_inq,
> Hannes> which can be easily adapted to using sysfs directly.
> 
> I just want to get out of the "userland sending random SCSI commands"
> business. That is a world of pain right now.

Well, in the words of Bill Clinton "I feel your pain".  However, I need
to know we won't pick up that same whole world of pain in the kernel.
Remember it's not just SCSI devices, it's also ATA devices and
potentially other block devices ... then there's blkid and all the weird
things it does. Then there's transport identifiers for multi-path
reservations and so on.

Convince me this path won't have us shifting a whole bunch of weird from
user space to the kernel without any reduction in the weird factor.
Remember the point is not what can we do for all the nicely behaved
SCSI-3 devices, it's what do we do for everything.

James

> I wanted to provide a handle that was guaranteed to be compatible with
> existing tooling. If you have worked around that in udev rules I guess
> that's OK with me.
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-06 14:38                           ` James Bottomley
@ 2014-02-06 15:13                             ` Hannes Reinecke
  2014-02-06 19:21                               ` Douglas Gilbert
  0 siblings, 1 reply; 25+ messages in thread
From: Hannes Reinecke @ 2014-02-06 15:13 UTC (permalink / raw)
  To: James Bottomley, martin.petersen
  Cc: linux-scsi, jlinton, kay, dgilbert, kai.makisara

On 02/06/2014 03:38 PM, James Bottomley wrote:
> On Thu, 2014-02-06 at 08:50 -0500, Martin K. Petersen wrote:
>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>
>>>> My patch provides both the original VPD 0x83 and 0x80 bits as well as
>>>> a handle identical to /sbin/scsi_id.
>>>>
>> Hannes> Bah, don't do that.  That should better be handled by udev
>> Hannes> rules.  I've got a set of patches moving from scsi_id to sg_inq,
>> Hannes> which can be easily adapted to using sysfs directly.
>>
>> I just want to get out of the "userland sending random SCSI commands"
>> business. That is a world of pain right now.
> 
> Well, in the words of Bill Clinton "I feel your pain".  However, I need
> to know we won't pick up that same whole world of pain in the kernel.
> Remember it's not just SCSI devices, it's also ATA devices and
> potentially other block devices ... then there's blkid and all the weird
> things it does. Then there's transport identifiers for multi-path
> reservations and so on.
> 
blkid is actually not so bad, _if_ it would distinguish between
'metadata not found' and 'I/O error during metadata read'.
I made a patch which hopefully should find it's way upstream.

And blkid just issues plain READ commands, so _this_ behaviour won't
change ...

> Convince me this path won't have us shifting a whole bunch of weird from
> user space to the kernel without any reduction in the weird factor.
> Remember the point is not what can we do for all the nicely behaved
> SCSI-3 devices, it's what do we do for everything.
> 
Well, first and foremost the patch should be limited to SCSI-3 (and
later devices). So we'd be insulated from the most obvious crap.

So that leaves only devices which claim to be SCSI-3 or something,
but still keel over when asked for VPD pages.
However, this type of devices will fail already, as 'sd.c' is
already asking for all sorts of VPD pages.
Which leaves only non-Disk devices, but those tend to have a better
SCSI implementation as you can't make quick bucks with, say, as SCSI
Enclosure device.

But the prime motivator behind this patch is that we can be
reasonably sure the device will answer at all.
When retrieving the EVPD pages from userspace you always have the
problem that the device might have gone away or being unresponsive
by the time you get around sending the SG_IO call.
So you always have these stuck user-space processes asking for
information which _had_ been present at one point. In particular
udev is prone for this; anyone who ever came across the message

udevd[]: worker unexpectedly returned with status 0x0100

knows what I'm talking about ...

And the more udev events for a device you get, the higher the
likelyhood for this to happen is.

Plus I could re-use this information for my scsi_dh_alua patchset,
and for xcopy and friends we'll be needing it, too.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] st: Do not rewind for SG_IO
  2014-02-06 15:13                             ` Hannes Reinecke
@ 2014-02-06 19:21                               ` Douglas Gilbert
  0 siblings, 0 replies; 25+ messages in thread
From: Douglas Gilbert @ 2014-02-06 19:21 UTC (permalink / raw)
  To: Hannes Reinecke, James Bottomley, martin.petersen
  Cc: linux-scsi, jlinton, kay, kai.makisara

On 14-02-06 10:13 AM, Hannes Reinecke wrote:
> On 02/06/2014 03:38 PM, James Bottomley wrote:
>> On Thu, 2014-02-06 at 08:50 -0500, Martin K. Petersen wrote:
>>>>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:
>>>
>>>>> My patch provides both the original VPD 0x83 and 0x80 bits as well as
>>>>> a handle identical to /sbin/scsi_id.
>>>>>
>>> Hannes> Bah, don't do that.  That should better be handled by udev
>>> Hannes> rules.  I've got a set of patches moving from scsi_id to sg_inq,
>>> Hannes> which can be easily adapted to using sysfs directly.
>>>
>>> I just want to get out of the "userland sending random SCSI commands"
>>> business. That is a world of pain right now.
>>
>> Well, in the words of Bill Clinton "I feel your pain".  However, I need
>> to know we won't pick up that same whole world of pain in the kernel.
>> Remember it's not just SCSI devices, it's also ATA devices and
>> potentially other block devices ... then there's blkid and all the weird
>> things it does. Then there's transport identifiers for multi-path
>> reservations and so on.
>>
> blkid is actually not so bad, _if_ it would distinguish between
> 'metadata not found' and 'I/O error during metadata read'.
> I made a patch which hopefully should find it's way upstream.
>
> And blkid just issues plain READ commands, so _this_ behaviour won't
> change ...
>
>> Convince me this path won't have us shifting a whole bunch of weird from
>> user space to the kernel without any reduction in the weird factor.
>> Remember the point is not what can we do for all the nicely behaved
>> SCSI-3 devices, it's what do we do for everything.
>>
> Well, first and foremost the patch should be limited to SCSI-3 (and
> later devices). So we'd be insulated from the most obvious crap.
>
> So that leaves only devices which claim to be SCSI-3 or something,
> but still keel over when asked for VPD pages.
> However, this type of devices will fail already, as 'sd.c' is
> already asking for all sorts of VPD pages.
> Which leaves only non-Disk devices, but those tend to have a better
> SCSI implementation as you can't make quick bucks with, say, as SCSI
> Enclosure device.
>
> But the prime motivator behind this patch is that we can be
> reasonably sure the device will answer at all.
> When retrieving the EVPD pages from userspace you always have the
> problem that the device might have gone away or being unresponsive
> by the time you get around sending the SG_IO call.
> So you always have these stuck user-space processes asking for
> information which _had_ been present at one point. In particular
> udev is prone for this; anyone who ever came across the message
>
> udevd[]: worker unexpectedly returned with status 0x0100
>
> knows what I'm talking about ...

Running a small array with an external power supply which
is insufficient for the task of spinning up more than 1 or
2 disks is interesting to watch :-)

Doug Gilbert

> And the more udev events for a device you get, the higher the
> likelyhood for this to happen is.
>
> Plus I could re-use this information for my scsi_dh_alua patchset,
> and for xcopy and friends we'll be needing it, too.
>
> Cheers,
>
> Hannes
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2014-02-06 19:21 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-31  8:46 [PATCH] st: Do not rewind for SG_IO Hannes Reinecke
2014-01-31 16:36 ` Jeremy Linton
2014-01-31 16:43 ` Jeremy Linton
2014-02-01 14:06   ` Hannes Reinecke
2014-02-01 15:23     ` "Kai Mäkisara (Kolumbus)"
2014-02-02 11:42       ` Hannes Reinecke
2014-02-02 19:15         ` "Kai Mäkisara (Kolumbus)"
2014-02-03  6:55           ` Hannes Reinecke
2014-02-03 14:50         ` Jeremy Linton
2014-02-03 15:06           ` Hannes Reinecke
2014-02-03 15:08             ` Jeremy Linton
2014-02-03 20:51               ` Kay Sievers
2014-02-03 21:11                 ` James Bottomley
2014-02-03 21:58                 ` Jeremy Linton
2014-02-03 22:15                   ` Kay Sievers
2014-02-03 22:26                     ` Jeremy Linton
2014-02-06 13:10                   ` Hannes Reinecke
2014-02-06 13:21                     ` Martin K. Petersen
2014-02-06 13:26                       ` Hannes Reinecke
2014-02-06 13:50                         ` Martin K. Petersen
2014-02-06 14:38                           ` James Bottomley
2014-02-06 15:13                             ` Hannes Reinecke
2014-02-06 19:21                               ` Douglas Gilbert
2014-02-03 21:16               ` Douglas Gilbert
2014-02-03 21:24                 ` Kay Sievers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.