[PATCH REPOST 0/2] rbd: standardize some variable names

[PATCH REPOST 0/2] rbd: standardize some variable names

[Alex Elder]

This series just makes the names of variables for
certain objects follow a consistent naming convention.

					-Alex

[PATCH REPOST 1/2] rbd: standardize rbd_request variable names
[PATCH REPOST 2/2] rbd: standardize ceph_osd_request variable names

[PATCH REPOST 1/2] rbd: standardize rbd_request variable names

[Alex Elder]

There are two names used for items of rbd_request structure type:
"req" and "req_data".  The former name is also used to represent
items of pointers to struct ceph_osd_request.

Change all variables that have these names so they are instead
called "rbd_req" consistently.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   50
++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 530a121..0091fa4 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1088,10 +1088,12 @@ static void rbd_coll_end_req_index(struct
request *rq,
 	spin_unlock_irq(q->queue_lock);
 }

-static void rbd_coll_end_req(struct rbd_request *req,
+static void rbd_coll_end_req(struct rbd_request *rbd_req,
 			     int ret, u64 len)
 {
-	rbd_coll_end_req_index(req->rq, req->coll, req->coll_index, ret, len);
+	rbd_coll_end_req_index(rbd_req->rq,
+				rbd_req->coll, rbd_req->coll_index,
+				ret, len);
 }

 /*
@@ -1119,12 +1121,12 @@ static int rbd_do_request(struct request *rq,
 	int ret;
 	u64 bno;
 	struct timespec mtime = CURRENT_TIME;
-	struct rbd_request *req_data;
+	struct rbd_request *rbd_req;
 	struct ceph_osd_request_head *reqhead;
 	struct ceph_osd_client *osdc;

-	req_data = kzalloc(sizeof(*req_data), GFP_NOIO);
-	if (!req_data) {
+	rbd_req = kzalloc(sizeof(*rbd_req), GFP_NOIO);
+	if (!rbd_req) {
 		if (coll)
 			rbd_coll_end_req_index(rq, coll, coll_index,
 					       -ENOMEM, len);
@@ -1132,8 +1134,8 @@ static int rbd_do_request(struct request *rq,
 	}

 	if (coll) {
-		req_data->coll = coll;
-		req_data->coll_index = coll_index;
+		rbd_req->coll = coll;
+		rbd_req->coll_index = coll_index;
 	}

 	dout("rbd_do_request object_name=%s ofs=%llu len=%llu coll=%p[%d]\n",
@@ -1150,12 +1152,12 @@ static int rbd_do_request(struct request *rq,

 	req->r_callback = rbd_cb;

-	req_data->rq = rq;
-	req_data->bio = bio;
-	req_data->pages = pages;
-	req_data->len = len;
+	rbd_req->rq = rq;
+	rbd_req->bio = bio;
+	rbd_req->pages = pages;
+	rbd_req->len = len;

-	req->r_priv = req_data;
+	req->r_priv = rbd_req;

 	reqhead = req->r_request->front.iov_base;
 	reqhead->snapid = cpu_to_le64(CEPH_NOSNAP);
@@ -1200,11 +1202,11 @@ static int rbd_do_request(struct request *rq,
 	return ret;

 done_err:
-	bio_chain_put(req_data->bio);
+	bio_chain_put(rbd_req->bio);
 	ceph_osdc_put_request(req);
 done_pages:
-	rbd_coll_end_req(req_data, ret, len);
-	kfree(req_data);
+	rbd_coll_end_req(rbd_req, ret, len);
+	kfree(rbd_req);
 	return ret;
 }

@@ -1213,7 +1215,7 @@ done_pages:
  */
 static void rbd_req_cb(struct ceph_osd_request *req, struct ceph_msg *msg)
 {
-	struct rbd_request *req_data = req->r_priv;
+	struct rbd_request *rbd_req = req->r_priv;
 	struct ceph_osd_reply_head *replyhead;
 	struct ceph_osd_op *op;
 	__s32 rc;
@@ -1232,20 +1234,20 @@ static void rbd_req_cb(struct ceph_osd_request
*req, struct ceph_msg *msg)
 		(unsigned long long) bytes, read_op, (int) rc);

 	if (rc == -ENOENT && read_op) {
-		zero_bio_chain(req_data->bio, 0);
+		zero_bio_chain(rbd_req->bio, 0);
 		rc = 0;
-	} else if (rc == 0 && read_op && bytes < req_data->len) {
-		zero_bio_chain(req_data->bio, bytes);
-		bytes = req_data->len;
+	} else if (rc == 0 && read_op && bytes < rbd_req->len) {
+		zero_bio_chain(rbd_req->bio, bytes);
+		bytes = rbd_req->len;
 	}

-	rbd_coll_end_req(req_data, rc, bytes);
+	rbd_coll_end_req(rbd_req, rc, bytes);

-	if (req_data->bio)
-		bio_chain_put(req_data->bio);
+	if (rbd_req->bio)
+		bio_chain_put(rbd_req->bio);

 	ceph_osdc_put_request(req);
-	kfree(req_data);
+	kfree(rbd_req);
 }

 static void rbd_simple_req_cb(struct ceph_osd_request *req, struct
ceph_msg *msg)
-- 
1.7.9.5

