Re: [PATCH] libmount: if ENOMEDIUM and tray is open, close tray and retry

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2 Oct 2017 16:11:04 +0200
Stanislav Brabec <sbrabec@xxxxxxx> wrote:

> Michal Suchánek wrote:
> > On Mon, 2 Oct 2017 15:06:33 +0200
> > Stanislav Brabec <sbrabec@xxxxxxx> wrote:
> >   
> >> Michal Suchánek wrote:  
> >>> it is completely reasonable to return ENOMEDIUM from kernel when
> >>> the tray is open and let userspace decide if it wants to attempt
> >>> to close the tray and how long it wants to wait for the tray to
> >>> close (it may be blocked/broken).
> >>>     
> >> But then, what is the purpose of /proc/sys/dev/cdrom/autoclose?  
> > 
> > It can be used as a hint in mount if closing the tray is desired or
> > not.  
> 
> Such feature could be completely implemented in the user space, so
> there is no reason to stay in the kernel in such a limited form.
> 
> 20 years ago, it was a completely working implementation inside the
> kernel. User space just called mount(), and it succeeded.
> 
> If that requires user space loop loop, then there is no reason to
> implement sysfs entry just to perform tray close inside mount()
> syscall and then fail anyway.
> 
> > I guess this used to work with IDE drivers not using SCSI emulation.
> > 
> > With SCSI the close tray command is executed like any other with
> > default timeout and obviously does time out in most cases. 
> > 
> > But how long should the kernel wait? Is 1 minute ok? Should it wait
> > 5 minutes in case the user did not align the medium properly and
> > needs to realign it? Who should set this timeout, where?
> >   
> The timeout should be equal to the time to close tray on the slowest
> device. Guessing something about 3 to 5 seconds may be fine. After
> that time (or several times until that time), just check when the
> tray starts to report that it is closed.

It may take 5s to physically close the drive but it takes much longer
until the medium is detected and can be read. I suspect there might be
some issue with interpreting the timeout in SCSI ioctls, though.

You get 

/* The CDROM is fairly slow, so we need a little extra time */
/* In fact, it is very slow if it has to spin up first */
#define IOCTL_TIMEOUT 30*HZ

which looks like the intended timeout is 30s.

However, the timeout is passed like

cgc.timeout = IOCTL_TIMEOUT (units for struct packet_command
unspecified)

result = scsi_execute(SDev, cgc->cmd, cgc->data_direction,
                              cgc->buffer, cgc->buflen,
                              (unsigned char *)cgc->sense, &sshdr,
                              cgc->timeout, IOCTL_RETRIES, 0, 0, NULL);

and finally

**
 * scsi_execute - insert request and wait for the result
 * @sdev:       scsi device
 * @cmd:        scsi command
 * @data_direction: data direction
 * @buffer:     data buffer
 * @bufflen:    len of buffer
 * @sense:      optional sense buffer
 * @sshdr:      optional decoded sense header
 * @timeout:    request timeout in seconds
 * @retries:    number of times to retry request
 * @flags:      flags for ->cmd_flags
 * @rq_flags:   flags for ->rq_flags
 * @resid:      optional residual length
 *
 * Returns the scsi_cmnd result field if a command was executed, or a
negative
 * Linux error code if we didn't get that far.
 */
int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
                 int data_direction, void *buffer, unsigned bufflen,
                 unsigned char *sense, struct scsi_sense_hdr *sshdr,
                 int timeout, int retries, u64 flags, req_flags_t
rq_flags, int *resid)


meaning that the 30*HZ value is interpreted as seconds if the docs are
right. Looks fishy to me.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux