| Message ID | 20241104112623.2675-1-Kai.Makisara@kolumbus.fi |
|---|---|
| Headers | show |
| Series | scsi: st: Device reset patches | expand |
Reviewed-by: John Meneghini <jmeneghi@redhat.com> Tested-by: John Meneghini <jmeneghi@redhat.com> Looks great, please merge. On 11/4/24 06:26, Kai Mäkisara wrote: > Most drives rewind the tape when the device is reset. Reading > and writing are not allowed until something is done to make > the tape position match the user's expectation (e.g., > rewind the tape). Add MTIOCGET and MTLOAD to operations allowed > after reset. MTIOCGET is modified to not touch the tape if > pos_unknown is non-zero. The tape location is known after MTLOAD. > > Signed-off-by: Kai Mäkisara <Kai.Makisara@kolumbus.fi> > --- > drivers/scsi/st.c | 29 +++++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c > index 8d27e6caf027..c9038284bc89 100644 > --- a/drivers/scsi/st.c > +++ b/drivers/scsi/st.c > @@ -3506,6 +3506,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) > int i, cmd_nr, cmd_type, bt; > int retval = 0; > unsigned int blk; > + bool cmd_mtiocget; > struct scsi_tape *STp = file->private_data; > struct st_modedef *STm; > struct st_partstat *STps; > @@ -3619,6 +3620,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) > */ > if (mtc.mt_op != MTREW && > mtc.mt_op != MTOFFL && > + mtc.mt_op != MTLOAD && > mtc.mt_op != MTRETEN && > mtc.mt_op != MTERASE && > mtc.mt_op != MTSEEK && > @@ -3732,17 +3734,28 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) > goto out; > } > > + cmd_mtiocget = cmd_type == _IOC_TYPE(MTIOCGET) && cmd_nr == _IOC_NR(MTIOCGET); > + > if ((i = flush_buffer(STp, 0)) < 0) { > - retval = i; > - goto out; > - } > - if (STp->can_partitions && > - (i = switch_partition(STp)) < 0) { > - retval = i; > - goto out; > + if (cmd_mtiocget && STp->pos_unknown) { > + /* flush fails -> modify status accordingly */ > + reset_state(STp); > + STp->pos_unknown = 1; > + } else { /* return error */ > + retval = i; > + goto out; > + } > + } else { /* flush_buffer succeeds */ > + if (STp->can_partitions) { > + i = switch_partition(STp); > + if (i < 0) { > + retval = i; > + goto out; > + } > + } > } > > - if (cmd_type == _IOC_TYPE(MTIOCGET) && cmd_nr == _IOC_NR(MTIOCGET)) { > + if (cmd_mtiocget) { > struct mtget mt_status; > > if (_IOC_SIZE(cmd_in) != sizeof(struct mtget)) {
Thanks for the fix Kai. While testing these changes with a tape drive I came across one more issue. Link: https://bugzilla.kernel.org/show_bug.cgi?id=219419#c14 I'll send a follow on patch which I think might address that issue. Martin, please merge these patches... we have a number of customers waiting for this fix. /John On 11/4/24 06:26, Kai Mäkisara wrote: > These two patches were developed in response to Bugzilla report > https://bugzilla.kernel.org/show_bug.cgi?id=219419 > > After device reset, the tape driver allows only operations tha don't > write or read anything from tape. The reason for this is that many > (most ?) drives rewind the tape after reset and the subsequent reads > or writes would not be at the tape location the user expects. Reading > and writing is allowed again when the user does something to position the > tape (e.g., rewind). > > The Bugzilla report considers the case when a user, after reset, tries > to read the drive status with MTIOCGET ioctl, but it fails. MTIOCGET > does not return much useful data after reset, but it can be allowed. > MTLOAD positions the tape and it should be allowed. The second patch > adds these to the set of allowed operations after device reset. > > The first patch fixes a bug seen when developing the second patch. > > Kai Mäkisara (2): > scsi: st: Don't modify unknown block number in MTIOCGET > scsi: st: Add MTIOCGET and MTLOAD to ioctls allowed after device reset > > drivers/scsi/st.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) >