[Sheepdog] [PATCH v2 2/3] sheep: timestamp the epoch log

Liu Yuan namei.unix at gmail.com
Wed Oct 26 13:48:55 CEST 2011


On 10/26/2011 05:40 PM, Liu Yuan wrote:

>>>
>>>>
>>>>>> diff --git a/sheep/ops.c b/sheep/ops.c
>>>>>> index 743eb0f..e2d7fb1 100644
>>>>>> --- a/sheep/ops.c
>>>>>> +++ b/sheep/ops.c
>>>>>> @@ -279,6 +279,7 @@ static int local_stat_cluster(const struct sd_req *req, struct sd_rsp *rsp,
>>>>>>  
>>>>>>  		rsp->data_length += sizeof(*log);
>>>>>>  		log->nr_nodes /= sizeof(log->nodes[0]);
>>>>>> +		log->time = *(time_t *)(&log->nodes[log->nr_nodes]);
>>>>
>>>> Looks a bit hacky to me.  In addition, this doesn't work if
>>>> sizeof(log->nodes[0]) < sizeof(time_t).
>>>
>>>
>>>
>>> Yes, tricky, but simplest. even sizeof(log->nodes[0]) < sizeof(time_t), I think this works out, cause
>>>
>>> we just need to get the address of end of the nodes array. Dereference
>>> is okay since it has converted into the right type(time_t *).
>>
>> If sizeof(log->nodes[0]) < sizeof(time_t),
>> log->nr_nodes /= sizeof(log->nodes[0]) doesn't set the correct number
>> of nodes.
>>
>>>
>>> any other more suitable means?
>>
>> I think we should define the epoch log layout and include followings:
>>
>>  - the number of nodes
>>  - the array of node
>>  - timestamp
>>
>>
> 
> 
> ah, thanks for the explanation. you are right.
> 
> I'll cook a v3 patch.
> 
> Thanks,
> Yuan


Kazum,
	There are many spots that rely on the old epoch layout (the node array
heads up), causing many change lines. so I think it is a bit nerdy to
change the layout, since we can safely assume time_t is *always* shorter
than sizeof(sheep node).

	Current hack is the simplest approach with smallest code change.
Changing the layout just for this epoch time somewhat cost too high.

Yuan



More information about the sheepdog mailing list