[Qemu-devel] [PATCH v4 32/32] blockdev: Remove bdrv_probe_device field from BlockDriver

[Colin Lord]

This commit finalizes the separation of the BlockDriver from its
device probing function. Now the accesses to these functions in block.c
occur through the protocol_probes array, and each function returns a
score and protocol name with which to find the corresponding driver.

Signed-off-by: Colin Lord <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
---
 block.c                         | 46 ++++++++++++++++++++++++++++++-----------
 block/host_cdrom-probe.c        | 23 ++++++++++++++-------
 block/host_device-probe.c       | 34 ++++++++++++++++++++----------
 block/raw-posix.c               |  3 ---
 block/raw-win32.c               |  1 -
 include/block/block_int.h       |  2 --
 include/block/probe.h           |  4 ++--
 scripts/modules/module_block.py | 12 ++---------
 8 files changed, 76 insertions(+), 49 deletions(-)

diff --git a/block.c b/block.c
index 20b93cf..a129376 100644
--- a/block.c
+++ b/block.c
@@ -59,6 +59,7 @@
 
 typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
                                   const char *filename, int *score);
+typedef const char *BdrvProbeDevFunc(const char *filename, int *score);
 
 static BdrvProbeFunc *format_probes[] = {
     bdrv_bochs_probe,
@@ -76,6 +77,13 @@ static BdrvProbeFunc *format_probes[] = {
     bdrv_vpc_probe
 };
 
+static BdrvProbeDevFunc *protocol_probes[] = {
+    hdev_probe_device,
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__linux__)
+    cdrom_probe_device
+#endif
+};
+
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
     QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
 
@@ -95,6 +103,8 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
+static BlockDriver *bdrv_do_find_protocol(const char *protocol);
+
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
 {
@@ -487,25 +497,37 @@ int get_tmp_filename(char *filename, int size)
 static BlockDriver *find_hdev_driver(const char *filename)
 {
     int score_max = 0, score;
+    const char *protocol_max = NULL;
+    const char *protocol;
+    BlockDriver *drv;
     size_t i;
-    BlockDriver *drv = NULL, *d;
+
+    for (i = 0; i < ARRAY_SIZE(protocol_probes); i++) {
+        protocol = protocol_probes[i](filename, &score);
+        if (score > score_max) {
+            protocol_max = protocol;
+            score_max = score;
+        }
+    }
+
+    if (!protocol_max) {
+        return NULL;
+    }
+
+    drv = bdrv_do_find_protocol(protocol_max);
+    if (drv) {
+        return drv;
+    }
 
     for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
-        if (block_driver_modules[i].has_probe_device) {
+        if (block_driver_modules[i].protocol_name &&
+            !strcmp(block_driver_modules[i].protocol_name, protocol_max)) {
             block_module_load_one(block_driver_modules[i].library_name);
+            break;
         }
     }
 
-    QLIST_FOREACH(d, &bdrv_drivers, list) {
-        if (d->bdrv_probe_device) {
-            score = d->bdrv_probe_device(filename);
-            if (score > score_max) {
-                score_max = score;
-                drv = d;
-            }
-        }
-    }
-
+    drv = bdrv_do_find_protocol(protocol);
     return drv;
 }
 
diff --git a/block/host_cdrom-probe.c b/block/host_cdrom-probe.c
index 1886cad..3f7d863 100644
--- a/block/host_cdrom-probe.c
+++ b/block/host_cdrom-probe.c
@@ -1,22 +1,28 @@
 #include "qemu/osdep.h"
 #include "block/probe.h"
 
+static const char *protocol = "host_cdrom";
+
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
-int cdrom_probe_device(const char *filename)
+const char *cdrom_probe_device(const char *filename, int *score)
 {
+    assert(score);
     if (strstart(filename, "/dev/cd", NULL) ||
-            strstart(filename, "/dev/acd", NULL))
-        return 100;
+        strstart(filename, "/dev/acd", NULL)) {
+        *score = 100;
+        return protocol;
+    }
     return 0;
 }
 #elif defined(__linux__)
 #include <sys/ioctl.h>
 #include <linux/cdrom.h>
