[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from it
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver |
Date: |
Wed, 6 Jul 2016 17:43:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 05.07.2016 17:24, Colin Lord wrote:
> Modifies the bochs probe to return the format name as well as the
> score as the final step of separating the probe function from the
> driver. This keeps the probe completely independent of the driver,
> making future modularization easier to accomplish. Returning the format
> name as well as the score allows the score to be correlated to the
> driver without the probe function needing to be part of the driver.
>
> Signed-off-by: Colin Lord <address@hidden>
> ---
> block.c | 19 +++++++++++++++++++
> block/bochs.c | 1 -
> block/probe/bochs.c | 25 ++++++++++++++++---------
> include/block/probe.h | 3 ++-
> 4 files changed, 37 insertions(+), 11 deletions(-)
As I've proposed before, maybe this would be a good place to rename the
probing function to bdrv_bochs_probe() since it is no longer a static
function.
>
> diff --git a/block.c b/block.c
> index 88a05b2..eab8a6e 100644
> --- a/block.c
> +++ b/block.c
> @@ -25,6 +25,7 @@
> #include "trace.h"
> #include "block/block_int.h"
> #include "block/blockjob.h"
> +#include "block/probe.h"
> #include "qemu/error-report.h"
> #include "module_block.h"
> #include "qemu/module.h"
> @@ -56,6 +57,13 @@
>
> #define NOT_DONE 0x7fffffff /* used while emulated sync operation in
> progress */
>
> +typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
> + const char *filename, int *score);
> +
> +static BdrvProbeFunc *format_probes[] = {
> + bochs_probe,
> +};
Works, but... Eh. Something like the following would suit my personal
tastes (I think by now everyone should have realized that I have
horrible taste) better:
typedef struct BdrvProbeFunc {
const char *format_name;
int (*probe)(const uint8_t *buf, int buf_size,
const char *filename);
} BdrvProbeFunc;
static BdrvProbeFunc *format_probes[] = {
{ "bochs", bochs_probe },
};
It just feels strange to me that the probing function always returns a
constant string.
(This is an optional suggestion, you don't need to follow it.)
> +
> static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
> QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
>
> @@ -576,6 +584,8 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int
> buf_size,
> const char *filename)
> {
> int score_max = 0, score;
> + const char *format_max = NULL;
> + const char *format;
> size_t i;
> BlockDriver *drv = NULL, *d;
>
> @@ -595,6 +605,15 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int
> buf_size,
> }
> }
>
> + 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);
> + }
> + }
I think the bdrv_find_format() call should be done after the loop
(otherwise we may unnecessarily load some formats which we then actually
don't use).
> +
> return drv;
> }
[...]
> diff --git a/block/probe/bochs.c b/block/probe/bochs.c
> index 8adc09f..8206930 100644
> --- a/block/probe/bochs.c
> +++ b/block/probe/bochs.c
> @@ -3,19 +3,26 @@
> #include "block/probe.h"
> #include "block/driver/bochs.h"
>
> -int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
> +const char *bochs_probe(const uint8_t *buf, int buf_size, const char
> *filename,
> + int *score)
> {
> + const char *format = "bochs";
> const struct bochs_header *bochs = (const void *)buf;
> + assert(score);
> + *score = 0;
>
> - if (buf_size < HEADER_SIZE)
> - return 0;
> + if (buf_size < HEADER_SIZE) {
> + return format;
> + }
>
> if (!strcmp(bochs->magic, HEADER_MAGIC) &&
> - !strcmp(bochs->type, REDOLOG_TYPE) &&
> - !strcmp(bochs->subtype, GROWING_TYPE) &&
> - ((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
> - (le32_to_cpu(bochs->version) == HEADER_V1)))
> - return 100;
> + !strcmp(bochs->type, REDOLOG_TYPE) &&
> + !strcmp(bochs->subtype, GROWING_TYPE) &&
> + ((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
> + (le32_to_cpu(bochs->version) == HEADER_V1))) {
> + *score = 100;
> + return format;
> + }
Ah. Seems like I've complained too early. :-)
Max
>
> - return 0;
> + return format;
> }
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v3 29/32] blockdev: Separate vpc probe from its driver, (continued)
- [Qemu-devel] [PATCH v3 31/32] blockdev: Separate out bdrv_probe_device functions, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 30/32] blockdev: Remove the .bdrv_probe field from BlockDrivers, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 32/32] blockdev: Remove bdrv_probe_device field from BlockDriver, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 16/32] blockdev: Move vpc probe to its own file, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 22/32] blockdev: Separate qcow probe from its driver, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 21/32] blockdev: Separate parallels probe from its driver, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 27/32] blockdev: Separate vhdx probe from its driver, Colin Lord, 2016/07/05
- [Qemu-devel] [PATCH v3 25/32] blockdev: Separate raw probe from its driver, Colin Lord, 2016/07/05