[Sheepdog] [RFC PATCH] sheep: introduce sd_op_template

[MORITA Kazutaka]

At Fri, 21 Oct 2011 18:18:51 +0800,
Liu Yuan wrote:
> 
> On 10/21/2011 04:28 PM, MORITA Kazutaka wrote:
> 
> > When we want to add a new operation (SD_OP_xxxxx), it is not clear
> > which codes we should modify.  And in some cases, we need to modify
> > codes everywhere to implement one operation.  This is not a good
> > design.
> > 
> > This patch abstracts out Sheepdog operations into sd_op_template, and
> > moves all the request processing codes to sheep/ops.c.
> > 
> > The definition of sd_op_template is as follows:
> > 
> > struct sd_op_template {
> >         enum sd_op_type type;
> > 
> >         int available_always;
> > 
> >         int (*process)(const struct sd_req *req, struct sd_rsp *rsp,
> >                        void *data);
> >         int (*post_process)(const struct sd_req *req, struct sd_rsp *rsp,
> >                             void *data);
> > };
> > 
> > 'type' is the type of the operation; SD_OP_TYPE_CLUSTER,
> > SD_OP_TYPE_STORE, or SD_OP_TYPE_IO.
> > 
> > 'available_always' is set to non-zero if the operations should be
> > processed even when the cluster is not working.
> > 
> 
> 
> how about s/available_always/force ?
> 
> > 'process()' and 'post_process()' are the main functions of this
> > operation.  process() will be called in the worker thread, and
> > post_process() will be called in the main thread.
> > 
> 
> 
> So how about name it more descriptive like process_worker() and
> process_main()?
> 
> > If type is SD_OP_TYPE_CLUSTER, it is guaranteed that only one node
> > processes a cluster operation at the same time.  We can use this for
> > something like distributed locking.  process() will be called on the
> > local node, and post_process() will be called on every nodes.
> > 
> > If type is SD_OP_TYPE_STORE, both process() and post_process() will be
> > called on the local node.
> > 
> 
> 
> Same reason, how about SD_OP_TYPE_LOCAL?
> 
> > If type is SD_OP_TYPE_IO, neither process() nor post_process() is used
> > because this type of operation is heavily intertwined with Sheepdog
> > core codes.  We will be unlikely to add new operations of this type.
> >
> 
> 
> So is there any possibility later we might adopt io quest to
> process()/post_process()?

I think there is almost no possibility.  I don't want to add another
operation to Sheepdog object storage without a big reason.
create/read/write/remove are enough to me.

Thanks,

Kazutaka