Re: [PATCH v3 1/4] scsi: st: Restore some drive settings after reset

[John Meneghini <jmeneghi@redhat.com>]

I've tested this patch out and it works as expected.

Reviewed-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: John Meneghini <jmeneghi@redhat.com>

On 1/20/25 2:49 PM, Kai Mäkisara wrote:
> Some of the allowed operations put the tape into a known position
> to continue operation assuming only the tape position has changed.
> But reset sets partition, density and block size to drive default
> values. These should be restored to the values before reset.
> 
> Normally the current block size and density are stored by the drive.
> If the settings have been changed, the changed values have to be
> saved by the driver across reset.
> 
> Signed-off-by: Kai Mäkisara <Kai.Makisara@kolumbus.fi>

> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index ebbd50ec0cda..0fc9afe60862 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
>   \f
> @@ -2930,14 +2929,17 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned int cmd_in, unsigned lon
>   		if (cmd_in == MTSETDENSITY) {
>   			(STp->buffer)->b_data[4] = arg;
>   			STp->density_changed = 1;	/* At least we tried ;-) */
> +			STp->changed_density = arg;
>   		} else if (cmd_in == SET_DENS_AND_BLK)
>   			(STp->buffer)->b_data[4] = arg >> 24;
>   		else
>   			(STp->buffer)->b_data[4] = STp->density;
>   		if (cmd_in == MTSETBLK || cmd_in == SET_DENS_AND_BLK) {
>   			ltmp = arg & MT_ST_BLKSIZE_MASK;
> -			if (cmd_in == MTSETBLK)
> +			if (cmd_in == MTSETBLK) {
>   				STp->blksize_changed = 1; /* At least we tried ;-) */
> +				STp->changed_blksize = arg;
> +			}
>   		} else
>   			ltmp = STp->block_size;
>   		(STp->buffer)->b_data[9] = (ltmp >> 16);
> @@ -3636,9 +3638,25 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>   				retval = (-EIO);
>   				goto out;
>   			}
> -			reset_state(STp);
> +			reset_state(STp); /* Clears pos_unknown */
>   			/* remove this when the midlevel properly clears was_reset */
>   			STp->device->was_reset = 0;

I see we are still clearing the mid layer was_reset flag here.  Is there any way we can remove this in a future patch?

/John