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

[MORITA Kazutaka]

At Wed, 26 Oct 2011 19:48:55 +0800,
Liu Yuan wrote:
> 
> 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.

Okay, let's consider this issue again when we add another value to
epoch log file.

Thanks,

Kazutaka