> On Thu, Mar 31, 2022 at 12:15:02AM +0200, mwilck@xxxxxxxx wrote:
> uevent merging by WWID relies on the uid_attrs config option. As we
> drop struct config between calls to uevent_merge(), we must be sure
> that the WWID is not changed in reconfigure().
>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> multipath/multipath.conf.5 | 2 ++
> multipathd/main.c | 53 +++++++++++++++++++++++++++++++-------
> 2 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index 605b46e..a9cd776 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -264,6 +264,8 @@ If this option is configured and matches the device
> node name of a device, it overrides any other configured methods for
> determining the WWID for this device.
> .PP
> +This option cannot be changed during runtime with the multipathd \fBreconfigure\fR command.
> +.PP
> The default is: \fB<unset>\fR. To enable uevent merging, set it e.g. to
> \(dqsd:ID_SERIAL dasd:ID_UID nvme:ID_WWN\(dq.
> .RE
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 13b1948..f514b32 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2835,11 +2835,52 @@ void rcu_free_config(struct rcu_head *head)
> free_config(conf);
> }
>
> +static bool reconfigure_check_uid_attrs(const struct _vector *old_attrs,
> + const struct _vector *new_attrs)
> +{
> + int i;
> + char *old;
> +
> + if (VECTOR_SIZE(old_attrs) != VECTOR_SIZE(new_attrs))
> + return true;
> +
> + vector_foreach_slot(old_attrs, old, i) {
> + char *new = VECTOR_SLOT(new_attrs, i);
> +
> + if (strcmp(old, new))
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void reconfigure_check(struct config *old, struct config *new)
> +{
> + int old_marginal_pathgroups;
> +
> + old_marginal_pathgroups = old->marginal_pathgroups;
> + if ((old_marginal_pathgroups == MARGINAL_PATHGROUP_FPIN) !=
> + (new->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)) {
> + condlog(1, "multipathd must be restarted to turn %s fpin marginal paths",
> + (old_marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)?
> + "off" : "on");
> + new->marginal_pathgroups = old_marginal_pathgroups;
> + }
> +
> + if (reconfigure_check_uid_attrs(&old->uid_attrs, &new->uid_attrs)) {
> + struct _vector v = new->uid_attrs;
> +
> + condlog(1, "multipathd must be restarted to change uid_attrs, keeping old values");
> + new->uid_attrs = old->uid_attrs;
> + vector_reset(&v);
This leaks the strings that v points to, right? This also can happen in
uid_attrs_handler(), I just noticed.
-Ben
> + old->uid_attrs = v;
> + }
> +}
> +
> static int
> reconfigure (struct vectors *vecs, enum force_reload_types reload_type)
> {
> struct config * old, *conf;
> - int old_marginal_pathgroups;
>
> conf = load_config(DEFAULT_CONFIGFILE);
> if (!conf)
> @@ -2870,14 +2911,8 @@ reconfigure (struct vectors *vecs, enum force_reload_types reload_type)
> uxsock_timeout = conf->uxsock_timeout;
>
> old = rcu_dereference(multipath_conf);
> - old_marginal_pathgroups = old->marginal_pathgroups;
> - if ((old_marginal_pathgroups == MARGINAL_PATHGROUP_FPIN) !=
> - (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)) {
> - condlog(1, "multipathd must be restarted to turn %s fpin marginal paths",
> - (old_marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)?
> - "off" : "on");
> - conf->marginal_pathgroups = old_marginal_pathgroups;
> - }
> + reconfigure_check(old, conf);
> +
> conf->sequence_nr = old->sequence_nr + 1;
> rcu_assign_pointer(multipath_conf, conf);
> call_rcu(&old->rcu, rcu_free_config);
> --
> 2.35.1
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
