qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH for-2.8 v3 2/3] module: Don't load the same modu


From: Max Reitz
Subject: Re: [Qemu-block] [PATCH for-2.8 v3 2/3] module: Don't load the same module if requested multiple times
Date: Mon, 29 Aug 2016 15:30:55 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0

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 <address@hidden>
> ---
>  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.

Max

> +        g_free(module_name);
> +        return;
> +    }
> +    g_hash_table_insert(loaded_modules, module_name, module_name);
> +
>      exec_dir = qemu_get_exec_dir();
>      dirs[i++] = g_strdup_printf("%s", CONFIG_QEMU_MODDIR);
>      dirs[i++] = g_strdup_printf("%s/..", exec_dir ? : "");
> @@ -180,8 +195,8 @@ void module_load_one(const char *prefix, const char 
> *lib_name)
>      exec_dir = NULL;
>  
>      for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> -        fname = g_strdup_printf("%s/%s%s%s",
> -                dirs[i], prefix, lib_name, HOST_DSOSUF);
> +        fname = g_strdup_printf("%s/%s%s",
> +                dirs[i], module_name, HOST_DSOSUF);
>          ret = module_load_file(fname);
>          g_free(fname);
>          fname = NULL;
> 


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]