From: Alex Elder <elder@inktank.com>
To: Josh Durgin <josh.durgin@inktank.com>
Cc: "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>
Subject: Re: [PATCH REPOST 6/6] rbd: move remaining osd op setup into rbd_osd_req_op_create()
Date: Thu, 17 Jan 2013 16:25:06 -0600 [thread overview]
Message-ID: <50F87A42.8010402@inktank.com> (raw)
In-Reply-To: <50F7800C.4070306@inktank.com>
On 01/16/2013 10:37 PM, Alex Elder wrote:
> On 01/16/2013 10:23 PM, Josh Durgin wrote:
>> On 01/04/2013 07:07 AM, Alex Elder wrote:
>>> The two remaining osd ops used by rbd are CEPH_OSD_OP_WATCH and
>>> CEPH_OSD_OP_NOTIFY_ACK. Move the setup of those operations into
>>> rbd_osd_req_op_create(), and get rid of rbd_create_rw_op() and
>>> rbd_destroy_op().
>>>
>>> Signed-off-by: Alex Elder <elder@inktank.com>
. . .
>>> + case CEPH_OSD_OP_NOTIFY_ACK:
>>> + case CEPH_OSD_OP_WATCH:
>>> + /* rbd_osd_req_op_create(NOTIFY_ACK, cookie, version) */
>>> + /* rbd_osd_req_op_create(WATCH, cookie, version, flag) */
>>> + op->watch.cookie = va_arg(args, u64);
>>> + op->watch.ver = va_arg(args, u64);
>>> + op->watch.ver = cpu_to_le64(op->watch.ver); /* XXX */
>>
>> why the /* XXX */ comment?
>
> Because it's the only value here that is converted
> from cpu byte order. It was added in this commit:
>
> a71b891bc7d77a070e723c8c53d1dd73cf931555
> rbd: send header version when notifying
>
> And I think was done without full understanding that it
> was being done different from all the others. I think
> it may be wrong but I haven't really looked at it yet.
> Pulling them all into this function made this difference
> more obvious.
I've created this to track getting this issue resolved:
http://tracker.newdream.net/issues/3847
> It was a "note to self" that I wanted to fix that.
>
> I normally try to resolve anything like that before I
> post for review but I guess I forgot. There may be
> others.
I'm deleting the "XXX" comment before I commit this change.
-Alex
. . .
next prev parent reply other threads:[~2013-01-17 22:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-04 15:03 [PATCH REPOST 0/6] rbd: consolidate osd request setup Alex Elder
2013-01-04 15:05 ` [PATCH REPOST 1/6] rbd: don't assign extent info in rbd_do_request() Alex Elder
2013-01-04 15:06 ` [PATCH REPOST 2/6] rbd: don't assign extent info in rbd_req_sync_op() Alex Elder
2013-01-04 15:06 ` [PATCH REPOST 3/6] rbd: initialize off and len in rbd_create_rw_op() Alex Elder
2013-01-04 15:06 ` [PATCH REPOST 4/6] rbd: define generalized osd request op routines Alex Elder
2013-01-04 15:06 ` [PATCH REPOST 5/6] rbd: move call osd op setup into rbd_osd_req_op_create() Alex Elder
2013-01-04 15:07 ` [PATCH REPOST 6/6] rbd: move remaining " Alex Elder
2013-01-17 4:23 ` Josh Durgin
2013-01-17 4:37 ` Alex Elder
2013-01-17 22:25 ` Alex Elder [this message]
2013-01-17 4:25 ` [PATCH REPOST 0/6] rbd: consolidate osd request setup Josh Durgin
2013-01-17 22:19 ` Alex Elder
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=50F87A42.8010402@inktank.com \
--to=elder@inktank.com \
--cc=ceph-devel@vger.kernel.org \
--cc=josh.durgin@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox