From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39361) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bgjk2-0005cJ-Th for qemu-devel@nongnu.org; Sun, 04 Sep 2016 22:34:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bgjk0-0003JP-R2 for qemu-devel@nongnu.org; Sun, 04 Sep 2016 22:34:33 -0400 Date: Mon, 5 Sep 2016 10:27:13 +0800 From: Fam Zheng Message-ID: <20160905022713.GA6024@al.usersys.redhat.com> References: <1471418433-20061-1-git-send-email-famz@redhat.com> <1471418433-20061-3-git-send-email-famz@redhat.com> <5a41b27e-6a76-d87e-5ff2-4528f132aa42@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5a41b27e-6a76-d87e-5ff2-4528f132aa42@redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.8 v3 2/3] module: Don't load the same module if requested multiple times List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, Stefan Hajnoczi , Kevin Wolf , qemu-block@nongnu.org On Mon, 08/29 15:30, Max Reitz wrote: > On 17.08.2016 09:20, Fam Zheng wrote: > > Use a hash table to keep record of all loaded modules, and return early > > if the requested module is already loaded. > > > > Signed-off-by: Fam Zheng > > --- > > util/module.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/util/module.c b/util/module.c > > index a5f7fbd..63efad6 100644 > > --- a/util/module.c > > +++ b/util/module.c > > @@ -163,14 +163,29 @@ void module_load_one(const char *prefix, const char *lib_name) > > char *fname = NULL; > > char *exec_dir; > > char *dirs[3]; > > + char *module_name; > > int i = 0; > > int ret; > > + static GHashTable *loaded_modules; > > > > if (!g_module_supported()) { > > fprintf(stderr, "Module is not supported by system.\n"); > > return; > > } > > > > + if (!loaded_modules) { > > + loaded_modules = g_hash_table_new(g_str_hash, g_str_equal); > > + } > > + > > + module_name = g_strdup_printf("%s%s", prefix, lib_name); > > + > > + if (g_hash_table_lookup(loaded_modules, module_name)) { > > + fprintf(stderr, "module is already loaded: %s\n", module_name); > > I'm not quite happy with this warning message. Loading a module is > automatically initiated by internal code in qemu, i.e. never done by the > user. Therefore, printing a message for the user does not make much > sense to me since the user cannot do anything about this. > > If it is truly wrong to attempt to load a module more than once, this > should be an assertion. > > However, I think it's perfectly fine to just allow qemu code to try to > load a module more than once and just ignore the request if we've > already loaded the module (as the commit message implies). In this case, > we don't need an error message or warning, though. > Agreed. I'll remove the warning. Thanks! Fam