From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33856) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLV7K-0000Jj-9P for qemu-devel@nongnu.org; Mon, 16 Sep 2013 05:29:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLV7E-0006nr-71 for qemu-devel@nongnu.org; Mon, 16 Sep 2013 05:29:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36324) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLV7D-0006nm-Ve for qemu-devel@nongnu.org; Mon, 16 Sep 2013 05:29:08 -0400 Date: Mon, 16 Sep 2013 17:28:52 +0800 From: Fam Zheng Message-ID: <20130916092852.GA25678@T430s.nay.redhat.com> References: <1379314227-8855-1-git-send-email-famz@redhat.com> <1379314227-8855-6-git-send-email-famz@redhat.com> <20130916085902.GA6005@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130916085902.GA6005@redhat.com> Subject: Re: [Qemu-devel] [PATCH v10 5/8] module: implement module loading Reply-To: famz@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: peter.maydell@linaro.org, mjt@tls.msk.ru, qemu-devel@nongnu.org, alex@alex.org.uk, pbonzini@redhat.com, vilanova@ac.upc.edu, rth@twiddle.net On Mon, 09/16 09:59, Daniel P. Berrange wrote: > On Mon, Sep 16, 2013 at 02:50:24PM +0800, Fam Zheng wrote: > > Added three types of modules: > > > > typedef enum { > > MODULE_LOAD_BLOCK = 0, > > MODULE_LOAD_UI, > > MODULE_LOAD_NET, > > MODULE_LOAD_MAX, > > } module_load_type; > > > > and their loading function: > > > > void module_load(module_load_type). > > > > which loads whitelisted ".so" files of the given type under ${MODDIR}. > > > > Modules of each type should be loaded in respective subsystem > > initialization code. > > > > The init function of dynamic module is no longer with > > __attribute__((constructor)) as static linked version, and need to be > > explicitly called once loaded. The function name is mangled with per > > configure fingerprint as: > > > > init_$(date +%s$$$RANDOM) > > > > Which is known to module_load function, and the loading fails if this > > symbol is not there. With this, modules built from a different > > tree/version/configure will not be loaded. > > > > The module loading code requires gmodule-2.0. > > > > Configure option "--enable-modules=L" can be used to restrict qemu to > > only build/load some whitelisted modules. > > > > Signed-off-by: Fam Zheng > > --- > > Makefile | 3 ++ > > block.c | 1 + > > configure | 44 +++++++++++++++++++++------- > > include/qemu/module.h | 23 +++++++++++++++ > > rules.mak | 9 ++++-- > > scripts/create_config | 22 ++++++++++++++ > > util/module.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > vl.c | 2 ++ > > 8 files changed, 172 insertions(+), 13 deletions(-) > > > > +void module_load(module_load_type type) > > +{ > > +#ifdef CONFIG_MODULES > > + const char *prefix; > > + char *fname = NULL; > > + const char **mp; > > + static const char *module_whitelist[] = { > > + CONFIG_MODULE_WHITELIST > > + }; > > + > > + if (!g_module_supported()) { > > + return; > > + } > > + > > + switch (type) { > > + case MODULE_LOAD_BLOCK: > > + prefix = "block-"; > > + break; > > + case MODULE_LOAD_UI: > > + prefix = "ui-"; > > + break; > > + case MODULE_LOAD_NET: > > + prefix = "ui-"; > > Wrong prefix. > > > + break; > > + default: > > + return; > > + } > > + > > + for (mp = &module_whitelist[0]; *mp; mp++) { > > + if (strncmp(prefix, *mp, strlen(prefix))) { > > + continue; > > + } > > + fname = g_strdup_printf("%s/%s%s", CONFIG_MODDIR, *mp, HOST_DSOSUF); > > + module_load_file(fname); > > + g_free(fname); > > + } > > + > > +#endif > > +} > > IMHO this method design is really crazy. If you want to have a > situation where you call module_load() multiple times, then > have separate whitelists for block/net/ui, instead of having > one global whitelist which you then have to load a subset of > each time. Alternatively just call module_load() once and > give rid of these enums for loading block/net/ui separately. > It's pretty clear that we have different subsets of modules to load for target emulator and qemu-img, so not having enums is not working. What's the disadvantage of this, and why are separate lists better? Thanks, Fam