Patchwork [v2] raw-posix: Detect CDROM via ioctl

login
register
mail settings
Submitter Cole Robinson
Date Jan. 12, 2010, 3:29 p.m.
Message ID <1263310152-4443-1-git-send-email-crobinso@redhat.com>
Download mbox | patch
Permalink /patch/42732/
State New
Headers show

Comments

Cole Robinson - Jan. 12, 2010, 3:29 p.m.
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(-)
Christoph Hellwig - Jan. 12, 2010, 5:11 p.m.
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.
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

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)