diff mbox series

[1/2] scsi: tape: add third party poweron reset handling

Message ID 20230822181413.1210647-1-jmeneghi@redhat.com
State New
Headers show
Series [1/2] scsi: tape: add third party poweron reset handling | expand

Commit Message

John Meneghini Aug. 22, 2023, 6:14 p.m. UTC
Many tape devices will automatically rewind following a poweron/reset.
This can result in data loss as other operations in the driver can write
to the tape when the position is unknown. E.g. MTEOM can write a
filemark at the beginning of the tape. This patch adds code to detect
poweron/reset unit attentions and prevents the driver from writing to
the tape when the position could be unknown.

Customer reported problem description:

We have experienced an issue with the SCSI tape driver (st) which has
led to data loss for us on two separate occasions in production, as well
as in a third case in which we were able to reproduce the failure in our
test environment.

The tape device involved is an Amazon Tape Gateway, a virtual tape
library (VTL) appliance which presents as a series of iSCSI targets
(multiple tape drives and a changer) and is backed by storage in Amazon
S3. The problem is a general one and not limited to any particular SCSI
transport or tape device, though the nature of both iSCSI and the VTL
make data loss somewhat more likely with this combination than with a
physical tape drive.

The observed behavior occurs when an error causes the VTL tape gateway
process (on the appliance) to crash and restart. This interrupts the
iSCSI TCP connections and, when it occurs during a write, causes the
write to fail with EIO. However, we then find that the virtual tape in
question is now completely blank. We raised this issue with AWS support,
thinking this must be a bug in the VTL appliance, but that turns out not
to be the case.

Per AWS support, when the gateway crashes in this manner, its notion of
the current tape position is reset to the beginning of the tape. It also
sets a unit attention condition, such that the next request results in a
CHECK CONDITION status with sense key UNIT ATTENTION and asc/ascq
indicating a device reset. According to their logs the next command
being sent is WRITE FILEMARK, which results in writing an FM at the
beginning of the tape, effectively discarding its contents.

In fact, once the write fails with EIO, our software attempts to recover
by rewinding and repositioning the tape, then resuming operation. If
this fails, it attempts to rewind and reposition again, write a marker
at the end of the tape, and then unmount. It does not under any
circumstances write either data or filemarks without having successfully
positioned the tape to a known point.

What actually happens is that, since the last operation was a write, the
kernel executes an implied MTWEOF operation (which translate to a Write
Filemarks command) before the rewind that was actually requested. This
seems not entirely unreasonable, provided the tape position is known.
However, once this request fails (due to the unit attention condition),
our next rewind attempt also triggers an implied MTWEOF, which does
_not_ fail (the unit attention condition persists only until the
initiator has been notified); this is the command that unexpectedly
erases the tape.

Our analysis is that the st driver is in fact completely ignoring the
UNIT ATTENTION and associated reset notification from the device. This
is not a condition that can be detected in the transport or mid-layer,
as it occurs entirely within the target and is reported only via the
UNIT ATTENTION sense key. The upper driver (i.e. st) needs to detect
this indication and reset its internal model of the device to an unknown
state.

Suggested-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
 drivers/scsi/st.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Kai Mäkisara Aug. 24, 2023, 9:03 a.m. UTC | #1
> On 22. Aug 2023, at 21.14, John Meneghini <jmeneghi@redhat.com> wrote:
> 
> Many tape devices will automatically rewind following a poweron/reset.
> This can result in data loss as other operations in the driver can write
> to the tape when the position is unknown. E.g. MTEOM can write a
> filemark at the beginning of the tape. This patch adds code to detect
> poweron/reset unit attentions and prevents the driver from writing to
> the tape when the position could be unknown.
> 
> Customer reported problem description:
> 
> ...

Good catch!

I think that, in an ideal world, the lower levels should detect the reset (STp->device->was_reset),
but this depends on how you define the model from the user point of view (what the virtual
HBA is). But, in the real world, this is a good safeguard and it solves a real problem.

Thanks, Kai

> Suggested-by: Jeffrey Hutzelman <jhutz@cmu.edu>
> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Acked-by: Kai Mäkisara <kai.makisara@kolumbus.fi <mailto:kai.makisara@kolumbus.fi>>
> ---
> drivers/scsi/st.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 14d7981ddcdd..338aa8c42968 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -414,6 +414,8 @@ static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt)
> if (cmdstatp->have_sense &&
>    cmdstatp->sense_hdr.asc == 0 && cmdstatp->sense_hdr.ascq == 0x17)
> STp->cleaning_req = 1; /* ASC and ASCQ => cleaning requested */
> + if (cmdstatp->have_sense && scode == UNIT_ATTENTION && cmdstatp->sense_hdr.asc == 0x29)
> + STp->pos_unknown = 1; /* ASC => power on / reset */
> 
> STp->pos_unknown |= STp->device->was_reset;
> 
> -- 
> 2.39.3
>
Martin K. Petersen Aug. 25, 2023, 2:17 a.m. UTC | #2
John,

> Many tape devices will automatically rewind following a poweron/reset.
> This can result in data loss as other operations in the driver can
> write to the tape when the position is unknown. E.g. MTEOM can write a
> filemark at the beginning of the tape. This patch adds code to detect
> poweron/reset unit attentions and prevents the driver from writing to
> the tape when the position could be unknown.

Applied to 6.6/scsi-staging, thanks!
Martin K. Petersen Aug. 31, 2023, 1:48 a.m. UTC | #3
On Tue, 22 Aug 2023 14:14:12 -0400, John Meneghini wrote:

> Many tape devices will automatically rewind following a poweron/reset.
> This can result in data loss as other operations in the driver can write
> to the tape when the position is unknown. E.g. MTEOM can write a
> filemark at the beginning of the tape. This patch adds code to detect
> poweron/reset unit attentions and prevents the driver from writing to
> the tape when the position could be unknown.
> 
> [...]

Applied to 6.6/scsi-queue, thanks!

[1/2] scsi: tape: add third party poweron reset handling
      https://git.kernel.org/mkp/scsi/c/9604eea5bd3a
diff mbox series

Patch

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 14d7981ddcdd..338aa8c42968 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -414,6 +414,8 @@  static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt)
 	if (cmdstatp->have_sense &&
 	    cmdstatp->sense_hdr.asc == 0 && cmdstatp->sense_hdr.ascq == 0x17)
 		STp->cleaning_req = 1; /* ASC and ASCQ => cleaning requested */
+	if (cmdstatp->have_sense && scode == UNIT_ATTENTION && cmdstatp->sense_hdr.asc == 0x29)
+		STp->pos_unknown = 1; /* ASC => power on / reset */
 
 	STp->pos_unknown |= STp->device->was_reset;