[Sheepdog] [RFC PATCH] sheep: introduce sd_op_template

[MORITA Kazutaka]

At Mon, 24 Oct 2011 14:16:55 +0900,
MORITA Kazutaka wrote:
> 
> 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.

Sorry, It looks like I misunderstood what you said.  I'd like to use
process()/post_process() for SD_OP_TYPE_IO in future.  But it will
need a lot of changes to Sheepdog codes.  Before that, we will need to
do much refinement on store.c, I think.

Thanks,

Kazutaka