[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] blockdev: Add dynamic module loading for bl
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] blockdev: Add dynamic module loading for block drivers |
Date: |
Thu, 23 Jun 2016 10:00:26 +0800 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Wed, 06/22 17:35, 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.
>
> All the necessary module information is located in a new structure found in
> include/qemu/module_block.h
>
> Signed-off-by: Marc Mari <address@hidden>
> Signed-off-by: Colin Lord <address@hidden>
> ---
> Makefile | 3 --
> block.c | 86
> +++++++++++++++++++++++++++++++++++++++++++++------
> include/qemu/module.h | 3 ++
> util/module.c | 37 ++++++----------------
> 4 files changed, 90 insertions(+), 39 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 8f8b6a2..461187c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -247,9 +247,6 @@ Makefile: $(version-obj-y) $(version-lobj-y)
> libqemustub.a: $(stub-obj-y)
> libqemuutil.a: $(util-obj-y)
>
> -block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",)
> NULL
> -util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
> -
> ######################################################################
>
> qemu-img.o: qemu-img-cmds.h
> diff --git a/block.c b/block.c
> index f54bc25..7a91434 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 "qemu/module_block.h"
> #include "qemu/module.h"
> #include "qapi/qmp/qerror.h"
> #include "qapi/qmp/qbool.h"
> @@ -242,11 +243,29 @@ BlockDriverState *bdrv_new(void)
> BlockDriver *bdrv_find_format(const char *format_name)
> {
> BlockDriver *drv1;
> + size_t i;
> +
> QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> if (!strcmp(drv1->format_name, format_name)) {
> return drv1;
> }
> }
> +
> + 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);
> + /* Copying code is not nice, but this way the current discovery
> is
> + * not modified. Calling recursively could fail if the library
> + * has been deleted.
> + */
> + QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> + if (!strcmp(drv1->format_name, format_name)) {
> + return drv1;
> + }
> + }
> + }
> + }
> +
> return NULL;
> }
>
> @@ -447,8 +466,15 @@ int get_tmp_filename(char *filename, int size)
> static BlockDriver *find_hdev_driver(const char *filename)
> {
> int score_max = 0, score;
> + size_t i;
> BlockDriver *drv = NULL, *d;
>
> + for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
> + if (block_driver_modules[i].has_probe_device) {
> + block_module_load_one(block_driver_modules[i].library_name);
> + }
> + }
> +
> QLIST_FOREACH(d, &bdrv_drivers, list) {
> if (d->bdrv_probe_device) {
> score = d->bdrv_probe_device(filename);
> @@ -470,6 +496,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
> char protocol[128];
> int len;
> const char *p;
> + size_t i;
>
> /* TODO Drivers without bdrv_file_open must be specified explicitly */
>
> @@ -496,6 +523,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
> len = sizeof(protocol) - 1;
> memcpy(protocol, filename, len);
> protocol[len] = '\0';
> +
> QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> if (drv1->protocol_name &&
> !strcmp(drv1->protocol_name, protocol)) {
> @@ -503,6 +531,23 @@ BlockDriver *bdrv_find_protocol(const char *filename,
> }
> }
>
> + for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
> + if (block_driver_modules[i].protocol_name &&
> + !strcmp(block_driver_modules[i].protocol_name, protocol)) {
> + block_module_load_one(block_driver_modules[i].library_name);
> + /* Copying code is not nice, but this way the current discovery
> is
> + * not modified. Calling recursively could fail if the library
> + * has been deleted.
> + */
> + QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> + if (drv1->protocol_name &&
> + !strcmp(drv1->protocol_name, protocol)) {
> + return drv1;
> + }
> + }
> + }
> + }
> +
> error_setg(errp, "Unknown protocol '%s'", protocol);
> return NULL;
> }
> @@ -525,8 +570,15 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int
> buf_size,
> const char *filename)
> {
> int score_max = 0, score;
> + 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);
> @@ -2738,26 +2790,42 @@ static int qsort_strcmp(const void *a, const void *b)
> return strcmp(a, b);
> }
>
> +static const char **add_format(const char **formats, int *count,
> + const char *format_name)
> +{
> + int i;
> +
> + for (i = 0; i < *count; i++) {
> + if (!strcmp(formats[i], format_name)) {
> + return formats;
> + }
> + }
> +
> + *count += 1;
> + formats = g_renew(const char *, formats, *count);
> + formats[*count] = format_name;
> + return formats;
> +}
> +
> void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
> void *opaque)
> {
> BlockDriver *drv;
> int count = 0;
> int i;
> + size_t n;
> const char **formats = NULL;
>
> QLIST_FOREACH(drv, &bdrv_drivers, list) {
> if (drv->format_name) {
> - bool found = false;
> - int i = count;
> - while (formats && i && !found) {
> - found = !strcmp(formats[--i], drv->format_name);
> - }
> + formats = add_format(formats, &count, drv->format_name);
> + }
> + }
>
> - if (!found) {
> - formats = g_renew(const char *, formats, count + 1);
> - formats[count++] = drv->format_name;
> - }
> + for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) {
> + if (block_driver_modules[n].format_name) {
> + formats = add_format(formats, &count,
> + block_driver_modules[n].format_name);
> }
> }
>
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 2370708..4729858 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -52,9 +52,12 @@ typedef enum {
> #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
> #define type_init(function) module_init(function, MODULE_INIT_QOM)
>
> +#define block_module_load_one(lib) module_load_one("block-", lib);
Superfluous ending semi-colon?
> +
> void register_module_init(void (*fn)(void), module_init_type type);
> void register_dso_module_init(void (*fn)(void), module_init_type type);
>
> void module_call_init(module_init_type type);
> +void module_load_one(const char *prefix, const char *lib_name);
>
> #endif
> diff --git a/util/module.c b/util/module.c
> index ce058ae..dbc6e52 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -91,14 +91,11 @@ void register_dso_module_init(void (*fn)(void),
> module_init_type type)
> QTAILQ_INSERT_TAIL(&dso_init_list, e, node);
> }
>
> -static void module_load(module_init_type type);
> -
> void module_call_init(module_init_type type)
> {
> ModuleTypeList *l;
> ModuleEntry *e;
>
> - module_load(type);
> l = find_type(type);
>
> QTAILQ_FOREACH(e, l, node) {
> @@ -163,14 +160,10 @@ out:
> }
> #endif
>
> -static void module_load(module_init_type type)
> +void module_load_one(const char *prefix, const char *lib_name)
> {
> #ifdef CONFIG_MODULES
> char *fname = NULL;
> - const char **mp;
> - static const char *block_modules[] = {
> - CONFIG_BLOCK_MODULES
> - };
> char *exec_dir;
> char *dirs[3];
> int i = 0;
> @@ -181,15 +174,6 @@ static void module_load(module_init_type type)
> return;
> }
>
> - switch (type) {
> - case MODULE_INIT_BLOCK:
> - mp = block_modules;
> - break;
> - default:
> - /* no other types have dynamic modules for now*/
> - return;
> - }
> -
> exec_dir = qemu_get_exec_dir();
> dirs[i++] = g_strdup_printf("%s", CONFIG_QEMU_MODDIR);
> dirs[i++] = g_strdup_printf("%s/..", exec_dir ? : "");
> @@ -198,16 +182,15 @@ static void module_load(module_init_type type)
> g_free(exec_dir);
> exec_dir = NULL;
>
> - for ( ; *mp; mp++) {
> - for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> - fname = g_strdup_printf("%s/%s%s", dirs[i], *mp, HOST_DSOSUF);
> - ret = module_load_file(fname);
> - g_free(fname);
> - fname = NULL;
> - /* Try loading until loaded a module file */
> - if (!ret) {
> - break;
> - }
> + for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> + fname = g_strdup_printf("%s/%s%s%s",
> + dirs[i], prefix, lib_name, HOST_DSOSUF);
> + ret = module_load_file(fname);
> + g_free(fname);
> + fname = NULL;
> + /* Try loading until loaded a module file */
> + if (!ret) {
> + break;
> }
> }
>
> --
> 2.5.5
>
>