From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1beMek-0006sA-6Z for qemu-devel@nongnu.org; Mon, 29 Aug 2016 09:31:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1beMed-0003PJ-VV for qemu-devel@nongnu.org; Mon, 29 Aug 2016 09:31:17 -0400 References: <1471418433-20061-1-git-send-email-famz@redhat.com> <1471418433-20061-3-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: <5a41b27e-6a76-d87e-5ff2-4528f132aa42@redhat.com> Date: Mon, 29 Aug 2016 15:30:55 +0200 MIME-Version: 1.0 In-Reply-To: <1471418433-20061-3-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9310O6x1DRw0Xnwgh1wXw00DbIxDeJ0lu" 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: Fam Zheng , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, Stefan Hajnoczi , Kevin Wolf , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9310O6x1DRw0Xnwgh1wXw00DbIxDeJ0lu From: Max Reitz To: Fam Zheng , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, Stefan Hajnoczi , Kevin Wolf , qemu-block@nongnu.org Message-ID: <5a41b27e-6a76-d87e-5ff2-4528f132aa42@redhat.com> Subject: Re: [PATCH for-2.8 v3 2/3] module: Don't load the same module if requested multiple times References: <1471418433-20061-1-git-send-email-famz@redhat.com> <1471418433-20061-3-git-send-email-famz@redhat.com> In-Reply-To: <1471418433-20061-3-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Fam Zheng > --- > util/module.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) >=20 > 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 ch= ar *lib_name) > char *fname =3D NULL; > char *exec_dir; > char *dirs[3]; > + char *module_name; > int i =3D 0; > int ret; > + static GHashTable *loaded_modules; > =20 > if (!g_module_supported()) { > fprintf(stderr, "Module is not supported by system.\n"); > return; > } > =20 > + if (!loaded_modules) { > + loaded_modules =3D g_hash_table_new(g_str_hash, g_str_equal); > + } > + > + module_name =3D 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 =3D qemu_get_exec_dir(); > dirs[i++] =3D g_strdup_printf("%s", CONFIG_QEMU_MODDIR); > dirs[i++] =3D g_strdup_printf("%s/..", exec_dir ? : ""); > @@ -180,8 +195,8 @@ void module_load_one(const char *prefix, const char= *lib_name) > exec_dir =3D NULL; > =20 > for (i =3D 0; i < ARRAY_SIZE(dirs); i++) { > - fname =3D g_strdup_printf("%s/%s%s%s", > - dirs[i], prefix, lib_name, HOST_DSOSUF); > + fname =3D g_strdup_printf("%s/%s%s", > + dirs[i], module_name, HOST_DSOSUF); > ret =3D module_load_file(fname); > g_free(fname); > fname =3D NULL; >=20 --9310O6x1DRw0Xnwgh1wXw00DbIxDeJ0lu Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEvBAEBCAAZBQJXxDkPEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQN3u B/9xx1AiD+IswvgXsRt40y7TQR0s09n2akar1A+5zoXtQkHlHnC1BHPq+E5I+ul7 TcUhAtKEiRycFMMTzeV4kCoPbA52Pp/6U6p261Agx+JWBodXHc0CdQ2ZwYtqyq7s AK4OZFgwnsNx1j4U9IxEQNqAHQ0cSe2BOIgPMzoLpCabMHO8Cs5UfrYKB/FQxp06 XfUrIL50ZgFFIG2l7ME8Y0Wlzu8oy4/8jCGTgiZYePtuhMMmgY+8+jT/2ylvgnLj ULgVCCd+B1dHvR9aR5opv+ZKOxGOgowpyvWwERzj/jypvTK4s718//Pfpo5o0nhj Gh7kOdULe3JYk9uKob1Djh12 =JBxX -----END PGP SIGNATURE----- --9310O6x1DRw0Xnwgh1wXw00DbIxDeJ0lu--