[sheepdog] [PATCH v7 5/6] shepherd: a new cluster manager specialized for sheepdog

[Hitoshi Mitake]

At Mon, 11 Mar 2013 13:38:51 +0800,
Liu Yuan wrote:
> 
> On 03/11/2013 11:48 AM, Hitoshi Mitake wrote:
> > +static void remove_handler(int fd, int events, void *data)
> > +{
> > +	struct sheep *s;
> > +	int ret, failed = 0;
> > +	eventfd_t val;
> > +	bool election = false;
> > +
> > +	ret = eventfd_read(remove_efd, &val);
> > +	if (ret < 0)
> > +		panic("eventfd_read() failed: %m");
> > +
> > +	sd_dprintf("removed sheeps: %lu", val);
> > +	assert(0 < val);
> > +
> > +
> > +remove:
> > +	/* FIXME */
> 
> FIX what?

The above comment is already obsolete. I'll remove it in the next series.

> 
> > +	list_for_each_entry(s, &sheep_list_head, sheep_list) {
> > +		if (s->state != SHEEP_STATE_LEAVING)
> > +			continue;
> > +
> 
> On 03/11/2013 11:48 AM, Hitoshi Mitake wrote:> +static void
> (*msg_handlers[])(struct sph_msg*, struct sheep *) = {
> > +	[SPH_CLI_MSG_JOIN] = msg_join,
> > +	[SPH_CLI_MSG_NEW_NODE_REPLY] = msg_new_node_reply,
> > +	[SPH_CLI_MSG_NOTIFY] = msg_notify,
> > +	[SPH_CLI_MSG_BLOCK] = msg_block,
> > +	[SPH_CLI_MSG_LEAVE] = msg_leave,
> > +};
> > +
> 
> I'd suggest
> 
> +static void (*msg_handlers[])(struct sph_msg*, struct sheep *) = {
> +	[SPH_CLI_MSG_JOIN] = sph_handle_join,
> +	[SPH_CLI_MSG_NEW_NODE_REPLY] = sph_handle_new_node_reply,
> +	[SPH_CLI_MSG_NOTIFY] = sph_handle_notify,
> +	[SPH_CLI_MSG_BLOCK] = sph_handle_block,
> +	[SPH_CLI_MSG_LEAVE] = sph_handle_leave,
> +};

These names seem to be suitable, but

> 
> How about NEW_NODE_REPLY -> JOIN_REPLY?

JOIN_REPLY is not suitable. Because JOIN_REPLY is already used by
the message of server -> client.

Thanks,
Hitoshi