From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze1Xu-0007rb-4j for qemu-devel@nongnu.org; Mon, 21 Sep 2015 09:54:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ze1Xp-0008LE-6I for qemu-devel@nongnu.org; Mon, 21 Sep 2015 09:54:18 -0400 Received: from mx2.parallels.com ([199.115.105.18]:42926) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze1Xo-0008Ks-Tx for qemu-devel@nongnu.org; Mon, 21 Sep 2015 09:54:13 -0400 References: <1441720402-1778-1-git-send-email-markmb@redhat.com> <1441720402-1778-2-git-send-email-markmb@redhat.com> From: "Denis V. Lunev" Message-ID: <56000AD2.9060306@parallels.com> Date: Mon, 21 Sep 2015 16:49:06 +0300 MIME-Version: 1.0 In-Reply-To: <1441720402-1778-2-git-send-email-markmb@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 1/2] Add dynamic module loading for block drivers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc_Mar=c3=ad?= , qemu-devel@nongnu.org Cc: Peter Maydell , Michael Tokarev , Fam Zheng , Stefan Hajnoczi On 09/08/2015 04:53 PM, Marc Marí wrote: > 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. This > is why libiscsi has been disabled as a module. > > All the necessary module information is located in a new structure found in > include/qemu/module_block.h > > Signed-off-by: Marc Marí > --- > block.c | 70 +++++++++++++++++++++++++++++++++++ > configure | 2 +- > include/qemu/module.h | 3 ++ > include/qemu/module_block.h | 90 +++++++++++++++++++++++++++++++++++++++++++++ > util/module.c | 38 ++++++------------- > 5 files changed, 175 insertions(+), 28 deletions(-) > create mode 100644 include/qemu/module_block.h > > diff --git a/block.c b/block.c > index 090923c..814429a 100644 > --- a/block.c > +++ b/block.c > @@ -27,6 +27,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/qjson.h" > @@ -277,11 +278,30 @@ void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify) > 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; > } > > @@ -484,8 +504,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); > @@ -507,6 +534,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 */ > > @@ -533,6 +561,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)) { > @@ -540,6 +569,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; > } > @@ -562,8 +608,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); > @@ -2784,6 +2837,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name), > BlockDriver *drv; > int count = 0; > int i; > + size_t n; > const char **formats = NULL; > > QLIST_FOREACH(drv, &bdrv_drivers, list) { > @@ -2801,6 +2855,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name), > } > } > > + for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) { > + if (block_driver_modules[n].format_name) { > + bool found = false; > + int i = count; > + while (formats && i && !found) { > + found = !strcmp(formats[--i], > + block_driver_modules[n].format_name); > + } > + > + if (!found) { > + formats = g_renew(const char *, formats, count + 1); > + formats[count++] = block_driver_modules[n].format_name; > + } > + } > + } > + > qsort(formats, count, sizeof(formats[0]), qsort_strcmp); > > for (i = 0; i < count; i++) { > diff --git a/configure b/configure > index d854936..72ac564 100755 > --- a/configure > +++ b/configure > @@ -4984,7 +4984,7 @@ if test "$bzip2" = "yes" ; then > fi > > if test "$libiscsi" = "yes" ; then > - echo "CONFIG_LIBISCSI=m" >> $config_host_mak > + echo "CONFIG_LIBISCSI=y" >> $config_host_mak > echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak > echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak > fi > diff --git a/include/qemu/module.h b/include/qemu/module.h > index 72d9498..0ad4bb7 100644 > --- a/include/qemu/module.h > +++ b/include/qemu/module.h > @@ -53,9 +53,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); > + > 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/include/qemu/module_block.h b/include/qemu/module_block.h > new file mode 100644 > index 0000000..d725db8 > --- /dev/null > +++ b/include/qemu/module_block.h > @@ -0,0 +1,90 @@ > +/* AUTOMATICALLY GENERATED, DO NOT MODIFY */ > +/* > + * QEMU Block Module Infrastructure > + * > + * Copyright Red Hat, Inc. 2015 > + * > + * Authors: > + * Marc Mari > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef QEMU_MODULE_BLOCK_H > +#define QEMU_MODULE_BLOCK_H > + > +#include "qemu-common.h" > + > +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[] = { > + { > + .library_name = "curl", > + .format_name = "http", > + .protocol_name = "http", > + }, > + { > + .library_name = "curl", > + .format_name = "https", > + .protocol_name = "https", > + }, > + { > + .library_name = "curl", > + .format_name = "ftp", > + .protocol_name = "ftp", > + }, > + { > + .library_name = "curl", > + .format_name = "ftps", > + .protocol_name = "ftps", > + }, > + { > + .library_name = "curl", > + .format_name = "tftp", > + .protocol_name = "tftp", > + }, > + { > + .library_name = "rbd", > + .format_name = "rbd", > + .protocol_name = "rbd", > + }, > + { > + .library_name = "gluster", > + .format_name = "gluster", > + .protocol_name = "gluster", > + }, > + { > + .library_name = "gluster", > + .format_name = "gluster", > + .protocol_name = "gluster+tcp", > + }, > + { > + .library_name = "gluster", > + .format_name = "gluster", > + .protocol_name = "gluster+unix", > + }, > + { > + .library_name = "gluster", > + .format_name = "gluster", > + .protocol_name = "gluster+rdma", > + }, > + { > + .library_name = "ssh", > + .format_name = "ssh", > + .protocol_name = "ssh", > + }, > + { > + .library_name = "dmg", > + .format_name = "dmg", > + .has_probe = true, > + }, > +}; > + > +#endif > + > diff --git a/util/module.c b/util/module.c > index 4bd4a94..992d317 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) { > @@ -149,6 +146,7 @@ static int module_load_file(const char *fname) > ret = -EINVAL; > } else { > QTAILQ_FOREACH(e, &dso_init_list, node) { > + e->init(); > register_module_init(e->init, e->type); > } > ret = 0; > @@ -163,14 +161,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 +175,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 +183,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; > } > } > I think that the API will be more convenient if block_module_load_one will return just loaded driver. Second loop will not be needed in this case. Den