On 01/06/2016 04:18 PM, Gabriel Krisman Bertazi wrote: > Brian King <brk...@linux.vnet.ibm.com> writes: > >>> I don't think replacing system with exec* is a much better solution >>> here. It's OK for your log system fix because we really need to call an >>> editor, but I think there are better ways to handle it here. >>> >>> I understand you just followed the original code, and to assume >>> multipath is in /sbin probably works for some scenarios, but it's >>> definitely not good and will fail for many others. We should at least >>> honor user's configured $PATH when invoking multipath. >>> >>> Much more surprising, looks like we have a *very* implicit dependency >>> for multipath-tools, which is bad, because none of the distro packages I >>> know considers that dependency. Not even the one we provide in the spec >>> directory. This is broken and will fail silently, even if harmlessly , >>> in any machine lacking multipath-tools. >> >> I do think this is still goodness overall. It gets rid of some hacky >> code and replaces it with less hacky code :) If the multipath call >> fails, no real harm will come, we'll just have stale multipath maps >> sitting around. Although I also agree that looking for a cleaner way >> to solve this would be nice. I'd prefer to avoid adding a dependency >> on libmultipath though if we can help it. > > yeah, I get why adding a hard dependency on libmultipath might not be a > very good idea. I wonder if some more restricted environments like > Petitboot have multipath-tools package so linking iprutils against it > might not be ideal. We'd need some machinery to disable it at build > time or some hack like that. > > I'm ok with applying a slightly modified version of this patch. I'd > recommend we change it to honor the user's $PATH, which can be done with > execvpe or similar. We should also alert downstream packagers about > this soft dependency, so they can properly handle it.
Honoring the users $PATH should be fine to do. > >> If there is some other way we could trigger userspace to flush unused >> maps without calling multipath directly we should pursue that. > > What if we call the DM_DEV_REMOVE ioctl [1] directly from iprutils? This > way we can remove the paths without invoking multipath or depending on > libmultipath. The DM ioctl interface is stable and (sort of) well > documented so it's not much trouble. The code would even get shorter :). > From a quick look at multipath-tools, I don't think it will disturb any > running multipathd instance, since it's similar to what the multipath > command does. That would be a very neat approach in my opinion. If we take that approach, I think we'd then need to: 1. Figure out the dm device associated with the sd device so we can make the ioctl call on the right multipath device. I don't think we have any code to do this today. 2. We'd also need to make sure the dm device is a multipath device and not just a dm device due to LVM, software RAID, etc. 3. We'd need to take care to also cleanup the children dm devices for the multipath device in the case the device has a partition table on it. 4. Multipath-tools also looks to have code to disable queue_if_no_path. We'd have to see what happens if that is set and we issue the DM_DEV_REMOVE ioctl. It seems like we'd have to add a bit of additional complexity to use DM_DEV_REMOVE Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center ------------------------------------------------------------------------------ _______________________________________________ Iprdd-devel mailing list Iprdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/iprdd-devel
