At Fri, 31 May 2013 21:45:31 +0800, Liu Yuan wrote: > > 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. Okay, I've applied this patch, thanks! Kazutaka |