[sheepdog] [PATCH] sheep: no need to set SD_FLAG_CMD_WRITE for local request

Liu Yuan namei.unix at gmail.com
Tue Aug 28 14:53:59 CEST 2012


On 08/28/2012 07:56 PM, MORITA Kazutaka wrote:
> When sheep receives a requests without SD_FLAG_CMD_WRITE, begin_rx()
> doesn't set req->data, so in such case prepare_cluster_msg() shouldn't
> set the invalid data to msg->data, no?  I believe multicasting invalid
> data makes it difficult for us to debug the code when we forget to add
> SD_FLAG_CMD_WRITE.  If it is true, we need to add SD_FLAG_CMD_WRITE
> even when the request is a local one because prepare_cluster_msg()
> cannot know whether the request is from local or not.
> 

It is internal function, so I don't think we need so strict checks. It seems that
for now we always send data for cluster requests. So this check just slow down
things. Even if it send the invalid data by accident, these data will be ignored
because the programmer write the request handler.

What cause most confusion to me earlier is that, why need to set CMD_WRITE flag when
I already set opcode as WRITE.

> I think it is worth considering to remove SD_FLAG_CMD_WRITE
> completely, but it is another issue to be discussed.  With the current
> design, we should add the flag when sending data strictly, at least
> for now.  With this patch, notify_recovery_completion_work() looks
> like trying to receive some data with SD_OP_COMPLETE_RECOVERY to me...

If we want to use SD_FLAG_CMD_WRITE to indicate data direction, we'd better use another
name such as SD_FLAG_DATA_IN/OUT. I guess I am not the only to ask why when setting something
as WRITE for twice in a request.

Thanks,
Yuan




More information about the sheepdog mailing list