On 04/08/15 20:05, Guoqing Jiang wrote: > Hi Goldwyn, > > Thanks for review. > > Goldwyn Rodrigues wrote: >>> + set_dlm_hookers(); /* get dlm funcs from libdlm_lt.so.3 */ >>> + >> Universal Comment: Let call it set_dlm_hooks as opposed to hookers. >> > Not sure I understood correctly, the second patch used set_hookers to > call set_dlm_hookers. I think Neil meant to ask you to please do a global s/hookers/hooks/ because hooker has a particular meaning in english which is different to what you intended. http://www.urbandictionary.com/define.php?term=hooker Hope that helps. Regards, Adam >>> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) >>> >>> +static struct dlm_hookers *dlm_hookers = NULL; >>> +static int is_dlm_hookers_ready = 0; >> This should not be required, just checking for dlm_hooks == NULL >> should be enough. This needs to be set accordingly in set_dlm_hooks. >> > is_dlm_hookers_ready is introduced to check the dlm_* functions is > appeared in libdlm_lt.so.3 > or not, in case there is problem within the dlm lib. > >>> +static struct dlm_lock_resource *dlm_lock_res = NULL; >>> +static int ast_called = 0; >>> + >>> +struct dlm_lock_resource { >>> + dlm_lshandle_t *ls; >>> + struct dlm_lksb lksb; >>> +}; >>> + >>> +int is_clustered(struct supertype *st) >>> +{ >>> + /* is it a cluster md or not */ >>> + if (is_dlm_hookers_ready && st->cluster_name) >>> + return 1; >>> + else >>> + return 0; >>> +} >>> + >>> +/* Using poll(2) to wait for and dispatch ASTs */ >>> +static int poll_for_ast(dlm_lshandle_t ls) >>> +{ >>> + struct pollfd pfd; >> Shouldn't you check dlm_hooks is NULL here? and starting of every >> function which requires dlm_hooks. >> >> Also, a return value from these functions do not mean an error, it >> means the library is not present. >> > I don't think so, because is_dlm_hookers_ready not only ensures > libdlm_lt.so.3 existed and it also make sure > all the needed dlm hookers are set. And > cluster_get_dlmlock/cluster_release_dlmlock/dlm_ast/poll_for_ast > could only be execute while is_dlm_hookers_ready is set to 1. > > Thanks, > Guoqing > -- > To unsubscribe from this list: send the line "unsubscribe linux-raid" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Adam Goryachev Website Managers www.websitemanagers.com.au -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
