From: Benjamin Marzinski <bmarzins@redhat.com>
To: mwilck@suse.com
Cc: dm-devel@redhat.com
Subject: Re: [PATCH v2 13/21] libmultipath: provide defaults for {get, put}_multipath_config
Date: Thu, 24 Sep 2020 23:34:58 -0500 [thread overview]
Message-ID: <20200925043458.GO11108@octiron.msp.redhat.com> (raw)
In-Reply-To: <20200924133716.14120-14-mwilck@suse.com>
On Thu, Sep 24, 2020 at 03:37:08PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> Add an implementation of get_multipath_config() and put_multipath_config()
> to libmultipath. The linker's symbol lookup rules will make sure that
> applications can override these functions if they need to. Defining
> these functions in libmultipath avoids the need to provide stubs
> for these functions in every appliation linking to libmultipath.
>
> libmultipath's internal functions simply refer to a static "struct config".
> It must be initialized with init_config() rather than load_config(),
> which always allocates a new struct for backward compatibility, and must
> be teared down using uninit_config().
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> libmultipath/config.c | 75 ++++++++++++++++++++++++++-----
> libmultipath/config.h | 21 +++++++--
> libmultipath/libmultipath.version | 10 +++++
> 3 files changed, 93 insertions(+), 13 deletions(-)
>
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 5c91a09..01b77df 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -27,6 +27,26 @@
> #include "mpath_cmd.h"
> #include "propsel.h"
>
> +static struct config __internal_config;
> +struct config *libmp_get_multipath_config(void)
This causes problems with the libmpathvalid library code I wrote. The
issue is that right now, when you run _init_config() if
get_multipath_config() returns NULL, you will use the default loglevel.
I would like the library user to have control of the log level, even
during the calls to _init_config().
One possiblity would be to make init_config() take verbosity as an
argument. There would also need to be some other config variable that
gets set at the start of init_config(), which is used by
libmp_get_multipath_config() to check if it is initialized.
-Ben
> +{
> + if (!__internal_config.hwtable)
> + /* not initialized */
> + return NULL;
> + return &__internal_config;
> +}
> +
> +struct config *get_multipath_config(void)
> + __attribute__((weak, alias("libmp_get_multipath_config")));
> +
> +void libmp_put_multipath_config(void *conf __attribute__((unused)))
> +{
> + /* empty */
> +}
> +
> +void put_multipath_config(void *conf)
> + __attribute__((weak, alias("libmp_put_multipath_config")));
> +
> static int
> hwe_strmatch (const struct hwentry *hwe1, const struct hwentry *hwe2)
> {
> @@ -574,17 +594,15 @@ restart:
> return;
> }
>
> -struct config *
> -alloc_config (void)
> +static struct config *alloc_config (void)
> {
> return (struct config *)MALLOC(sizeof(struct config));
> }
>
> -void
> -free_config (struct config * conf)
> +static void _uninit_config(struct config *conf)
> {
> if (!conf)
> - return;
> + conf = &__internal_config;
>
> if (conf->multipath_dir)
> FREE(conf->multipath_dir);
> @@ -650,7 +668,27 @@ free_config (struct config * conf)
> free_hwtable(conf->hwtable);
> free_hwe(conf->overrides);
> free_keywords(conf->keywords);
> - FREE(conf);
> +
> + memset(conf, 0, sizeof(*conf));
> +}
> +
> +void uninit_config(void)
I didn't realize that this was o.k. I thought you had to specify static
in the function definition, even if it was specified in the protype.
> +{
> + _uninit_config(&__internal_config);
> +}
> +
> +void free_config(struct config *conf)
> +{
> + if (!conf)
> + return;
> + else if (conf == &__internal_config) {
> + condlog(0, "ERROR: %s called for internal config. Use uninit_config() instead",
> + __func__);
> + return;
> + }
> +
> + _uninit_config(conf);
> + free(conf);
> }
>
> /* if multipath fails to process the config directory, it should continue,
> @@ -719,12 +757,29 @@ static void set_max_checkint_from_watchdog(struct config *conf)
> }
> #endif
>
> +static int _init_config (const char *file, struct config *conf);
> +
> +int init_config(const char *file)
> +{
> + return _init_config(file, &__internal_config);
> +}
> +
> struct config *load_config(const char *file)
> {
> struct config *conf = alloc_config();
>
> + if (conf && !_init_config(file, conf))
> + return conf;
> +
> + free(conf);
> + return NULL;
> +}
> +
> +int _init_config (const char *file, struct config *conf)
> +{
> +
> if (!conf)
> - return NULL;
> + conf = &__internal_config;
>
> /*
> * internal defaults
> @@ -896,10 +951,10 @@ struct config *load_config(const char *file)
> !conf->wwids_file || !conf->prkeys_file)
> goto out;
>
> - return conf;
> + return 0;
> out:
> - free_config(conf);
> - return NULL;
> + _uninit_config(conf);
> + return 1;
> }
>
> char *get_uid_attribute_by_attrs(struct config *conf,
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index 116fe37..5997b71 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -251,10 +251,25 @@ void free_mptable (vector mptable);
> int store_hwe (vector hwtable, struct hwentry *);
>
> struct config *load_config (const char *file);
> -struct config * alloc_config (void);
> void free_config (struct config * conf);
> -extern struct config *get_multipath_config(void);
> -extern void put_multipath_config(void *);
> +int init_config(const char *file);
> +void uninit_config(void);
> +
> +/*
> + * libmultipath provides default implementations of
> + * get_multipath_config() and put_multipath_config().
> + * Applications using these should use init_config(file, NULL)
> + * to load the configuration, rather than load_config(file).
> + * Likewise, uninit_config() should be used for teardown, but
> + * using free_config() for that is supported, too.
> + * Applications can define their own {get,put}_multipath_config()
> + * functions, which override the library-internal ones, but
> + * could still call libmp_{get,put}_multipath_config().
> + */
> +struct config *libmp_get_multipath_config(void);
> +struct config *get_multipath_config(void);
> +void libmp_put_multipath_config(void *);
> +void put_multipath_config(void *);
>
> int parse_uid_attrs(char *uid_attrs, struct config *conf);
> char *get_uid_attribute_by_attrs(struct config *conf,
> diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
> index 0a96010..81bcc9d 100644
> --- a/libmultipath/libmultipath.version
> +++ b/libmultipath/libmultipath.version
> @@ -218,3 +218,13 @@ global:
> libmp_dm_task_run;
> cleanup_mutex;
> } LIBMULTIPATH_0.8.4.1;
> +
> +LIBMULTIPATH_0.8.4.3 {
> +global:
> + libmp_get_multipath_config;
> + get_multipath_config;
> + libmp_put_multipath_config;
> + put_multipath_config;
> + init_config;
> + uninit_config;
> +} LIBMULTIPATH_0.8.4.2;
> --
> 2.28.0
next prev parent reply other threads:[~2020-09-25 4:34 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-24 13:36 [PATCH v2 00/21] multipath-tools: shutdown, libdevmapper races, globals mwilck
2020-09-24 13:36 ` [PATCH v2 01/21] multipathd: allow shutdown during configure() mwilck
2020-09-24 13:36 ` [PATCH v2 02/21] multipathd: avoid sending "READY=1" to systemd on early shutdown mwilck
2020-09-24 13:36 ` [PATCH v2 03/21] multipathd: send "STOPPING=1" to systemd on shutdown mwilck
2020-09-24 13:36 ` [PATCH v2 04/21] multipathd: send "RELOADING=1" to systemd on DAEMON_CONFIGURE state mwilck
2020-09-24 13:37 ` [PATCH v2 05/21] multipathd: use volatile qualifier for running_state mwilck
2020-09-24 13:37 ` [PATCH v2 06/21] multipathd: generalize and fix wait_for_state_change_if() mwilck
2020-09-24 13:37 ` [PATCH v2 07/21] multipathd: set_config_state(): avoid code duplication mwilck
2020-09-24 13:37 ` [PATCH v2 08/21] multipathd: cancel threads early during shutdown mwilck
2020-09-24 13:37 ` [PATCH v2 09/21] multipath-tools: don't call dm_lib_release() any more mwilck
2020-09-24 13:37 ` [PATCH v2 10/21] libmultipath: devmapper: refactor libdm version determination mwilck
2020-09-25 23:48 ` Benjamin Marzinski
2020-09-24 13:37 ` [PATCH v2 11/21] libmultipath: protect racy libdevmapper calls with a mutex mwilck
2020-09-29 5:57 ` Benjamin Marzinski
2020-09-29 9:09 ` Martin Wilck
2020-09-24 13:37 ` [PATCH v2 12/21] libmultipath: constify file argument in config parser mwilck
2020-09-24 13:37 ` [PATCH v2 13/21] libmultipath: provide defaults for {get, put}_multipath_config mwilck
2020-09-25 4:34 ` Benjamin Marzinski [this message]
2020-09-25 20:00 ` Martin Wilck
2020-09-25 21:57 ` Benjamin Marzinski
2020-09-26 9:43 ` Martin Wilck
2020-09-27 1:03 ` Benjamin Marzinski
2020-09-24 13:37 ` [PATCH v2 14/21] libmpathpersist: allow using libmultipath " mwilck
2020-09-24 13:37 ` [PATCH v2 15/21] multipath: use {get_put}_multipath_config from libmultipath mwilck
2020-09-24 13:37 ` [PATCH v2 16/21] mpathpersist: use {get, put}_multipath_config() " mwilck
2020-09-24 13:37 ` [PATCH v2 17/21] libmultipath: add udev and logsink symbols mwilck
2020-09-25 23:03 ` Benjamin Marzinski
2020-09-25 23:05 ` Martin Wilck
2020-09-24 13:37 ` [PATCH v2 18/21] multipath: remove logsink and udev mwilck
2020-09-24 13:37 ` [PATCH v2 19/21] libmpathpersist: call libmultipath_{init, exit}() mwilck
2020-09-25 23:55 ` Benjamin Marzinski
2020-09-24 13:37 ` [PATCH v2 20/21] mpathpersist: remove logsink and udev mwilck
2020-09-25 23:56 ` Benjamin Marzinski
2020-09-24 13:37 ` [PATCH v2 21/21] multipathd: " mwilck
2020-09-26 0:19 ` Benjamin Marzinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200925043458.GO11108@octiron.msp.redhat.com \
--to=bmarzins@redhat.com \
--cc=dm-devel@redhat.com \
--cc=mwilck@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).