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

Christoph Hellwig hch at infradead.org
Mon Nov 21 11:37:01 CET 2011


This is a very nice cleanup for the I/O code.

A few comments below:

> @@ -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?

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).

> -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.




More information about the sheepdog mailing list