[PATCH] scsi: st: Fix input/output error on empty drive reset

[PATCH] scsi: st: Fix input/output error on empty drive reset

[Rafael Rocha]

A previous change was introduced to prevent data loss during a power-on reset
when a tape is present inside the drive. This change set the "pos_unknown" flag
to true to avoid operations that could compromise data by performing actions
from an untracked position. The relevant commit is:

Commit: 9604eea5bd3ae1fa3c098294f4fc29ad687141ea
Subject: scsi: st: Add third-party power-on reset handling

As a consequence of this change, a new issue has surfaced: the driver now
returns an "Input/output error" even for empty drives when the drive, host, or
bus is reset. This issue stems from the "flush_buffer" function, which first
checks whether the "pos_unknown" flag is set. If the flag is set, the user will
encounter an "Input/output error" until the tape position is known again. This
behavior differs from the previous implementation, where empty drives were not
affected at system start up time, allowing tape software to send commands to
the driver to retrieve the drive's status and other information.

The current behavior prioritizes the "pos_unknown" flag over the "ST_NO_TAPE"
status, leading to issues for software that detects drives during system
startup. This software will receive an "Input/output error" until a tape is
loaded and its position is known.

To resolve this, the "ST_NO_TAPE" status should take priority when the drive is
empty, allowing communication with the drive following a power-on reset. At the
same time, the change should continue to protect data by maintaining the
"pos_unknown" flag when the drive contains a tape and its position is unknown.

Signed-off-by: Rafael Rocha <rrochavi@fnal.gov>
---
 drivers/scsi/st.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 0d8ce1a92168..be881d5bac05 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -834,6 +834,9 @@ static int flush_buffer(struct scsi_tape *STp, int seek_next)
 	int backspace, result;
 	struct st_partstat *STps;
 
+        if (STp->ready != ST_READY)
+                return 0;
+
 	/*
 	 * If there was a bus reset, block further access
 	 * to this device.
@@ -841,8 +844,6 @@ static int flush_buffer(struct scsi_tape *STp, int seek_next)
 	if (STp->pos_unknown)
 		return (-EIO);
 
-	if (STp->ready != ST_READY)
-		return 0;
 	STps = &(STp->ps[STp->partition]);
 	if (STps->rw == ST_WRITING)	/* Writing */
 		return st_flush_write_buffer(STp);
-- 
2.39.3 (Apple Git-146)

Re: [PATCH] scsi: st: Fix input/output error on empty drive reset

["Kai Mäkisara (Kolumbus)"]

> On 5. Sep 2024, at 20.39, Rafael Rocha <vidurri@gmail.com> wrote:
> 
> A previous change was introduced to prevent data loss during a power-on reset
> when a tape is present inside the drive. This change set the "pos_unknown" flag
> to true to avoid operations that could compromise data by performing actions
> from an untracked position. The relevant commit is:
> 
> Commit: 9604eea5bd3ae1fa3c098294f4fc29ad687141ea
> Subject: scsi: st: Add third-party power-on reset handling
> 
The pos_unknown flag was introduced to prevent writing and reading from an
unknown position (usually when the drive rewinds the tape when the device
is reset). This commit added code to catch a case which the midlevel did
not catch.

> As a consequence of this change, a new issue has surfaced: the driver now
> returns an "Input/output error" even for empty drives when the drive, host, or
> bus is reset. This issue stems from the "flush_buffer" function, which first
> checks whether the "pos_unknown" flag is set. If the flag is set, the user will
> encounter an "Input/output error" until the tape position is known again. This
> behavior differs from the previous implementation, where empty drives were not
> affected at system start up time, allowing tape software to send commands to
> the driver to retrieve the drive's status and other information.
> 
> The current behavior prioritizes the "pos_unknown" flag over the "ST_NO_TAPE"
> status, leading to issues for software that detects drives during system
> startup. This software will receive an "Input/output error" until a tape is
> loaded and its position is known.
> 
> To resolve this, the "ST_NO_TAPE" status should take priority when the drive is
> empty, allowing communication with the drive following a power-on reset. At the
> same time, the change should continue to protect data by maintaining the
> "pos_unknown" flag when the drive contains a tape and its position is unknown.

