Re: [Qemu-devel] [PATCH v7 3/4] blockdev: Add dynamic module loading for block drivers

[Max Reitz]

On 08.08.2016 20:07, Colin Lord wrote:
> From: Marc Mari <address@hidden>
> 
> Extend the current module interface to allow for block drivers to be
> loaded dynamically on request. The only block drivers that can be
> converted into modules are the drivers that don't perform any init
> operation except for registering themselves.
> 
> In addition, only the protocol drivers are being modularized, as they
> are the only ones which see significant performance benefits. The format
> drivers do not generally link to external libraries, so modularizing
> them is of no benefit from a performance perspective.
> 
> All the necessary module information is located in a new structure found
> in module_block.h
> 
> Signed-off-by: Marc Marí <address@hidden>
> Signed-off-by: Colin Lord <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
>  Makefile              |  3 ---
>  block.c               | 62 
> +++++++++++++++++++++++++++++++++++++++++++++------
>  block/Makefile.objs   |  3 +--
>  include/qemu/module.h |  3 +++
>  util/module.c         | 38 +++++++++----------------------
>  5 files changed, 70 insertions(+), 39 deletions(-)
> 

[...]

> diff --git a/block.c b/block.c
> index 30d64e6..6c5e249 100644
> --- a/block.c
> +++ b/block.c
> @@ -26,6 +26,7 @@
>  #include "block/block_int.h"
>  #include "block/blockjob.h"
>  #include "qemu/error-report.h"
> +#include "module_block.h"
>  #include "qemu/module.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qmp/qbool.h"
> @@ -241,17 +242,40 @@ BlockDriverState *bdrv_new(void)
>      return bs;
>  }
>  
> -BlockDriver *bdrv_find_format(const char *format_name)
> +static BlockDriver *bdrv_do_find_format(const char *format_name)
>  {
>      BlockDriver *drv1;
> +
>      QLIST_FOREACH(drv1, &bdrv_drivers, list) {
>          if (!strcmp(drv1->format_name, format_name)) {
>              return drv1;
>          }
>      }
> +
>      return NULL;
>  }
>  
> +BlockDriver *bdrv_find_format(const char *format_name)
> +{
> +    BlockDriver *drv1;
> +    size_t i;
> +
> +    drv1 = bdrv_do_find_format(format_name);
> +    if (drv1) {
> +        return drv1;
> +    }
> +
> +    /* The driver isn't registered, maybe we need to load a module */
> +    for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
> +        if (!strcmp(block_driver_modules[i].format_name, format_name)) {
> +            block_module_load_one(block_driver_modules[i].library_name);
> +            break;
> +        }
> +    }
> +
> +    return bdrv_do_find_format(format_name);
> +}
> +

Did you reintroduce this function for dmg? I thought Fam is taking care
of that? I'm confused as to how Fam's patch for dmg and this series are
supposed to interact; the fact that the script added in patch 2 breaks
down with Fam's patch isn't exactly helping...

Hm, so is this series now supposed to be applied without Fam's patch
with the idea of sorting dmg out later on?

Max

>  static int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
>  {
>      static const char *whitelist_rw[] = {

signature.asc
Description: OpenPGP digital signature