-int cdrom_probe_device(const char *filename)
+const char *cdrom_probe_device(const char *filename, int *score)
 {
     int fd, ret;
-    int prio = 0;
     struct stat st;
+    assert(score);
+    *score = 0;
 
     fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
     if (fd < 0) {
@@ -29,12 +35,13 @@ int cdrom_probe_device(const char *filename)
 
     /* Attempt to detect via a CDROM specific ioctl */
     ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
-    if (ret >= 0)
-        prio = 100;
+    if (ret >= 0) {
+        *score = 100;
+    }
 
 outc:
     qemu_close(fd);
 out:
-    return prio;
+    return protocol;
 }
 #endif
diff --git a/block/host_device-probe.c b/block/host_device-probe.c
index ebd969b..b4e4d20 100644
--- a/block/host_device-probe.c
+++ b/block/host_device-probe.c
@@ -2,29 +2,41 @@
 #include "block/probe.h"
 #include "qemu/cutils.h"
 
+static const char *protocol = "host_device";
+
 #ifdef _WIN32
-int hdev_probe_device(const char *filename)
+const char *hdev_probe_device(const char *filename, int *score)
 {
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return 100;
-    if (is_windows_drive(filename))
-        return 100;
-    return 0;
+    assert(score);
+    *score = 100;
+    if (strstart(filename, "/dev/cdrom", NULL)) {
+        return protocol;
+    }
+    if (is_windows_drive(filename)) {
+        return protocol
+    }
+    *score = 0;
+    return protocol;
 }
 #else
-int hdev_probe_device(const char *filename)
+const char *hdev_probe_device(const char *filename, int *score)
 {
     struct stat st;
+    assert(score);
 
     /* allow a dedicated CD-ROM driver to match with a higher priority */
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return 50;
+    if (strstart(filename, "/dev/cdrom", NULL)) {
+        *score = 50;
+        return protocol;
+    }
 
     if (stat(filename, &st) >= 0 &&
             (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
-        return 100;
+        *score = 100;
+        return protocol;
     }
 
-    return 0;
+    *score = 0;
+    return protocol;
 }
 #endif
diff --git a/block/raw-posix.c b/block/raw-posix.c
index f42b74a..90c65f9 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -2367,7 +2367,6 @@ static BlockDriver bdrv_host_device = {
     .protocol_name        = "host_device",
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe_device  = hdev_probe_device,
     .bdrv_parse_filename = hdev_parse_filename,
     .bdrv_file_open     = hdev_open,
     .bdrv_close         = raw_close,
@@ -2466,7 +2465,6 @@ static BlockDriver bdrv_host_cdrom = {
     .protocol_name      = "host_cdrom",
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe_device = cdrom_probe_device,
     .bdrv_parse_filename = cdrom_parse_filename,
     .bdrv_file_open     = cdrom_open,
     .bdrv_close         = raw_close,
@@ -2592,7 +2590,6 @@ static BlockDriver bdrv_host_cdrom = {
     .protocol_name      = "host_cdrom",
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe_device = cdrom_probe_device,
     .bdrv_parse_filename = cdrom_parse_filename,
     .bdrv_file_open     = cdrom_open,
     .bdrv_close         = raw_close,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 5afab2a..b4f64e7 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -696,7 +696,6 @@ static BlockDriver bdrv_host_device = {
     .instance_size     = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_parse_filename = hdev_parse_filename,
-    .bdrv_probe_device = hdev_probe_device,
     .bdrv_file_open    = hdev_open,
     .bdrv_close                = raw_close,
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 23f229d..87bb487 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -99,8 +99,6 @@ struct BlockDriver {
     bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
                                              BlockDriverState *candidate);
 
-    int (*bdrv_probe_device)(const char *filename);
-
     /* Any driver implementing this callback is expected to be able to handle
      * NULL file names in its .bdrv_open() implementation */
     void (*bdrv_parse_filename)(const char *filename, QDict *options, Error 
**errp);
diff --git a/include/block/probe.h b/include/block/probe.h
index 4d5feb4..e96cd2f 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -27,7 +27,7 @@ const char *bdrv_vmdk_probe(const uint8_t *buf, int buf_size,
                             const char *filename, int *score);
 const char *bdrv_vpc_probe(const uint8_t *buf, int buf_size,
                            const char *filename, int *score);
-int hdev_probe_device(const char *filename);
-int cdrom_probe_device(const char *filename);
+const char *hdev_probe_device(const char *filename, int *score);
+const char *cdrom_probe_device(const char *filename, int *score);
 
 #endif
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
index dba1717..db4fb54 100644
--- a/scripts/modules/module_block.py
+++ b/scripts/modules/module_block.py
@@ -23,16 +23,13 @@ def get_string_struct(line):
 
     return data[2].replace('"', '')[:-1]
 
-def add_module(fheader, library, format_name, protocol_name,
-               probe_device):
+def add_module(fheader, library, format_name, protocol_name):
     lines = []
     lines.append('.library_name = "' + library + '",')
     if format_name != "":
         lines.append('.format_name = "' + format_name + '",')
     if protocol_name != "":
         lines.append('.protocol_name = "' + protocol_name + '",')
-    if probe_device:
-        lines.append('.has_probe_device = true,')
 
     text = '\n        '.join(lines)
     fheader.write('\n    {\n        ' + text + '\n    },')
@@ -50,18 +47,14 @@ def process_file(fheader, filename):
                     format_name = get_string_struct(line)
                 elif line.find(".protocol_name") != -1:
                     protocol_name = get_string_struct(line)
-                elif line.find(".bdrv_probe_device") != -1:
-                    probe_device = True
                 elif line == "};":
-                    add_module(fheader, library, format_name, protocol_name,
-                               probe_device)
+                    add_module(fheader, library, format_name, protocol_name)
                     found_start = False
             elif line.find("static BlockDriver") != -1:
                 found_something = True
                 found_start = True
                 format_name = ""
                 protocol_name = ""
-                probe_device = False
 
         if not found_something:
             print("No BlockDriver struct found in " + filename + ". \
@@ -88,7 +81,6 @@ static const struct {
     const char *format_name;
     const char *protocol_name;
     const char *library_name;
-    bool has_probe_device;
 } block_driver_modules[] = {''')
 
 def print_bottom(fheader):
-- 
2.5.5