From: Josh Durgin <josh.durgin@inktank.com>
To: Alex Elder <elder@inktank.com>
Cc: "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>
Subject: Re: [PATCH] libceph: define osd_req_opcode_valid()
Date: Wed, 03 Apr 2013 11:39:11 -0700 [thread overview]
Message-ID: <515C774F.1000109@inktank.com> (raw)
In-Reply-To: <51560684.5060509@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 03/29/2013 02:24 PM, Alex Elder wrote:
> Define a separate function to determine the validity of an opcode,
> and use it inside osd_req_encode_op() in order to unclutter that
> function.
>
> Don't update the destination op at all--and return zero--if an
> unsupported or unrecognized opcode is seen in osd_req_encode_op().
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> net/ceph/osd_client.c | 126
> ++++++++++++++++++++++++++++---------------------
> 1 file changed, 72 insertions(+), 54 deletions(-)
>
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 015bf9f..4e5c043 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -220,70 +220,24 @@ struct ceph_osd_request
> *ceph_osdc_alloc_request(struct ceph_osd_client *osdc,
> }
> EXPORT_SYMBOL(ceph_osdc_alloc_request);
>
> -static u64 osd_req_encode_op(struct ceph_osd_request *req,
> - struct ceph_osd_op *dst,
> - struct ceph_osd_req_op *src)
> +static bool osd_req_opcode_valid(u16 opcode)
> {
> - u64 out_data_len = 0;
> - struct ceph_pagelist *pagelist;
> -
> - dst->op = cpu_to_le16(src->op);
> -
> - switch (src->op) {
> - case CEPH_OSD_OP_STAT:
> - break;
> + switch (opcode) {
> case CEPH_OSD_OP_READ:
> - case CEPH_OSD_OP_WRITE:
> - if (src->op == CEPH_OSD_OP_WRITE)
> - out_data_len = src->extent.length;
> - dst->extent.offset = cpu_to_le64(src->extent.offset);
> - dst->extent.length = cpu_to_le64(src->extent.length);
> - dst->extent.truncate_size =
> - cpu_to_le64(src->extent.truncate_size);
> - dst->extent.truncate_seq =
> - cpu_to_le32(src->extent.truncate_seq);
> - break;
> - case CEPH_OSD_OP_CALL:
> - pagelist = kmalloc(sizeof (*pagelist), GFP_NOFS);
> - BUG_ON(!pagelist);
> - ceph_pagelist_init(pagelist);
> -
> - dst->cls.class_len = src->cls.class_len;
> - dst->cls.method_len = src->cls.method_len;
> - dst->cls.indata_len = cpu_to_le32(src->cls.indata_len);
> - ceph_pagelist_append(pagelist, src->cls.class_name,
> - src->cls.class_len);
> - ceph_pagelist_append(pagelist, src->cls.method_name,
> - src->cls.method_len);
> - ceph_pagelist_append(pagelist, src->cls.indata,
> - src->cls.indata_len);
> -
> - req->r_data_out.type = CEPH_OSD_DATA_TYPE_PAGELIST;
> - req->r_data_out.pagelist = pagelist;
> - out_data_len = pagelist->length;
> - break;
> - case CEPH_OSD_OP_STARTSYNC:
> - break;
> - case CEPH_OSD_OP_NOTIFY_ACK:
> - case CEPH_OSD_OP_WATCH:
> - dst->watch.cookie = cpu_to_le64(src->watch.cookie);
> - dst->watch.ver = cpu_to_le64(src->watch.ver);
> - dst->watch.flag = src->watch.flag;
> - break;
> - default:
> - pr_err("unrecognized osd opcode %d\n", src->op);
> - WARN_ON(1);
> - break;
> + case CEPH_OSD_OP_STAT:
> case CEPH_OSD_OP_MAPEXT:
> case CEPH_OSD_OP_MASKTRUNC:
> case CEPH_OSD_OP_SPARSE_READ:
> case CEPH_OSD_OP_NOTIFY:
> + case CEPH_OSD_OP_NOTIFY_ACK:
> case CEPH_OSD_OP_ASSERT_VER:
> + case CEPH_OSD_OP_WRITE:
> case CEPH_OSD_OP_WRITEFULL:
> case CEPH_OSD_OP_TRUNCATE:
> case CEPH_OSD_OP_ZERO:
> case CEPH_OSD_OP_DELETE:
> case CEPH_OSD_OP_APPEND:
> + case CEPH_OSD_OP_STARTSYNC:
> case CEPH_OSD_OP_SETTRUNC:
> case CEPH_OSD_OP_TRIMTRUNC:
> case CEPH_OSD_OP_TMAPUP:
> @@ -291,11 +245,11 @@ static u64 osd_req_encode_op(struct
> ceph_osd_request *req,
> case CEPH_OSD_OP_TMAPGET:
> case CEPH_OSD_OP_CREATE:
> case CEPH_OSD_OP_ROLLBACK:
> + case CEPH_OSD_OP_WATCH:
> case CEPH_OSD_OP_OMAPGETKEYS:
> case CEPH_OSD_OP_OMAPGETVALS:
> case CEPH_OSD_OP_OMAPGETHEADER:
> case CEPH_OSD_OP_OMAPGETVALSBYKEYS:
> - case CEPH_OSD_OP_MODE_RD:
> case CEPH_OSD_OP_OMAPSETVALS:
> case CEPH_OSD_OP_OMAPSETHEADER:
> case CEPH_OSD_OP_OMAPCLEAR:
> @@ -326,13 +280,77 @@ static u64 osd_req_encode_op(struct
> ceph_osd_request *req,
> case CEPH_OSD_OP_RDUNLOCK:
> case CEPH_OSD_OP_UPLOCK:
> case CEPH_OSD_OP_DNLOCK:
> + case CEPH_OSD_OP_CALL:
> case CEPH_OSD_OP_PGLS:
> case CEPH_OSD_OP_PGLS_FILTER:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static u64 osd_req_encode_op(struct ceph_osd_request *req,
> + struct ceph_osd_op *dst,
> + struct ceph_osd_req_op *src)
> +{
> + u64 out_data_len = 0;
> + struct ceph_pagelist *pagelist;
> +
> + if (WARN_ON(!osd_req_opcode_valid(src->op))) {
> + pr_err("unrecognized osd opcode %d\n", src->op);
> +
> + return 0;
> + }
> +
> + switch (src->op) {
> + case CEPH_OSD_OP_STAT:
> + break;
> + case CEPH_OSD_OP_READ:
> + case CEPH_OSD_OP_WRITE:
> + if (src->op == CEPH_OSD_OP_WRITE)
> + out_data_len = src->extent.length;
> + dst->extent.offset = cpu_to_le64(src->extent.offset);
> + dst->extent.length = cpu_to_le64(src->extent.length);
> + dst->extent.truncate_size =
> + cpu_to_le64(src->extent.truncate_size);
> + dst->extent.truncate_seq =
> + cpu_to_le32(src->extent.truncate_seq);
> + break;
> + case CEPH_OSD_OP_CALL:
> + pagelist = kmalloc(sizeof (*pagelist), GFP_NOFS);
> + BUG_ON(!pagelist);
> + ceph_pagelist_init(pagelist);
> +
> + dst->cls.class_len = src->cls.class_len;
> + dst->cls.method_len = src->cls.method_len;
> + dst->cls.indata_len = cpu_to_le32(src->cls.indata_len);
> + ceph_pagelist_append(pagelist, src->cls.class_name,
> + src->cls.class_len);
> + ceph_pagelist_append(pagelist, src->cls.method_name,
> + src->cls.method_len);
> + ceph_pagelist_append(pagelist, src->cls.indata,
> + src->cls.indata_len);
> +
> + req->r_data_out.type = CEPH_OSD_DATA_TYPE_PAGELIST;
> + req->r_data_out.pagelist = pagelist;
> + out_data_len = pagelist->length;
> + break;
> + case CEPH_OSD_OP_STARTSYNC:
> + break;
> + case CEPH_OSD_OP_NOTIFY_ACK:
> + case CEPH_OSD_OP_WATCH:
> + dst->watch.cookie = cpu_to_le64(src->watch.cookie);
> + dst->watch.ver = cpu_to_le64(src->watch.ver);
> + dst->watch.flag = src->watch.flag;
> + break;
> + default:
> pr_err("unsupported osd opcode %s\n",
> ceph_osd_op_name(src->op));
> WARN_ON(1);
> - break;
> +
> + return 0;
> }
> + dst->op = cpu_to_le16(src->op);
> dst->payload_len = cpu_to_le32(src->payload_len);
>
> return out_data_len;
>
prev parent reply other threads:[~2013-04-03 18:39 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-29 21:24 [PATCH] libceph: define osd_req_opcode_valid() Alex Elder
2013-04-03 18:39 ` Josh Durgin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=515C774F.1000109@inktank.com \
--to=josh.durgin@inktank.com \
--cc=ceph-devel@vger.kernel.org \
--cc=elder@inktank.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.