On Thu, 2017-06-22 at 08:21 +0200, Hannes Reinecke wrote: > On 06/21/2017 05:06 PM, Martin Wilck wrote: > > Setting a device handler only works if retain_attached_hw_handler > > is 'no', or if the kernel didn't auto-assign a handler. If this > > is not the case, don't even attempt to set a different handler. > > > > This requires reading the sysfs "dh_state" path attribute. > > For internal consistency, this attribute must be updated after > > domap(). > > > > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> > > --- > > libmultipath/configure.c | 52 > > +++++++++++++++++++++++++++++++++++++++++++++++- > > libmultipath/discovery.c | 4 ++++ > > libmultipath/propsel.c | 15 ++++++++++++++ > > libmultipath/structs.h | 2 ++ > > 4 files changed, 72 insertions(+), 1 deletion(-) > > > > [...] > > Hmm. > > Not sure if I fully agree with this. > I do see the need to read 'dh_state' from pathinfo(), just to figure > out > if an hardware handler is already loaded. > > But once select_hwhandler is done it's quite pointless to update the > dh_state; what exactly _would_ be the error action in this case? > Plus the code detects the failure, but then doesn't do anything with > it... My concern was that multipathd might carry along wrong state and possibly print it in log messages, irritating users. It's true, there is no reasonable error action, and it's quite a lot of code just for this minor purpose. > So, please, if you insist on checking dh_state please implement > correct > error action here, like updating the 'hwhandler' value to that found > in > dh_state or disabling the hardware handler if it's found to be > detached. If it's fine with you and other reviewers, I'll happily remove that part of the patch, and just keep the part in select_hwhandler(). If we want proper error handling, we'd need to check that the handler of the loaded map as well as the handlers of all paths are set to the handler configured in multipathd. Unless I'm mistaken, it isn't guaranteed that all paths will be using the same handler after a map is set up. Besides re-reading the dh_state of all paths, this check would also require re-reading and disassembling the map, and no reasonable error action is to be seen other then updating the internal state, lots of code for almost nothing. Cheers, Martin -- Dr. Martin Wilck <mwilck@xxxxxxxx>, Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
