Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix MOSDOp encoding #6174

Merged
merged 4 commits into from Oct 20, 2015

Conversation

Projects
None yet
3 participants
@liewegas
Copy link
Member

liewegas commented Oct 3, 2015

We were previously using clenit_id in print() even though it was not decoded,
and the reqid handling was semi-broken.

messages/MOSDOp: avoid uninit/undecoded fields in print()
Signed-off-by: Sage Weil <sage@redhat.com>
@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Oct 6, 2015

lgtm with a nit which might not make any sense.

messages/MOSDOp: cleanup
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-13345 branch from 5bfb818 to 9c1a36e Oct 9, 2015

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Oct 9, 2015

@tchaikov I cleaned this up, and fixed the v6 case too (which was totally broken.. reqid is at the very end!).

This'll need to go through some upgrade tests before merging, but see if it looks right first?

@tchaikov tchaikov self-assigned this Oct 9, 2015

features(feat) {
set_tid(tid);

// also put the client_inc in reqid.inc, so that get_reqid() can
// be used before the full message is decode.

This comment has been minimized.

@tchaikov

tchaikov Oct 9, 2015

Contributor

s/decode/decoded/

@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Oct 12, 2015

lgtm other than a typo and the order of checking final_decode_needed and header.version in finish_decode().

liewegas added some commits Oct 9, 2015

messages/MOSDOp: decode complete message for v6, too.
We can't avoid this because we need the reqid before we've done the
full decoding, and that is at the very end of v6.  Too bad, so sad!

Signed-off-by: Sage Weil <sage@redhat.com>
messages/MOSDOp: fix reqid encoding/decoding
Normally, we don't fill in reqid at all (it's only for proxied ops).  But
we need the reqid from the partial decode.  Put client_inc in reqid.inc
when encoding the new format, and when decoding the old formats.  Then
fabricate the correct reqid in get_reqid().

Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-13345 branch 2 times, most recently from 5563183 to 0bf2a79 Oct 12, 2015

@liewegas liewegas added the needs-qa label Oct 12, 2015

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Oct 12, 2015

fixed those 2 things, thanks!

@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Oct 13, 2015

lgtm after the qa run passes.

liewegas added a commit that referenced this pull request Oct 20, 2015

Merge pull request #6174 from liewegas/wip-13345
fix MOSDOp encoding

Reviewed-by: Kefu Chai <kchai@redhat.com>

@liewegas liewegas merged commit 29fdadc into ceph:master Oct 20, 2015

@liewegas liewegas deleted the liewegas:wip-13345 branch Oct 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.