On Tue, Nov 16, 2021 at 10:01:15PM +0800, lixiaokeng wrote:
> The update_map will frequently be called and there will be
> unnecessary checks of reseravtion. We add prflag to path
> to avoid this.
>
> The pp->state changes from others to up or ghost, the
> mpath_pr_event_handle should be called. The
> mpath_pr_event_handle in ev_add_path may not be called,
> so set pp->prkey PRKEY_NO when path is removed.
This patch kind of confuses me. You only check pp->prkey before calling
mpath_pr_event_handle() in update_map(). I get from your commit message
that you are doing this to keep from frequent, unnecessary calls. But
isn't update_map() only called when a multipath device is first created,
or when multipathd stops waiting for something that it noticed during
device creation? I don't see how this can be frequently called on a
multipath device. What am I missing?
-Ben
>
> Fix: 4db4fa
> Signed-off-by: Lixiaokeng <lixiaokeng>
> ---
> libmpathpersist/mpath_persist.c | 2 +-
> libmultipath/structs.c | 1 +
> libmultipath/structs.h | 12 ++++++++++++
> multipathd/cli_handlers.c | 15 ++++++++++-----
> multipathd/main.c | 5 +++--
> 5 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
> index 803a2a28..f88a2e89 100644
> --- a/libmpathpersist/mpath_persist.c
> +++ b/libmpathpersist/mpath_persist.c
> @@ -924,7 +924,7 @@ int update_map_pr(struct multipath *mpp)
>
> if (isFound)
> {
> - mpp->prflag = 1;
> + mpp->prflag = PRFLAG_OK;
> condlog(2, "%s: prflag flag set.", mpp->alias );
> }
>
> diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> index e8cacb4b..82dbd565 100644
> --- a/libmultipath/structs.c
> +++ b/libmultipath/structs.c
> @@ -122,6 +122,7 @@ uninitialize_path(struct path *pp)
> pp->dmstate = PSTATE_UNDEF;
> pp->uid_attribute = NULL;
> pp->getuid = NULL;
> + pp->prflag = PRFLAG_NO;
>
> if (checker_selected(&pp->checker))
> checker_put(&pp->checker);
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 399540e7..5b77218b 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -249,6 +249,17 @@ enum recheck_wwid_states {
> RECHECK_WWID_ON = YNU_YES,
> };
>
> +/*
> + * PRFLAG_NO for path, it means reservation should be checked.
> + * PRFLAG_NO for multipath, it means mpp has no prkey.
> + * PRFLAG_OK for path, it means reservation has been checked.
> + * PRFLAG_OK for multipath, it means mpp has prkey.
> + */
> +enum prflag_states {
> + PRFLAG_NO = 0,
> + PRFLAG_OK = 1,
> +};
> +
> struct vpd_vendor_page {
> int pg;
> const char *name;
> @@ -327,6 +338,7 @@ struct path {
> /* configlet pointers */
> vector hwe;
> struct gen_path generic_path;
> + int prflag;
> };
>
> typedef int (pgpolicyfn) (struct multipath *, vector);
> diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> index 6d3a0ae2..8662fad7 100644
> --- a/multipathd/cli_handlers.c
> +++ b/multipathd/cli_handlers.c
> @@ -1341,7 +1341,7 @@ cli_setprstatus(void * v, char ** reply, int * len,
> void * data)
> return 1;
>
> if (!mpp->prflag) {
> - mpp->prflag = 1;
> + mpp->prflag = PRFLAG_OK;
> condlog(2, "%s: prflag set", param);
> }
>
> @@ -1352,9 +1352,11 @@ cli_setprstatus(void * v, char ** reply, int * len,
> void * data)
> int
> cli_unsetprstatus(void * v, char ** reply, int * len, void * data)
> {
> - struct multipath * mpp;
> - struct vectors * vecs = (struct vectors *)data;
> - char * param = get_keyparam(v, MAP);
> + int i;
> + struct multipath *mpp;
> + struct path *pp;
> + struct vectors *vecs = (struct vectors *)data;
> + char *param = get_keyparam(v, MAP);
>
> param = convert_dev(param, 0);
> get_path_layout(vecs->pathvec, 0);
> @@ -1364,7 +1366,10 @@ cli_unsetprstatus(void * v, char ** reply, int * len,
> void * data)
> return 1;
>
> if (mpp->prflag) {
> - mpp->prflag = 0;
> + mpp->prflag = PRFLAG_NO;
> + vector_foreach_slot(mpp->paths, pp, i) {
> + pp->prflag = PRFLAG_NO;
> + }
> condlog(2, "%s: prflag unset", param);
> }
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 82ab3ed1..6ef6495b 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -506,7 +506,7 @@ retry:
>
> if (mpp->prflag) {
> vector_foreach_slot(mpp->paths, pp, i) {
> - if ((pp->state == PATH_UP) || (pp->state ==
> PATH_GHOST)) {
> + if (!pp->prflag && ((pp->state == PATH_UP) ||
> (pp->state == PATH_GHOST))) {
> /* persistent reseravtion check*/
> mpath_pr_event_handle(pp);
> }
> @@ -3570,7 +3570,8 @@ void * mpath_pr_event_handler_fn (void * pathp )
> {
> condlog(0,"%s: Reservation registration failed. Error: %d",
> pp->dev, ret);
> }
> - mpp->prflag = 1;
> + mpp->prflag = PRFLAG_OK;
> + pp->prflag = PRFLAG_OK;
>
> free(param);
> out:
> --
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
