On Wed, Aug 14, 2019 at 09:22:17PM +0000, Martin Wilck wrote: > On Fri, 2019-08-02 at 11:33 -0500, Benjamin Marzinski wrote: > > In preparation for changing the path grouping code, add some unit > > tests > > to verify that it works correctly. The only test that currently fails > > (and so it being skipped) is using MULTIBUS when mp->paths is empty. > > All > > the other path grouping policies free mp->paths, even if it is empty. > > one_group() should as well. This will be fixed when the path grouping > > code is updated. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > --- > > tests/Makefile | 2 +- > > tests/pgpolicy.c | 708 > > +++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 709 insertions(+), 1 deletion(-) > > create mode 100644 tests/pgpolicy.c > > > > + > > +static void test_group_by_node_name_3_groups4(void **state) > > +{ > > + char *node_name[] = {"a","b","c","a"}; > > + int prio[] = {3,1,3,1}; > > + int group0[] = {2}; > > + int group1[] = {0,3}; > > + int group2[] = {1}; > > + int *groups[] = {group0, group1, group2}; > > + int group_size[] = {1,2,1}; > > + > > + set_priority(p4, prio, 4); > > + set_tgt_node_name(p4, node_name, 4); > > + assert_int_equal(group_by_node_name(&mp4), 0); > > + verify_pathgroups(&mp4, p4, groups, group_size, 3); > > +} > > Nothing wrong with your code, but this is one example where I would say > our prio group ordering is counter-intuitive. Does it really make sense > to order the group of two paths with prio {3, 1} *after* the group with > just one prio 3 path? That's kind of tricky, because with the round-robin path selector, just having one fast path in the group might be the correct answer, while with the dynamic path selectors, it seems like having both paths would be better. My person issue with out path grouping is that I don't think that what group_by_prio is what many devices want. For many devices, the paths are going to be grouped by something static, like what controller the paths go to. In that case, all the overhead of remaking that pathgroups whenever the priority changes is a bunch of wasted overhead. Also, it can cause situations where is multipathd notices a priority change at the wrong moment, it will temporarily make garbage pathgroups, with some paths using their old priority, and some using their new priority (which multipathd hasn't noticed yet). -Ben > > Regards, > Martin > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel