[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