diff mbox

[v3,32/32] blockdev: Remove bdrv_probe_device field from BlockDriver

Message ID 1467732272-23368-33-git-send-email-clord@redhat.com
State New
Headers show

Commit Message

clord@redhat.com July 5, 2016, 3:24 p.m. UTC
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 <clord@redhat.com>
---
 block.c                         | 46 ++++++++++++++++++++++++++++++-----------
 block/probe/host_cdrom.c        | 23 ++++++++++++++-------
 block/probe/host_device.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(-)

Comments

Max Reitz July 6, 2016, 4:44 p.m. UTC | #1
On 05.07.2016 17:24, Colin Lord wrote:
> 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 <clord@redhat.com>
> ---
>  block.c                         | 46 ++++++++++++++++++++++++++++++-----------
>  block/probe/host_cdrom.c        | 23 ++++++++++++++-------
>  block/probe/host_device.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(-)

As I suggested for patch 17, I'd split the additions to block.c from the
conversion of the actual probe functions.

> diff --git a/block.c b/block.c
> index 7e441fe..bc1046b 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[] = {
>      bochs_probe,
> @@ -76,6 +77,13 @@ static BdrvProbeFunc *format_probes[] = {
>      vpc_probe
>  };
>  
> +static BdrvProbeDevFunc *protocol_probes[] = {
> +    hdev_probe_device,
> +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__linux__)
> +    cdrom_probe_device
> +#endif
> +};
> +

Same as what I've said in my reply to patch 17: I'd rather have a struct
that contains the name of the protocol and the probe function separated
from each other than have the probe function return the protocol name.

In case you decide not to follow this suggestion (nor the suggestion on
splitting this patch) though:

Reviewed-by: Max Reitz <mreitz@redhat.com>

>  static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
>      QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
>
diff mbox

Patch

diff --git a/block.c b/block.c
index 7e441fe..bc1046b 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[] = {
     bochs_probe,
@@ -76,6 +77,13 @@  static BdrvProbeFunc *format_probes[] = {
     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/probe/host_cdrom.c b/block/probe/host_cdrom.c
index 1886cad..3f7d863 100644
--- a/block/probe/host_cdrom.c
+++ b/block/probe/host_cdrom.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/probe/host_device.c b/block/probe/host_device.c
index ebd969b..b4e4d20 100644
--- a/block/probe/host_device.c
+++ b/block/probe/host_device.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 56b2952..9497f5b 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 44cb503..8c9a3f8 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 2bca115..f0340c6 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 2732f56..c974414 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -27,7 +27,7 @@  const char *vmdk_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
 const char *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 18200e2..e0f5896 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\t'.join(lines)
     fheader.write('\n\t{\n\t' + text + '\n\t},')
@@ -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):