Hello Mike: Thanks for your respond. > 1) in parse_path() you need to always initialize q the condition of q initialization is if (m->retain_attached_hw_handler)” comparing with previous condition if (test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags) || m->hw_handler_name) it does not increase the possibility of q initialization. > 2) setting m->hw_handler_name to attached_handler_name needs to still > happen regardless of whether m->hw_handler_name was previously set The patch does not change the logic above, does it? > 3) the "retain_attached_hw_handler" feature should still be allowed on > the command line, no sense to break multipath-tools Yes, actually I am still preparing two other patches now, one patch modifies parse_features() to ignore unsupported parameter and do not return failure. Another patch modifies multipath to remove this parameter also. > In addition, your patch actually breaks the ability to cope with –EBUSY > from the call to scsi_dh_attach(). Meaning we wouldn't properly fall > back to retaining the attached hw handler. scsi_dh_attach() return –EBUSY only if that device has attached a different handle, after previous processing, the handle would be same, so it is impossible to return –EBUSY in scsi_dh_attach(), we do not need to process it. Regards, Tang <tt> </tt> 发件人: Mike Snitzer <snitzer@redhat.com> 收件人: tang.junhui@zte.com.cn, 抄送: zhang.kai16@zte.com.cn, dm-devel@redhat.com, agk@redhat.com 日期: 2016/11/29 05:52 主题: Re: [dm-devel] [PATCH 2/2] dm-mpath: Remove useless retain_attached_hw_handler parameter 发件人: dm-devel-bounces@redhat.com <hr noshade> <tt>On Thu, Nov 24 2016 at 2:11am -0500, tang.junhui@zte.com.cn <tang.junhui@zte.com.cn> wrote: > From: "tang.junhui" <tang.junhui@zte.com.cn> > > Hardware handle would be retained no matter parameter > retain_attached_hw_handler is set or not in the logic > of current code. So remove this useless parameter. Right, that wasn't always the case. Previously (before commit ) dm-mpath would first detach the attached handler. I'm not completely opposed to removing the code that checks MPATHF_RETAIN_ATTACHED_HW_HANDLER in parse_path() but your proposed patch is broken in 2 ways: 1) in parse_path() you need to always initialize q 2) setting m->hw_handler_name to attached_handler_name needs to still happen regardless of whether m->hw_handler_name was previously set 3) the "retain_attached_hw_handler" feature should still be allowed on the command line, no sense to break multipath-tools But honestly, I'm not seeing any reason to not just leave the existing code. -- dm-devel mailing list dm-devel@redhat.com </tt><a href="https://www.redhat.com/mailman/listinfo/dm-devel"><tt>https://www.redhat.com/mailman/listinfo/dm-devel</tt></a><tt> </tt>