[sheepdog] [PATCH v4 1/3] sheep: rework update node logic

[MORITA Kazutaka]

At Wed, 10 Jul 2013 21:29:02 +0800,
Liu Yuan wrote:
> 
> On Wed, Jul 10, 2013 at 10:13:18PM +0900, MORITA Kazutaka wrote:
> > At Wed, 10 Jul 2013 17:58:45 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Wed, Jul 10, 2013 at 06:07:32PM +0900, MORITA Kazutaka wrote:
> > > > Sorry, one more comment I've noticed this time.
> > > > 
> > > > > @@ -343,6 +346,17 @@ static bool __corosync_dispatch_one(struct corosync_event *cevent)
> > > > >  		sd_notify_handler(&cevent->sender.ent, cevent->msg,
> > > > >  						 cevent->msg_len);
> > > > >  		break;
> > > > > +	case COROSYNC_EVENT_TYPE_UPDATE_NODE:
> > > > > +		node = &cevent->sender.ent;
> > > > > +
> > > > > +		if (node_eq(node, &this_node.ent))
> > > > > +			this_node.ent = *node;
> > > > 
> > > > Using cpg_node_equal looks cleaner.
> > > 
> > > We have to init cpg_node to use cpg_node_equal, it is tedious.
> > 
> > cpg_node_equal() checks whether the cpg_nodes are the same sheep
> > daemon.  corosync_update_node() set the enough information for the
> > comparison to cevent->sender, so
> > 
> >   cpg_node_equal(&cevent->sender, &this_node)
> > 
> > should (must) work correctly.  We don't have to initialize any other
> > variables.
> 
> update_node() didn't set other information except sd_node. No?

Ah, sorry, I've noticed that cpg_node_equal() doesn't use sd_node info
to compare cpg_nodes, but it uses nodeid and pid.  However, then I
think we should set nodeid and pid in update_node().  It looks weird
to me that we cannot compare the sender node only when the event type
is COROSYNC_EVENT_TYPE_UPDATE_NODE in __corosync_dispatch_one().

Setting information to compare the sender node doesn't introduce any
complexity:

==
static int corosync_update_node(struct sd_node *node)
{
     struct cpg_node cnode = this_node;
     cnode.ent = *node;
==

Thanks,

Kazutaka