[Sheepdog] [PATCH 3/3] sheep: use do_process_work() in io request

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


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

>> @@ -234,6 +234,11 @@ int get_cluster_copies(uint8_t *copies);
>>  int set_cluster_flags(uint16_t flags);
>>  int get_cluster_flags(uint16_t *flags);
>>  
>> +extern int store_create_and_write_obj(const struct sd_req *req, struct sd_rsp *rsp, void *data);
>> +extern int store_write_obj(const struct sd_req *req, struct sd_rsp *rsp, void *data);
>> +extern int store_read_obj(const struct sd_req *req, struct sd_rsp *rsp, void *data);
>> +extern int store_remove_obj(const struct sd_req *req, struct sd_rsp *rsp, void *data);
> 
> Any reason you use the extern keywords while the other functions don't?
> 


Okay, not much. It's easier for me to drop those 'extern'

> Also it seems like sheepdog code generally avoids lines longer than 80
> characters, at least I would love to keep it that way (but I'm not the
> maintainer, so don't put to much weight into my words).
> 


Okay.

>> -static int store_remove_obj(struct request *req, uint32_t epoch)
>> +int store_remove_obj(const struct sd_req *req, struct sd_rsp *rsp, void *data)
>>  {
>> -	struct sd_obj_req *hdr = (struct sd_obj_req *)&req->rq;
>> +	struct sd_obj_req *hdr = (struct sd_obj_req *)req;
> 
> No really a comment to this patch, but rather to the protocol headers
> in general:  Any reason why the sd_obj_req defintion (and others like
> it) can't simply include the sd_req in their defintion?  In that case
> we could actually use type-safe container_of constructs for things like
> this one.
> 


I am not sure (I only have around 2 month long coding on sheepdog), but
I notice some of code written the way you sugguest, so I think it is a
historic leftover.

Thanks,
Yuan



More information about the sheepdog mailing list