> Signed-off-by: Rafael Rocha <rrochavi@fnal.gov>
> 
The patch changes the semantics of flush_buffer() slightly. Obviously, nothing should
be flushed if position is unknown, but the return code changes when the drive is not
ready. This changes the path the code takes after reset if the drive is not ready.
I looked at the code and this should not cause problems. So:

Acked-by: Kai Mäkisara <kai.makisara@kolumbus.fi <mailto:kai.makisara@kolumbus.fi>>

As an sdded note: when looking at the code, another possibility might be to not
set pos_unknown if (STp->ready != ST_READY), But if your simple change is
enough, it is wise not to make more complex changes.

Thanks,
Kai

Re: [PATCH] scsi: st: Fix input/output error on empty drive reset

[Martin K. Petersen]

Rafael,

> To resolve this, the "ST_NO_TAPE" status should take priority when the
> drive is empty, allowing communication with the drive following a
> power-on reset. At the same time, the change should continue to
> protect data by maintaining the "pos_unknown" flag when the drive
> contains a tape and its position is unknown.

Applied to 6.12/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: st: Fix input/output error on empty drive reset

[Martin K. Petersen]

On Thu, 05 Sep 2024 12:39:21 -0500, Rafael Rocha wrote:

> A previous change was introduced to prevent data loss during a power-on reset
> when a tape is present inside the drive. This change set the "pos_unknown" flag
> to true to avoid operations that could compromise data by performing actions
> from an untracked position. The relevant commit is:
> 
> Commit: 9604eea5bd3ae1fa3c098294f4fc29ad687141ea
> Subject: scsi: st: Add third-party power-on reset handling
> 
> [...]

Applied to 6.12/scsi-queue, thanks!

[1/1] scsi: st: Fix input/output error on empty drive reset
      https://git.kernel.org/mkp/scsi/c/3d882cca73be

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH] scsi: st: Fix input/output error on empty drive reset

[Rafael Rocha]

Tape drives are returning an "Input/output error" when a drive, the host, or
the bus is reset. This behavior is expected when a tape is present to prevent
data loss. However, the driver currently drops these errors even for empty
drives due to the following change:

Commit: 9604eea5bd3ae1fa3c098294f4fc29ad687141ea
Subject: scsi: st: Add third-party power-on reset handling
Link: https://github.com/torvalds/linux/commit/9604eea5bd3ae1fa3c098294f4fc29ad687141ea

This issue is causing several tape software applications to crash on startup or
when performing drive health checks, as noted in the following CERN CTA Tape
software discussion:
https://cta-community.web.cern.ch/t/input-output-error-from-tape-drive-device-dev-nst0/302

To correct this behavior, it is necessary to either check for the presence of a
tape before blocking the device or revise the drive's readiness verification at
the beginning of the flush function.

Signed-off-by: Rafael Rocha <rrochavi@fnal.gov>
---
 drivers/scsi/st.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 0d8ce1a92168..10bda3543e93 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -834,6 +834,9 @@ static int flush_buffer(struct scsi_tape *STp, int seek_next)
 	int backspace, result;
 	struct st_partstat *STps;
 
+	if (STp->ready != ST_READY)
+                return 0;
+
 	/*
 	 * If there was a bus reset, block further access
 	 * to this device.
@@ -841,8 +844,6 @@ static int flush_buffer(struct scsi_tape *STp, int seek_next)
 	if (STp->pos_unknown)
 		return (-EIO);
 
-	if (STp->ready != ST_READY)
-		return 0;
 	STps = &(STp->ps[STp->partition]);
 	if (STps->rw == ST_WRITING)	/* Writing */
 		return st_flush_write_buffer(STp);
-- 
2.39.3 (Apple Git-146)