[v3,30/32] blockdev: Remove the .bdrv_probe field from BlockDrivers

Commit Message

[e-mail] July 5, 2016, 3:24 p.m. UTC

This commit finalizes the separation of the block driver and probe
function by removing the .bdrv_probe field from all BlockDrivers.
Probing is now accomplished solely by iterating over the array of probe
function pointers in the format_probes array.

Signed-off-by: Colin Lord <[e-mail]>
---
 block.c                         | 20 +-------------------
 block/raw-posix.c               |  1 -
 include/block/block_int.h       |  1 -
 scripts/modules/module_block.py | 10 ++--------
 4 files changed, 3 insertions(+), 29 deletions(-)

Comments

Max Reitz July 6, 2016, 4:17 p.m. UTC | #1

On 05.07.2016 17:24, Colin Lord wrote:
> This commit finalizes the separation of the block driver and probe
> function by removing the .bdrv_probe field from all BlockDrivers.
> Probing is now accomplished solely by iterating over the array of probe
> function pointers in the format_probes array.
> 
> Signed-off-by: Colin Lord <[e-mail]>
> ---
>  block.c                         | 20 +-------------------
>  block/raw-posix.c               |  1 -
>  include/block/block_int.h       |  1 -
>  scripts/modules/module_block.py | 10 ++--------
>  4 files changed, 3 insertions(+), 29 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 8226124..7e441fe 100644
> --- a/block.c
> +++ b/block.c
> @@ -599,34 +599,16 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
>      const char *format_max = NULL;
>      const char *format;
>      size_t i;
> -    BlockDriver *drv = NULL, *d;
> -
> -    for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
> -        if (block_driver_modules[i].has_probe) {
> -            block_module_load_one(block_driver_modules[i].library_name);
> -        }
> -    }
> -
> -    QLIST_FOREACH(d, &bdrv_drivers, list) {
> -        if (d->bdrv_probe) {
> -            score = d->bdrv_probe(buf, buf_size, filename);
> -            if (score > score_max) {
> -                score_max = score;
> -                drv = d;
> -            }
> -        }
> -    }
>  
>      for (i = 0; i < ARRAY_SIZE(format_probes); i++) {
>          format = format_probes[i](buf, buf_size, filename, &score);
>          if (score > score_max) {
>              score_max = score;
>              format_max = format;
> -            drv = bdrv_find_format(format_max);
>          }
>      }
>  
> -    return drv;
> +    return bdrv_find_format(format_max);

OK, so you move that function call here. Then, I'd at least like to see
a comment in patch 17 that you are going to do this later (like "TODO:
Move this call outside of this loop").

For this patch, however:

Reviewed-by: Max Reitz <[e-mail]>

>  }
>  
>  static int find_image_format(BlockDriverState *bs, const char *filename,

Patch

diff --git a/block.c b/block.c
index 8226124..7e441fe 100644
--- a/block.c
+++ b/block.c
@@ -599,34 +599,16 @@  BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
     const char *format_max = NULL;
     const char *format;
     size_t i;
-    BlockDriver *drv = NULL, *d;
-
-    for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
-        if (block_driver_modules[i].has_probe) {
-            block_module_load_one(block_driver_modules[i].library_name);
-        }
-    }
-
-    QLIST_FOREACH(d, &bdrv_drivers, list) {
-        if (d->bdrv_probe) {
-            score = d->bdrv_probe(buf, buf_size, filename);
-            if (score > score_max) {
-                score_max = score;
-                drv = d;
-            }
-        }
-    }
 
     for (i = 0; i < ARRAY_SIZE(format_probes); i++) {
         format = format_probes[i](buf, buf_size, filename, &score);
         if (score > score_max) {
             score_max = score;
             format_max = format;
-            drv = bdrv_find_format(format_max);
         }
     }
 
-    return drv;
+    return bdrv_find_format(format_max);
 }
 
 static int find_image_format(BlockDriverState *bs, const char *filename,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index bef7a67..a6ad689 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1936,7 +1936,6 @@  BlockDriver bdrv_file = {
     .protocol_name = "file",
     .instance_size = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe = NULL, /* no probe for protocols */
     .bdrv_parse_filename = raw_parse_filename,
     .bdrv_file_open = raw_open,
     .bdrv_reopen_prepare = raw_reopen_prepare,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2057156..2bca115 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -99,7 +99,6 @@  struct BlockDriver {
     bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
                                              BlockDriverState *candidate);
 
-    int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
     int (*bdrv_probe_device)(const char *filename);
 
     /* Any driver implementing this callback is expected to be able to handle
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
index 4075574..18200e2 100644
--- a/scripts/modules/module_block.py
+++ b/scripts/modules/module_block.py
@@ -24,15 +24,13 @@  def get_string_struct(line):
     return data[2].replace('"', '')[:-1]
 
 def add_module(fheader, library, format_name, protocol_name,
-                probe, probe_device):
+               probe_device):
     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:
-        lines.append('.has_probe = true,')
     if probe_device:
         lines.append('.has_probe_device = true,')
 
@@ -52,20 +50,17 @@  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") != -1:
-                    probe = True
                 elif line.find(".bdrv_probe_device") != -1:
                     probe_device = True
                 elif line == "};":
                     add_module(fheader, library, format_name, protocol_name,
-                                probe, probe_device)
+                               probe_device)
                     found_start = False
             elif line.find("static BlockDriver") != -1:
                 found_something = True
                 found_start = True
                 format_name = ""
                 protocol_name = ""
-                probe = False
                 probe_device = False
 
         if not found_something:
@@ -93,7 +88,6 @@  static const struct {
     const char *format_name;
     const char *protocol_name;
     const char *library_name;
-    bool has_probe;
     bool has_probe_device;
 } block_driver_modules[] = {''')