From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Marzinski Subject: Re: [PATCH v2] libmultipath: setup_map(): don't break multipath attributes Date: Tue, 15 Sep 2020 15:54:38 -0500 Message-ID: <20200915205438.GR11108@octiron.msp.redhat.com> References: <20200910195611.8439-1-mwilck@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20200910195611.8439-1-mwilck@suse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com Content-Disposition: inline To: mwilck@suse.com Cc: lixiaokeng@huawei.com, dm-devel@redhat.com, Zhiqiang Liu , linfeilong@huawei.com List-Id: dm-devel.ids On Thu, Sep 10, 2020 at 09:56:11PM +0200, mwilck@suse.com wrote: > From: Martin Wilck > > 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 > Signed-off-by: Martin Wilck > --- > > 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