On Mon, 2020-05-18 at 13:53 -0500, Benjamin Marzinski wrote: > On Fri, May 15, 2020 at 08:37:16PM +0000, Martin Wilck wrote: > > On Thu, 2020-05-14 at 20:59 -0500, Benjamin Marzinski wrote: > > > This code pulls the multipath path validation code out of > > > configure(), > > > and puts it into its own function, check_path_valid(). This > > > function > > > calls a new libmultipath function, is_path_valid() to check just > > > path > > > requested. This seperation exists so that is_path_valid() can be > > > reused > > > by future code. This code will give almost the same answer as the > > > existing code, with the exception that now, if a device is > > > currently > > > multipathed, it will always be a valid multipath path. > > > > > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > > > Great job getting the logic right! Readability massively improved. > > Almost ack, a few comments and questions below. > > > > Regards, > > Martin > > > > > > > - conf->find_multipaths |= _FIND_MULTIPATHS_I; > > > + if (conf->find_multipaths == FIND_MULTIPATHS_ON > > > + conf->find_multipaths == > > > FIND_MULTIPATHS_STRICT) > > > + conf->find_multipaths = > > > FIND_MULTIPATHS_SMART; > > > + else if (conf->find_multipaths == > > > FIND_MULTIPATHS_OFF) > > > + conf->find_multipaths = > > > FIND_MULTIPATHS_GREEDY; > > > > Ok. Previously FIND_MULTIPATHS_SMART was not the same value as > > FIND_MULTIPATHS_STRICT + _FIND_MULTIPATHS_I. Effectively, this > > doesn't > > change logic, but only because the check for ignore_new_devs_on() > > in > > should_multipath() is actually redundant. (IIRC in the past we'd > > determined that "strict" + "ignore_wwids" makes no sense). > > > > And I still feel like it doesn't make any sense, so this was > intentional. Are you arguing that we need that state, or are you > just > pointing this out? Just pointing it out. That's why I wrote "Ok" in the first place. But I stumbled on it at first sight, and took a while to realize that your patch got it right. Which is worthwhile to note IMO, in order not to stumble on it next time. Sorry for not expressing that clearly enough. Martin -- Dr. Martin Wilck <mwilck@xxxxxxxx>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
