[Sheepdog] [PATCH 2/3] sheep: refactor local and io request handling

Liu Yuan namei.unix at gmail.com
Mon Nov 21 11:56:04 CET 2011


On 11/21/2011 06:30 PM, Christoph Hellwig wrote:

> On Sun, Nov 20, 2011 at 07:11:26PM +0800, Liu Yuan wrote:
>> From: Liu Yuan <tailai.ly at taobao.com>
>>
>> They don't share any code or logic, let's split 'em out.
> 
> Yes, I had a somewhat similar patch in my queue.
> 
>> +static int do_local_rw(struct request *req, uint32_t epoch);
> 
> I'm not sure that's a good name.  Object removal or later a cache
> flush aren't simply read/write operations.  do_local_io maybe?
> 


Okay, I firstly use do_local_io, but I noticed other code path use it
too (vdi read/write) and it only does local read/write for now, then I
renamed it to do_local_rw.

If you later plan to code it support other io operations, it's better
name it do_local_io.

> Note that I'm also not a big fan of the do_ prefix - it doesn't really
> add any value to the function name.
> 
>> +void do_local_request(struct work *work, int idx)
>> +{
>> +	struct request *req = container_of(work, struct request, work);
>> +	struct sd_obj_rsp *rsp = (struct sd_obj_rsp *)&req->rp;
>> +	int ret = SD_RES_SUCCESS;
>> +
>> +	dprintf("local queue request: %d\n", idx);
>> +
>> +	if (has_process_work(req->op))
>> +		ret = do_process_work(req->op, &req->rq, &req->rp, req->data);
>> +
>> +	rsp->result = ret;
>> +}
> 
> Any reason this function stays in store.c, I'd have keept it with its
> only caller in sdnet.c.
> 


Okay, its cleaner to move it outside.

Thanks,
Yuan



More information about the sheepdog mailing list