Re: [PATCH REPOST 6/6] rbd: move remaining osd op setup into rbd_osd_req_op_create()

[Alex Elder <elder@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

. . .