Re: [dm-devel] [PATCH V2 01/11] libmultipath: add group_by_tpg path_grouping_policy

[Benjamin Marzinski <bmarzins@redhat.com>]

On Wed, Jun 07, 2023 at 06:31:19PM +0000, Martin Wilck wrote:
> On Tue, 2023-06-06 at 15:13 -0500, Benjamin Marzinski wrote:
> >  #endif
> > diff --git a/libmultipath/prioritizers/alua.c
> > b/libmultipath/prioritizers/alua.c
> > index 0ab06e2b..4888a974 100644
> > --- a/libmultipath/prioritizers/alua.c
> > +++ b/libmultipath/prioritizers/alua.c
> > @@ -65,6 +65,7 @@ get_alua_info(struct path * pp, unsigned int
> > timeout)
> >                         return -ALUA_PRIO_NOT_SUPPORTED;
> >                 return -ALUA_PRIO_RTPG_FAILED;
> >         }
> > +       pp->tpg_id = tpg;
> >         condlog(3, "%s: reported target port group is %i", pp->dev,
> 
> 
> I still think that we should log a change here. Perhaps we should keep
> the existing condlog() and just use prio 2 if the tpg_id changed, and
> prio 4 if it didn't (the current 3 clutters the logs quite a bit).

Oops. My Bad. I meant to add that. I'll send a follow-on patch. 

-Ben

> 
> Regards
> Martin
> 
> 
> 
> > tpg);
> >         rc = get_asymmetric_access_state(pp, tpg, timeout);
> >         if (rc < 0) {
> > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> > index a25cc921..841fa247 100644
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -35,7 +35,8 @@ pgpolicyfn *pgpolicies[] = {
> >         one_group,
> >         group_by_serial,
> >         group_by_prio,
> > -       group_by_node_name
> > +       group_by_node_name,
> > +       group_by_tpg,
> >  };
> >  
> >  #define do_set(var, src, dest,
> > msg)                                    \
> > @@ -249,10 +250,26 @@ out:
> >         return 0;
> >  }
> >  
> > +static bool
> > +verify_alua_prio(struct multipath *mp)
> > +{
> > +       int i;
> > +       struct path *pp;
> > +
> > +       vector_foreach_slot(mp->paths, pp, i) {
> > +               const char *name = prio_name(&pp->prio);
> > +               if (strncmp(name, PRIO_ALUA, PRIO_NAME_LEN) &&
> > +                   strncmp(name, PRIO_SYSFS, PRIO_NAME_LEN))
> > +                        return false;
> > +       }
> > +       return true;
> > +}
> > +
> >  int select_pgpolicy(struct config *conf, struct multipath * mp)
> >  {
> >         const char *origin;
> >         char buff[POLICY_NAME_SIZE];
> > +       int log_prio = 3;
> >  
> >         if (conf->pgpolicy_flag > 0) {
> >                 mp->pgpolicy = conf->pgpolicy_flag;
> > @@ -265,9 +282,15 @@ int select_pgpolicy(struct config *conf, struct
> > multipath * mp)
> >         mp_set_conf(pgpolicy);
> >         mp_set_default(pgpolicy, DEFAULT_PGPOLICY);
> >  out:
> > +       if (mp->pgpolicy == GROUP_BY_TPG && !verify_alua_prio(mp)) {
> > +               mp->pgpolicy = DEFAULT_PGPOLICY;
> > +               origin = "(setting: emergency fallback - not all
> > paths use alua prio)";
> > +               log_prio = 1;
> > +       }
> >         mp->pgpolicyfn = pgpolicies[mp->pgpolicy];
> >         get_pgpolicy_name(buff, POLICY_NAME_SIZE, mp->pgpolicy);
> > -       condlog(3, "%s: path_grouping_policy = %s %s", mp->alias,
> > buff, origin);
> > +       condlog(log_prio, "%s: path_grouping_policy = %s %s", mp-
> > >alias, buff,
> > +               origin);
> >         return 0;
> >  }
> >  
> > diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> > index 87e84d5d..39504dca 100644
> > --- a/libmultipath/structs.c
> > +++ b/libmultipath/structs.c
> > @@ -125,6 +125,7 @@ alloc_path (void)
> >                 pp->sg_id.proto_id = PROTOCOL_UNSET;
> >                 pp->fd = -1;
> >                 pp->tpgs = TPGS_UNDEF;
> > +               pp->tpg_id = GROUP_ID_UNDEF;
> >                 pp->priority = PRIO_UNDEF;
> >                 pp->checkint = CHECKINT_UNDEF;
> >                 checker_clear(&pp->checker);
> > diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> > index a1208751..0eea19b4 100644
> > --- a/libmultipath/structs.h
> > +++ b/libmultipath/structs.h
> > @@ -317,6 +317,8 @@ struct hd_geometry {
> >  };
> >  #endif
> >  
> > +#define GROUP_ID_UNDEF -1
> > +
> >  struct path {
> >         char dev[FILE_NAME_SIZE];
> >         char dev_t[BLK_DEV_SIZE];
> > @@ -372,6 +374,7 @@ struct path {
> >         /* configlet pointers */
> >         vector hwe;
> >         struct gen_path generic_path;
> > +       int tpg_id;
> >  };
> >  
> >  typedef int (pgpolicyfn) (struct multipath *, vector);
> > diff --git a/multipath/main.c b/multipath/main.c
> > index 90f940f1..b78f3162 100644
> > --- a/multipath/main.c
> > +++ b/multipath/main.c
> > @@ -157,6 +157,7 @@ usage (char * progname)
> >                 "          . group_by_serial     one priority group
> > per serial\n"
> >                 "          . group_by_prio       one priority group
> > per priority lvl\n"
> >                 "          . group_by_node_name  one priority group
> > per target node\n"
> > +               "          . group_by_tpg        one priority group
> > per ALUA target port group\n"
> >                 "  -v lvl  verbosity level:\n"
> >                 "          . 0 no output\n"
> >                 "          . 1 print created devmap names only\n"
> > diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> > index b4dccd1b..b65a543d 100644
> > --- a/multipath/multipath.conf.5
> > +++ b/multipath/multipath.conf.5
> > @@ -233,6 +233,10 @@ per-multipath option in the configuration file.
> >  One priority group per target node name. Target node names are
> > fetched
> >  in \fI/sys/class/fc_transport/target*/node_name\fR.
> >  .TP
> > +.I group_by_tpg
> > +One priority group per ALUA target port group. In order to use this
> > policy,
> > +all paths in the multipath device must have \fIprio\fR set to
> > \fBalua\fR.
> > +.TP
> >  The default is: \fBfailover\fR
> >  .RE
> >  .
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel