Re: [Qemu-devel] [PATCH 3/4] raw-posix: split hdev drivers

[Christoph Hellwig <hch@lst.de>]

On Mon, May 25, 2009 at 02:08:10PM +0300, Avi Kivity wrote:
> Christoph Hellwig wrote:
> >Instead of declaring one BlockDriver for all host devices declared one
> >for each type:  a generic one for normal disk devices, a Linux floppy
> >driver and a CDROM driver for Linux and FreeBSD.  This gets rid of a lot
> >of messy ifdefs and switching based on the type in the various removal
> >device methods.
> >
> >block.c grows a new method to find the correct host device driver based
> >on OS-sepcific criteria.  I would love to move this into some OS-dependant
> >file but I don't think we have a place where it fits nicely yet.
> >
> >  
> 
> Add a ->probe_host_device() which accepts the filename (or maybe an fd) 
> as a parameter.  First pass does a ->probe_host_device() for all drivers 
> that support it, second pass tries ->probe().

Here's a patch to implement that incrementally.  I wonder why we have
the is_windows_drive check in find_protocol - shouldn't that be handled
entirely in find_hdev_driver?


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/block/raw-posix.c
===================================================================
--- qemu.orig/block/raw-posix.c	2009-05-26 10:42:04.372841227 +0200
+++ qemu/block/raw-posix.c	2009-05-26 10:44:15.290813943 +0200
@@ -946,6 +946,22 @@ kern_return_t GetBSDPath( io_iterator_t 
 
 #endif
 
+static int hdev_probe_device(const char *filename)
+{
+    struct stat st;
+
+    /* allow a dedicated CD-ROM driver to match with a higher priority */
+    if (strstart(filename, "/dev/cdrom", NULL))
+        return 50;
+
+    if (stat(filename, &st) >= 0 &&
+            (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
+        return 100;
+    }
+
+    return 0;
+}
+
 static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVRawState *s = bs->opaque;
@@ -1138,6 +1154,7 @@ static int hdev_create(const char *filen
 static BlockDriver bdrv_host_device = {
     .format_name	= "host_device",
     .instance_size	= sizeof(BDRVRawState),
+    .bdrv_probe_device	= hdev_probe_device,
     .bdrv_open		= hdev_open,
     .bdrv_close		= raw_close,
     .bdrv_create        = hdev_create,
@@ -1187,6 +1204,14 @@ static int floppy_open(BlockDriverState 
     return 0;
 }
 
+static int floppy_probe_device(const char *filename)
+{
+    if (strstart(filename, "/dev/fd", NULL))
+        return 100;
+    return 0;
+}
+
+
 static int floppy_is_inserted(BlockDriverState *bs)
 {
     return fd_open(bs) >= 0;
@@ -1232,6 +1257,7 @@ static int floppy_eject(BlockDriverState
 static BlockDriver bdrv_host_floppy = {
     .format_name        = "host_floppy",
     .instance_size      = sizeof(BDRVRawState),
+    .bdrv_probe_device	= floppy_probe_device,
     .bdrv_open          = floppy_open,
     .bdrv_close         = raw_close,
     .bdrv_create        = hdev_create,
@@ -1265,6 +1291,13 @@ static int cdrom_open(BlockDriverState *
     return raw_open_common(bs, filename, flags);
 }
 
+static int cdrom_probe_device(const char *filename)
+{
+    if (strstart(filename, "/dev/cd", NULL))
+        return 100;
+    return 0;
+}
+
 static int cdrom_is_inserted(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
@@ -1309,6 +1342,7 @@ static int cdrom_set_locked(BlockDriverS
 static BlockDriver bdrv_host_cdrom = {
     .format_name        = "host_cdrom",
     .instance_size      = sizeof(BDRVRawState),
+    .bdrv_probe_device	= cdrom_probe_device,
     .bdrv_open          = cdrom_open,
     .bdrv_close         = raw_close,
     .bdrv_create        = hdev_create,
@@ -1355,6 +1389,14 @@ static int cdrom_open(BlockDriverState *
     return 0;
 }
 
+static int cdrom_probe_device(const char *filename)
+{
+    if (strstart(filename, "/dev/cd", NULL) ||
+            strstart(filename, "/dev/acd", NULL))
+        return 100;
+    return 0;
+}
+
 static int cdrom_reopen(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
@@ -1425,6 +1467,7 @@ static int cdrom_set_locked(BlockDriverS
 static BlockDriver bdrv_host_cdrom = {
     .format_name        = "host_cdrom",
     .instance_size      = sizeof(BDRVRawState),
+    .bdrv_probe_device	= cdrom_probe_device,
     .bdrv_open          = cdrom_open,
     .bdrv_close         = raw_close,
     .bdrv_create        = hdev_create,
Index: qemu/block_int.h
===================================================================
--- qemu.orig/block_int.h	2009-05-26 10:42:04.391815291 +0200
+++ qemu/block_int.h	2009-05-26 10:51:38.200838865 +0200
@@ -48,6 +48,7 @@ struct BlockDriver {
     const char *format_name;
     int instance_size;
     int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
+    int (*bdrv_probe_device)(const char *filename);
     int (*bdrv_open)(BlockDriverState *bs, const char *filename, int flags);
     int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
                      uint8_t *buf, int nb_sectors);
@@ -186,4 +187,8 @@ void *qemu_blockalign(BlockDriverState *
 
 extern BlockDriverState *bdrv_first;
 
+#ifdef _WIN32
+int is_windows_drive(const char *filename);
+#endif
+
 #endif /* BLOCK_INT_H */
Index: qemu/block.c
===================================================================
--- qemu.orig/block.c	2009-05-26 10:42:04.400815321 +0200
+++ qemu/block.c	2009-05-26 10:51:20.142813836 +0200
@@ -260,7 +260,7 @@ static int is_windows_drive_prefix(const
             filename[1] == ':');
 }
 
-static int is_windows_drive(const char *filename)
+int is_windows_drive(const char *filename)
 {
     if (is_windows_drive_prefix(filename) &&
         filename[2] == '\0')
@@ -304,43 +304,23 @@ static BlockDriver *find_protocol(const 
  * Detect host devices. By convention, /dev/cdrom[N] is always
  * recognized as a host CDROM.
  */
-#ifdef _WIN32
 static BlockDriver *find_hdev_driver(const char *filename)
 {
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return bdrv_find_format("host_device");
-    if (is_windows_drive(filename))
-        return bdrv_find_format("host_device");
-    return NULL;
-}
-#else
-static BlockDriver *find_hdev_driver(const char *filename)
-{
-    struct stat st;
+    int score_max = 0, score;
+    BlockDriver *drv = NULL, *d;
 
-#ifdef __linux__
-    if (strstart(filename, "/dev/fd", NULL))
-        return bdrv_find_format("host_floppy");
-    if (strstart(filename, "/dev/cd", NULL))
-        return bdrv_find_format("host_cdrom");
-#elif defined(__FreeBSD__)
-    if (strstart(filename, "/dev/cd", NULL) ||
-        strstart(filename, "/dev/acd", NULL)) {
-        return bdrv_find_format("host_cdrom");
-    }
-#else
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return bdrv_find_format("host_device");
-#endif
-
-    if (stat(filename, &st) >= 0 &&
-            (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
-        return bdrv_find_format("host_device");
+    for (d = first_drv; d; d = d->next) {
+        if (d->bdrv_probe_device) {
+            score = d->bdrv_probe_device(filename);
+            if (score > score_max) {
+                score_max = score;
+                drv = d;
+            }
+        }
     }
 
-    return NULL;
+    return drv;
 }
-#endif
 
 static BlockDriver *find_image_format(const char *filename)
 {
Index: qemu/block/raw-win32.c
===================================================================
--- qemu.orig/block/raw-win32.c	2009-05-26 10:42:04.383816228 +0200
+++ qemu/block/raw-win32.c	2009-05-26 10:50:57.934985432 +0200
@@ -302,6 +302,15 @@ static int find_device_type(BlockDriverS
     }
 }
 
+static int hdev_probe_device(const char *filename)
+{
+    if (strstart(filename, "/dev/cdrom", NULL))
+        return 100;
+    if (is_windows_drive(filename))
+        return 100;
+    return 0;
+}
+
 static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVRawState *s = bs->opaque;
@@ -387,6 +396,7 @@ static int raw_set_locked(BlockDriverSta
 static BlockDriver bdrv_host_device = {
     .format_name	= "host_device",
     .instance_size	= sizeof(BDRVRawState),
+    .bdrv_probe_device	= hdev_probe_device,
     .bdrv_open		= hdev_open,
     .bdrv_close		= raw_close,
     .bdrv_flush		= raw_flush,