Re: [PATCH 1/2] Safeguard against writing to an active device of another node

[Adam Goryachev <mailinglists@websitemanagers.com.au>]

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Adam Goryachev
Website Managers
www.websitemanagers.com.au