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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

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);
>  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]