On Thu, Sep 10, 2020 at 09:56:11PM +0200, mwilck@xxxxxxxx wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> setup_map() is called both for new maps (e.g. from coalesce_paths())
> and existing maps (e.g. from reload_map(), resize_map()). In the former
> case, the map will be removed from global data structures, so incomplete
> initialization is not an issue. But In the latter case, removal isn't
> generally possible. We expect that mpp->features, mpp->hwhandler,
> mpp->selector have been initialized and are are non-NULL. We must make sure
> not to break this assumption because of an error in this setup_map()
> invocation. As these properties aren't likely to change during an update
> operation, saving and restoring them is better than leaving the map
> improperly initialized.
>
Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
>
> v1->v2: forgot to remove the call to free_multipath_attributes().
>
> This is supposed to be applied on top of lixiaokeng's patch
> "libmultipath: check whether mpp->features is NUll in setup_map".
>
> libmultipath/configure.c | 29 +++++++++++++++++++++++++----
> libmultipath/util.h | 7 +++++++
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 5d5d941..8fd12df 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -298,6 +298,7 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
> struct pathgroup * pgp;
> struct config *conf;
> int i, n_paths, marginal_pathgroups;
> + char *save_attr;
>
> /*
> * don't bother if devmap size is unknown
> @@ -307,10 +308,6 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
> return 1;
> }
>
> - /*
> - * free features, selector, and hwhandler properties if they are being reused
> - */
> - free_multipath_attributes(mpp);
> if (mpp->disable_queueing && VECTOR_SIZE(mpp->paths) != 0)
> mpp->disable_queueing = 0;
>
> @@ -328,11 +325,35 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
>
> select_pgfailback(conf, mpp);
> select_pgpolicy(conf, mpp);
> +
> + /*
> + * If setup_map() is called from e.g. from reload_map() or resize_map(),
> + * make sure that we don't corrupt attributes.
> + */
> + save_attr = steal_ptr(mpp->selector);
> select_selector(conf, mpp);
> + if (!mpp->selector)
> + mpp->selector = save_attr;
> + else
> + free(save_attr);
> +
> select_no_path_retry(conf, mpp);
> select_retain_hwhandler(conf, mpp);
> +
> + save_attr = steal_ptr(mpp->features);
> select_features(conf, mpp);
> + if (!mpp->features)
> + mpp->features = save_attr;
> + else
> + free(save_attr);
> +
> + save_attr = steal_ptr(mpp->hwhandler);
> select_hwhandler(conf, mpp);
> + if (!mpp->hwhandler)
> + mpp->hwhandler = save_attr;
> + else
> + free(save_attr);
> +
> select_rr_weight(conf, mpp);
> select_minio(conf, mpp);
> select_mode(conf, mpp);
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index 52aa559..045bbee 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> @@ -110,4 +110,11 @@ static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
> bf->bits[bit / bits_per_slot] &= ~(1ULL << (bit % bits_per_slot));
> }
>
> +#define steal_ptr(x) \
> + ({ \
> + void *___p = x; \
> + x = NULL; \
> + ___p; \
> + })
> +
> #endif /* _UTIL_H */
> --
> 2.28.0
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
