[Cluster-devel] Re: [PATCH 1/2] NLM failover unlock commands

[J. Bruce Fields]

On Tue, Jan 15, 2008 at 11:14:54AM -0500, Wendy Cheng wrote:
> J. Bruce Fields wrote:
>>>  +static ssize_t failover_unlock_ip(struct file *file, char *buf, 
>>> size_t size)
>>> +{
>>> +	__be32 server_ip;
>>> +	char *fo_path;
>>> +	char *mesg;
>>> +
>>> +	/* sanity check */
>>> +	if (size <= 0)
>>> +		return -EINVAL;
>>>     
>>
>> Not only is size never negative, it's actually an unsigned type here, so
>> this is a no-op.
>>   
>
> Based on comment from Neil, let's keep this ?

Yes, I misread, apologies!

>>   
>>> +
>>> +	server_ip = in_aton(fo_path);
>>>     
>>
>> It'd be nice if we could sanity-check this.  (Is there code already in
>> the kernel someplace to do this?)
>>   
>
> The argument here is that if this is a bogus address, the search (of nlm  
> files list) will fail (not found) later anyway.

Or it could accidentally find some other address?

> Failover is normally  
> time critical. Quicker we finish, less issues will be seen. The sanity  
> check here can be skipped (imo).

>> And, this isn't your problem for now, but eventually I guess this will
>> have to accept an ivp6 address as well?
>>   
>
> yeah .. Thinking about this right now.. May be borrowing the code from  
> ip_map_parse() as Neil pointed out in another mail ?

Yes, that looks easy enough.

I think the sanity checking would make it easier to extend the interface
later (for ipv6 or whatever else we might discover we need) because
we'll be able to depend on older kernels failing in a predictable way.

--b.