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

[Liu Yuan]

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