[PATCH REPOST 2/2] rbd: standardize ceph_osd_request variable names

[Alex Elder]

There are spots where a ceph_osds_request pointer variable is given
the name "req".  Since we're dealing with (at least) three types of
requests (block layer, rbd, and osd), I find this slightly
distracting.

Change such instances to use "osd_req" consistently to make the
abstraction represented a little more obvious.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   60
++++++++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 0091fa4..85131de 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1111,12 +1111,12 @@ static int rbd_do_request(struct request *rq,
 			  struct ceph_osd_req_op *ops,
 			  struct rbd_req_coll *coll,
 			  int coll_index,
-			  void (*rbd_cb)(struct ceph_osd_request *req,
-					 struct ceph_msg *msg),
+			  void (*rbd_cb)(struct ceph_osd_request *,
+					 struct ceph_msg *),
 			  struct ceph_osd_request **linger_req,
 			  u64 *ver)
 {
-	struct ceph_osd_request *req;
+	struct ceph_osd_request *osd_req;
 	struct ceph_file_layout *layout;
 	int ret;
 	u64 bno;
@@ -1143,67 +1143,68 @@ static int rbd_do_request(struct request *rq,
 		(unsigned long long) len, coll, coll_index);

 	osdc = &rbd_dev->rbd_client->client->osdc;
-	req = ceph_osdc_alloc_request(osdc, flags, snapc, ops,
+	osd_req = ceph_osdc_alloc_request(osdc, flags, snapc, ops,
 					false, GFP_NOIO, pages, bio);
-	if (!req) {
+	if (!osd_req) {
 		ret = -ENOMEM;
 		goto done_pages;
 	}

-	req->r_callback = rbd_cb;
+	osd_req->r_callback = rbd_cb;

 	rbd_req->rq = rq;
 	rbd_req->bio = bio;
 	rbd_req->pages = pages;
 	rbd_req->len = len;

-	req->r_priv = rbd_req;
+	osd_req->r_priv = rbd_req;

-	reqhead = req->r_request->front.iov_base;
+	reqhead = osd_req->r_request->front.iov_base;
 	reqhead->snapid = cpu_to_le64(CEPH_NOSNAP);

-	strncpy(req->r_oid, object_name, sizeof(req->r_oid));
-	req->r_oid_len = strlen(req->r_oid);
+	strncpy(osd_req->r_oid, object_name, sizeof(osd_req->r_oid));
+	osd_req->r_oid_len = strlen(osd_req->r_oid);

-	layout = &req->r_file_layout;
+	layout = &osd_req->r_file_layout;
 	memset(layout, 0, sizeof(*layout));
 	layout->fl_stripe_unit = cpu_to_le32(1 << RBD_MAX_OBJ_ORDER);
 	layout->fl_stripe_count = cpu_to_le32(1);
 	layout->fl_object_size = cpu_to_le32(1 << RBD_MAX_OBJ_ORDER);
 	layout->fl_pg_pool = cpu_to_le32((int) rbd_dev->spec->pool_id);
 	ret = ceph_calc_raw_layout(osdc, layout, snapid, ofs, &len, &bno,
-				   req, ops);
+				   osd_req, ops);
 	rbd_assert(ret == 0);

-	ceph_osdc_build_request(req, ofs, &len,
+	ceph_osdc_build_request(osd_req, ofs, &len,
 				ops,
 				snapc,
 				&mtime,
-				req->r_oid, req->r_oid_len);
+				osd_req->r_oid, osd_req->r_oid_len);

 	if (linger_req) {
-		ceph_osdc_set_request_linger(osdc, req);
-		*linger_req = req;
+		ceph_osdc_set_request_linger(osdc, osd_req);
+		*linger_req = osd_req;
 	}

-	ret = ceph_osdc_start_request(osdc, req, false);
+	ret = ceph_osdc_start_request(osdc, osd_req, false);
 	if (ret < 0)
 		goto done_err;

 	if (!rbd_cb) {
-		ret = ceph_osdc_wait_request(osdc, req);
+		u64 version;
+
+		ret = ceph_osdc_wait_request(osdc, osd_req);
+		version = le64_to_cpu(osd_req->r_reassert_version.version);
 		if (ver)
-			*ver = le64_to_cpu(req->r_reassert_version.version);
-		dout("reassert_ver=%llu\n",
-			(unsigned long long)
-				le64_to_cpu(req->r_reassert_version.version));
-		ceph_osdc_put_request(req);
+			*ver = version;
+		dout("reassert_ver=%llu\n", (unsigned long long) version);
+		ceph_osdc_put_request(osd_req);
 	}
 	return ret;

 done_err:
 	bio_chain_put(rbd_req->bio);
-	ceph_osdc_put_request(req);
+	ceph_osdc_put_request(osd_req);
 done_pages:
 	rbd_coll_end_req(rbd_req, ret, len);
 	kfree(rbd_req);
@@ -1213,9 +1214,9 @@ done_pages:
 /*
  * Ceph osd op callback
  */
-static void rbd_req_cb(struct ceph_osd_request *req, struct ceph_msg *msg)
+static void rbd_req_cb(struct ceph_osd_request *osd_req, struct
ceph_msg *msg)
 {
-	struct rbd_request *rbd_req = req->r_priv;
+	struct rbd_request *rbd_req = osd_req->r_priv;
 	struct ceph_osd_reply_head *replyhead;
 	struct ceph_osd_op *op;
 	__s32 rc;
@@ -1246,13 +1247,14 @@ static void rbd_req_cb(struct ceph_osd_request
*req, struct ceph_msg *msg)
 	if (rbd_req->bio)
 		bio_chain_put(rbd_req->bio);

-	ceph_osdc_put_request(req);
+	ceph_osdc_put_request(osd_req);
 	kfree(rbd_req);
 }

-static void rbd_simple_req_cb(struct ceph_osd_request *req, struct
ceph_msg *msg)
+static void rbd_simple_req_cb(struct ceph_osd_request *osd_req,
+				struct ceph_msg *msg)
 {
-	ceph_osdc_put_request(req);
+	ceph_osdc_put_request(osd_req);
 }

 /*
-- 
1.7.9.5

Re: [PATCH REPOST 1/2] rbd: standardize rbd_request variable names

[Dan Mick]

Reviewed-by: Dan Mick <dan.mick@inktank.com>

On 01/03/2013 02:38 PM, Alex Elder wrote:
> There are two names used for items of rbd_request structure type:
> "req" and "req_data".  The former name is also used to represent
> items of pointers to struct ceph_osd_request.
>
> Change all variables that have these names so they are instead
> called "rbd_req" consistently.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   50
> ++++++++++++++++++++++++++------------------------
>   1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 530a121..0091fa4 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1088,10 +1088,12 @@ static void rbd_coll_end_req_index(struct
> request *rq,
>   	spin_unlock_irq(q->queue_lock);
>   }
>
> -static void rbd_coll_end_req(struct rbd_request *req,
> +static void rbd_coll_end_req(struct rbd_request *rbd_req,
>   			     int ret, u64 len)
>   {
> -	rbd_coll_end_req_index(req->rq, req->coll, req->coll_index, ret, len);
> +	rbd_coll_end_req_index(rbd_req->rq,
> +				rbd_req->coll, rbd_req->coll_index,
> +				ret, len);
>   }
>
>   /*
> @@ -1119,12 +1121,12 @@ static int rbd_do_request(struct request *rq,
>   	int ret;
>   	u64 bno;
>   	struct timespec mtime = CURRENT_TIME;
> -	struct rbd_request *req_data;
> +	struct rbd_request *rbd_req;
>   	struct ceph_osd_request_head *reqhead;
>   	struct ceph_osd_client *osdc;
>
> -	req_data = kzalloc(sizeof(*req_data), GFP_NOIO);
> -	if (!req_data) {
> +	rbd_req = kzalloc(sizeof(*rbd_req), GFP_NOIO);
> +	if (!rbd_req) {
>   		if (coll)
>   			rbd_coll_end_req_index(rq, coll, coll_index,
>   					       -ENOMEM, len);
> @@ -1132,8 +1134,8 @@ static int rbd_do_request(struct request *rq,
>   	}
>
>   	if (coll) {
> -		req_data->coll = coll;
> -		req_data->coll_index = coll_index;
> +		rbd_req->coll = coll;
> +		rbd_req->coll_index = coll_index;
>   	}
>
>   	dout("rbd_do_request object_name=%s ofs=%llu len=%llu coll=%p[%d]\n",
> @@ -1150,12 +1152,12 @@ static int rbd_do_request(struct request *rq,
>
>   	req->r_callback = rbd_cb;
>
> -	req_data->rq = rq;
> -	req_data->bio = bio;
> -	req_data->pages = pages;
> -	req_data->len = len;
> +	rbd_req->rq = rq;
> +	rbd_req->bio = bio;
> +	rbd_req->pages = pages;
> +	rbd_req->len = len;
>
> -	req->r_priv = req_data;
> +	req->r_priv = rbd_req;
>
>   	reqhead = req->r_request->front.iov_base;
>   	reqhead->snapid = cpu_to_le64(CEPH_NOSNAP);
> @@ -1200,11 +1202,11 @@ static int rbd_do_request(struct request *rq,
>   	return ret;
>
>   done_err:
> -	bio_chain_put(req_data->bio);
> +	bio_chain_put(rbd_req->bio);
>   	ceph_osdc_put_request(req);
>   done_pages:
> -	rbd_coll_end_req(req_data, ret, len);
> -	kfree(req_data);
> +	rbd_coll_end_req(rbd_req, ret, len);
> +	kfree(rbd_req);
>   	return ret;
>   }
>
> @@ -1213,7 +1215,7 @@ done_pages:
>    */
>   static void rbd_req_cb(struct ceph_osd_request *req, struct ceph_msg *msg)
>   {
> -	struct rbd_request *req_data = req->r_priv;
> +	struct rbd_request *rbd_req = req->r_priv;
>   	struct ceph_osd_reply_head *replyhead;
>   	struct ceph_osd_op *op;
>   	__s32 rc;
> @@ -1232,20 +1234,20 @@ static void rbd_req_cb(struct ceph_osd_request
> *req, struct ceph_msg *msg)
>   		(unsigned long long) bytes, read_op, (int) rc);
>
>   	if (rc == -ENOENT && read_op) {
> -		zero_bio_chain(req_data->bio, 0);
> +		zero_bio_chain(rbd_req->bio, 0);
>   		rc = 0;
> -	} else if (rc == 0 && read_op && bytes < req_data->len) {
> -		zero_bio_chain(req_data->bio, bytes);
> -		bytes = req_data->len;
> +	} else if (rc == 0 && read_op && bytes < rbd_req->len) {
> +		zero_bio_chain(rbd_req->bio, bytes);
> +		bytes = rbd_req->len;
>   	}
>
> -	rbd_coll_end_req(req_data, rc, bytes);
> +	rbd_coll_end_req(rbd_req, rc, bytes);
>
> -	if (req_data->bio)
> -		bio_chain_put(req_data->bio);
> +	if (rbd_req->bio)
> +		bio_chain_put(rbd_req->bio);
>
>   	ceph_osdc_put_request(req);
> -	kfree(req_data);
> +	kfree(rbd_req);
>   }
>
>   static void rbd_simple_req_cb(struct ceph_osd_request *req, struct
> ceph_msg *msg)
>

Re: [PATCH REPOST 2/2] rbd: standardize ceph_osd_request variable names

[Dan Mick]

Reviewed-by: Dan Mick <dan.mick@inktank.com>

On 01/03/2013 02:40 PM, Alex Elder wrote:
> There are spots where a ceph_osds_request pointer variable is given
> the name "req".  Since we're dealing with (at least) three types of
> requests (block layer, rbd, and osd), I find this slightly
> distracting.
>
> Change such instances to use "osd_req" consistently to make the
> abstraction represented a little more obvious.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   60
> ++++++++++++++++++++++++++-------------------------
>   1 file changed, 31 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 0091fa4..85131de 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1111,12 +1111,12 @@ static int rbd_do_request(struct request *rq,
>   			  struct ceph_osd_req_op *ops,
>   			  struct rbd_req_coll *coll,
>   			  int coll_index,
> -			  void (*rbd_cb)(struct ceph_osd_request *req,
> -					 struct ceph_msg *msg),
> +			  void (*rbd_cb)(struct ceph_osd_request *,
> +					 struct ceph_msg *),
>   			  struct ceph_osd_request **linger_req,
>   			  u64 *ver)
>   {
> -	struct ceph_osd_request *req;
> +	struct ceph_osd_request *osd_req;
>   	struct ceph_file_layout *layout;
>   	int ret;
>   	u64 bno;
> @@ -1143,67 +1143,68 @@ static int rbd_do_request(struct request *rq,
>   		(unsigned long long) len, coll, coll_index);
>
>   	osdc = &rbd_dev->rbd_client->client->osdc;
> -	req = ceph_osdc_alloc_request(osdc, flags, snapc, ops,
> +	osd_req = ceph_osdc_alloc_request(osdc, flags, snapc, ops,
>   					false, GFP_NOIO, pages, bio);
> -	if (!req) {
> +	if (!osd_req) {
>   		ret = -ENOMEM;
>   		goto done_pages;
>   	}
>
> -	req->r_callback = rbd_cb;
> +	osd_req->r_callback = rbd_cb;
>
>   	rbd_req->rq = rq;
>   	rbd_req->bio = bio;
>   	rbd_req->pages = pages;
>   	rbd_req->len = len;
>
> -	req->r_priv = rbd_req;
> +	osd_req->r_priv = rbd_req;
>
> -	reqhead = req->r_request->front.iov_base;
> +	reqhead = osd_req->r_request->front.iov_base;
>   	reqhead->snapid = cpu_to_le64(CEPH_NOSNAP);
>
> -	strncpy(req->r_oid, object_name, sizeof(req->r_oid));
> -	req->r_oid_len = strlen(req->r_oid);
> +	strncpy(osd_req->r_oid, object_name, sizeof(osd_req->r_oid));
> +	osd_req->r_oid_len = strlen(osd_req->r_oid);
>
> -	layout = &req->r_file_layout;
> +	layout = &osd_req->r_file_layout;
>   	memset(layout, 0, sizeof(*layout));
>   	layout->fl_stripe_unit = cpu_to_le32(1 << RBD_MAX_OBJ_ORDER);
>   	layout->fl_stripe_count = cpu_to_le32(1);
>   	layout->fl_object_size = cpu_to_le32(1 << RBD_MAX_OBJ_ORDER);
>   	layout->fl_pg_pool = cpu_to_le32((int) rbd_dev->spec->pool_id);
>   	ret = ceph_calc_raw_layout(osdc, layout, snapid, ofs, &len, &bno,
> -				   req, ops);
> +				   osd_req, ops);
>   	rbd_assert(ret == 0);
>
> -	ceph_osdc_build_request(req, ofs, &len,
> +	ceph_osdc_build_request(osd_req, ofs, &len,
>   				ops,
>   				snapc,
>   				&mtime,
> -				req->r_oid, req->r_oid_len);
> +				osd_req->r_oid, osd_req->r_oid_len);
>
>   	if (linger_req) {
> -		ceph_osdc_set_request_linger(osdc, req);
> -		*linger_req = req;
> +		ceph_osdc_set_request_linger(osdc, osd_req);
> +		*linger_req = osd_req;
>   	}
>
> -	ret = ceph_osdc_start_request(osdc, req, false);
> +	ret = ceph_osdc_start_request(osdc, osd_req, false);
>   	if (ret < 0)
>   		goto done_err;
>
>   	if (!rbd_cb) {
> -		ret = ceph_osdc_wait_request(osdc, req);
> +		u64 version;
> +
> +		ret = ceph_osdc_wait_request(osdc, osd_req);
> +		version = le64_to_cpu(osd_req->r_reassert_version.version);
>   		if (ver)
> -			*ver = le64_to_cpu(req->r_reassert_version.version);
> -		dout("reassert_ver=%llu\n",
> -			(unsigned long long)
> -				le64_to_cpu(req->r_reassert_version.version));
> -		ceph_osdc_put_request(req);
> +			*ver = version;
> +		dout("reassert_ver=%llu\n", (unsigned long long) version);
> +		ceph_osdc_put_request(osd_req);
>   	}
>   	return ret;
>
>   done_err:
>   	bio_chain_put(rbd_req->bio);
> -	ceph_osdc_put_request(req);
> +	ceph_osdc_put_request(osd_req);
>   done_pages:
>   	rbd_coll_end_req(rbd_req, ret, len);
>   	kfree(rbd_req);
> @@ -1213,9 +1214,9 @@ done_pages:
>   /*
>    * Ceph osd op callback
>    */
> -static void rbd_req_cb(struct ceph_osd_request *req, struct ceph_msg *msg)
> +static void rbd_req_cb(struct ceph_osd_request *osd_req, struct
> ceph_msg *msg)
>   {
> -	struct rbd_request *rbd_req = req->r_priv;
> +	struct rbd_request *rbd_req = osd_req->r_priv;
>   	struct ceph_osd_reply_head *replyhead;
>   	struct ceph_osd_op *op;
>   	__s32 rc;
> @@ -1246,13 +1247,14 @@ static void rbd_req_cb(struct ceph_osd_request
> *req, struct ceph_msg *msg)
>   	if (rbd_req->bio)
>   		bio_chain_put(rbd_req->bio);
>
> -	ceph_osdc_put_request(req);
> +	ceph_osdc_put_request(osd_req);
>   	kfree(rbd_req);
>   }
>
> -static void rbd_simple_req_cb(struct ceph_osd_request *req, struct
> ceph_msg *msg)
> +static void rbd_simple_req_cb(struct ceph_osd_request *osd_req,
> +				struct ceph_msg *msg)
>   {
> -	ceph_osdc_put_request(req);
> +	ceph_osdc_put_request(osd_req);
>   }
>
>   /*
>

Re: [PATCH REPOST 0/2] rbd: standardize some variable names

[Josh Durgin]

On 01/03/2013 02:37 PM, Alex Elder wrote:
> This series just makes the names of variables for
> certain objects follow a consistent naming convention.
>
> 					-Alex
>
> [PATCH REPOST 1/2] rbd: standardize rbd_request variable names
> [PATCH REPOST 2/2] rbd: standardize ceph_osd_request variable names

These both look good to me.

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>