On 09/27/2011 01:18 PM, MORITA Kazutaka wrote: > At Mon, 26 Sep 2011 18:57:23 +0800, > Liu Yuan wrote: >> From: Liu Yuan<tailai.ly at taobao.com> >> >> >> Signed-off-by: Liu Yuan<tailai.ly at taobao.com> >> --- >> sheep/group.c | 34 +++++++++++++++++++++++++++------- >> 1 files changed, 27 insertions(+), 7 deletions(-) >> >> diff --git a/sheep/group.c b/sheep/group.c >> index 812f6a0..4bf275e 100644 >> --- a/sheep/group.c >> +++ b/sheep/group.c >> @@ -142,6 +142,26 @@ CPG_EVENT_WORK_FNS(RUNNING, running) >> CPG_EVENT_WORK_FNS(SUSPENDED, suspended) >> CPG_EVENT_WORK_FNS(JOINING, joining) >> >> +static inline int join_message(struct message_header *m) >> +{ >> + return m->op == SD_MSG_JOIN; >> +} >> + >> +static inline int vdi_op_message(struct message_header *m) >> +{ >> + return m->op == SD_MSG_VDI_OP; >> +} >> + >> +static inline int master_chg_message(struct message_header *m) >> +{ >> + return m->op == SD_MSG_MASTER_CHANGED; >> +} >> + >> +static inline int leave_message(struct message_header *m) >> +{ >> + return m->op == SD_MSG_LEAVE; >> +} >> + > Are these helper functions really necessary? This patch looks a bit > odd to me. But if there is a reason for introducing those, I'm not > against it. > > Other than this, I think there is no problem. My testcase was > passed. :) > The main idea behind these helpers is to abstract away internal implementation of sheepdog message. For example, if we want to change these equation checks into bit operation when we consider necessary , these helpers would be helpful. For other reason, I think sheepdog internal structure (both code flow and data structure )would need retrofits and I just try to do it a bit by bit. Anyway, I am okay to drop these helpers. Thanks, Yuan |