diff mbox

[v2] raw-posix: Detect CDROM via ioctl

Message ID 1263310152-4443-1-git-send-email-crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson Jan. 12, 2010, 3:29 p.m. UTC
Current CDROM detection is hardcoded based on source file name.
Make this smarter by attempting a CDROM specific ioctl.

This makes '-cdrom /dev/sr0' succeed with no media present.

v2: Give ioctl check higher priority than filename check

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 block/raw-posix.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig Jan. 12, 2010, 5:11 p.m. UTC | #1
On Tue, Jan 12, 2010 at 10:29:11AM -0500, Cole Robinson wrote:
>  static int cdrom_probe_device(const char *filename)
>  {
> +    int fd, ret, prio;
> +
>      if (strstart(filename, "/dev/cd", NULL))
> -        return 100;
> -    return 0;
> +        prio = 50;
> +
> +    fd = open(filename, O_RDONLY | O_NONBLOCK);
> +    if (fd < 0) {
> +        goto out;
> +    }
> +
> +    /* Attempt to detect via a CDROM specific ioctl */
> +    ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
> +    if (!(ret < 0 && errno == EINVAL))
> +        prio = 100;
> +
> +    close(fd);
> +out:
> +    return prio;
>  }

Looks good.  We'll now get an open each from the cdrom and floppy
drivers for each image we're trying to open, but I guess that should be
fine.
Jamie Lokier Jan. 13, 2010, 11:39 p.m. UTC | #2
Christoph Hellwig wrote:
> On Tue, Jan 12, 2010 at 10:29:11AM -0500, Cole Robinson wrote:
> >  static int cdrom_probe_device(const char *filename)
> >  {
> > +    int fd, ret, prio;
> > +
> >      if (strstart(filename, "/dev/cd", NULL))
> > -        return 100;
> > -    return 0;
> > +        prio = 50;
> > +
> > +    fd = open(filename, O_RDONLY | O_NONBLOCK);
> > +    if (fd < 0) {
> > +        goto out;
> > +    }
> > +
> > +    /* Attempt to detect via a CDROM specific ioctl */
> > +    ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
> > +    if (!(ret < 0 && errno == EINVAL))
> > +        prio = 100;
> > +
> > +    close(fd);
> > +out:
> > +    return prio;
> >  }
> 
> Looks good.  We'll now get an open each from the cdrom and floppy
> drivers for each image we're trying to open, but I guess that should be
> fine.

A theoretical problem, that applies to all devices detected in this way:

A few ioctl values (the integers which the names expand to) overlap
between different devices.  It's not policy but it has happened.  This
shows up in strace sometimes, for example on Linux the isatty() C
library function uses TCGETS, but in strace it shows as:

    SNDCTL_TMR_TIMEBASE or TCGETS

I tried isatty() on Linux sound devices and it seemed to be reliable anyway :-)

But, in theory, on some host or other, the CD-ROM and floppy checks
could do accidentally do something else on the wrong sort of device.
I don't know how assiduous hosts are about keeping the number spaces separate.

-- Jamie
diff mbox

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 5a6a22b..b7254d8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1140,9 +1140,24 @@  static int cdrom_open(BlockDriverState *bs, const char *filename, int flags)
 
 static int cdrom_probe_device(const char *filename)
 {
+    int fd, ret, prio;
+
     if (strstart(filename, "/dev/cd", NULL))
-        return 100;
-    return 0;
+        prio = 50;
+
+    fd = open(filename, O_RDONLY | O_NONBLOCK);
+    if (fd < 0) {
+        goto out;
+    }
+
+    /* Attempt to detect via a CDROM specific ioctl */
+    ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
+    if (!(ret < 0 && errno == EINVAL))
+        prio = 100;
+
+    close(fd);
+out:
+    return prio;
 }
 
 static int cdrom_is_inserted(BlockDriverState *bs)