On 05/31/2013 08:52 PM, MORITA Kazutaka wrote: > At Fri, 31 May 2013 18:35:59 +0800, > Liu Yuan wrote: >> >> diff --git a/sheep/group.c b/sheep/group.c >> index b9399ee..f74ef10 100644 >> --- a/sheep/group.c >> +++ b/sheep/group.c >> @@ -1242,6 +1242,7 @@ void update_node_size(struct sd_node *node) >> int idx = get_node_idx(cur_vinfo, node); >> assert(idx != -1); >> cur_vinfo->nodes[idx].space = node->space; >> + sys->cdrv->update_node(node); >> } > > The current cluster driver model is: > > - Sheep triggers a node event with a cluster driver interface. > - The cluster driver updates its internal state, and invokes a > callback to notify the change to sheep. > > In that sense, this fix is a bit weird to me. > > I think a better fix is: > > 1. make SD_OP_REWEIGHT a local request. > 2. call cdrv->update_node() in cluster_reweight(). > 3. order the update_node event with other nonblocking events and call > sd_update_node() as a callback of the update_node event. > 4. call update_node_size() and kick_node_recover() in sd_update_node() > > Then, we can remove a SD_OP_UPDATE_SIZE operation and FIXME in the > local driver. > > That being said, the current fix also looks correct to me (except the > local driver), so I'm fine with merging this code without changes. > Yeah, your fix looks better integrated but this patch win as a simpler with less change approach and the node can be updated inside a ordered notify event. Local driver just a test driver so there is nothing to worry. That said, I'd like to merge my fix for 0.6.0 and prepare a another patch as you described. Thanks, Yuan |