From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Benjamin Marzinski" Subject: Re: [PATCH 0/7] no_path_retry/queue_if_no_path logic & logging/man page fixes Date: Thu, 15 Jun 2017 14:46:40 -0500 Message-ID: <20170615194640.GC2940@octiron.msp.redhat.com> References: <20170613225554.11181-1-mwilck@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20170613225554.11181-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 To: Martin Wilck Cc: dm-devel@redhat.com, Xose Vazquez Perez List-Id: dm-devel.ids On Wed, Jun 14, 2017 at 12:55:47AM +0200, Martin Wilck wrote: > This patch set attempts to sanitize the logic used for consistently handling > options that can be set both via the "features" string and explicit multipath.conf > options. This is most prominently "no_path_retry" vs. "queue_if_no_path", but also > "retain_attached_hw_handler" vs. the feature of the same name. > > The logic this patch set follows is: > - If "no_path_retry" is set to any value, "queue_if_no_path" is ignored > (this is the case nowadays for almost all "real" storage arrays, thanks to hwtable). > - Otherwise, if "queue_if_no_path" is set, treat it like "no_path_retry queue". > > ... likewise for "retain_attached_hw_handler". I wholly approve of this change. I do have some minor issue with some of the individual patches. > The logic is implemented in select_features() (which needs to be called after > select_no_path_retry()) for multipath groups, and similarly in merge_hwe() for > hwtable entries. The actual feature setting is made in assemble_map(), I also > fix a problem there. > > The patch set documents the behavior in the man page, and adds some more > man page fixes. > > Finally, by skipping superfluous default initializations in load_config(), the > log messages for the respective config settings become more appropriate. > > Review and comments are highly welcome. > > Regards, > Martin > > Martin Wilck (7): > libmultipath: load_config: skip setting unnecessary defaults > libmultipath: add/remove_feature: use const char* for feature > libmultipath: clarify option conflicts for "features" > libmultipath: merge_hwe: fix queue_if_no_path logic > libmultipath: assemble_map: fix queue_if_no_path logic > multipath.conf.5: document no_path_retry vs. queue_if_no_path > multipath.conf.5: Remove ??? and other minor fixes > > libmultipath/config.c | 36 ++++++++++++++------------------ > libmultipath/configure.c | 4 ++-- > libmultipath/dmparser.c | 5 ++--- > libmultipath/propsel.c | 39 +++++++++++++++++++++------------- > libmultipath/structs.c | 30 +++++++++++++------------- > libmultipath/structs.h | 4 ++-- > multipath/multipath.conf.5 | 52 ++++++++++++++++++++++++++-------------------- > 7 files changed, 92 insertions(+), 78 deletions(-) > > -- > 2.13.0