Re: [Qemu-devel] [PATCH for-2.8 v3 2/3] module: Don't load the same module if requested multiple times

[Fam Zheng <famz@redhat.com>]

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 <famz@redhat.com>
> > ---
> >  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