Skip to content

messages/MOSDOp: clear reqid inc for v6 encoding#8299

Merged
liewegas merged 1 commit intoceph:masterfrom
liewegas:wip-15230
Mar 27, 2016
Merged

messages/MOSDOp: clear reqid inc for v6 encoding#8299
liewegas merged 1 commit intoceph:masterfrom
liewegas:wip-15230

Conversation

@liewegas
Copy link
Member

For the new v7 encoding, we put the client incarnation in the reqid so that
we can have it at the beginning of the encoding. However, we do not
want to do this for the v6 encoding, as that version of get_reqid() will
return reqid if it is not == osd_reqid_t(). Previously we were setting
the field in the constructor, but since it's specific to the v7 encoding,
put it there (in encode_payload()) instead.

Fixes: #15230
Signed-off-by: Sage Weil sage@redhat.com

::encode(fake_reqid, payload);
} else {
::encode(reqid, payload);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to use the actual reqid, not just fill in the inc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the reqid == osd_reqid_t path...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d'oh. Something about this this whole setup was confusing me previously, but I agree it looks good now.

For the new v7 encoding, we put the client incarnation in the
reqid so that we can have it at the beginning of the encoding.
However, we do *not* want to do this for the v6 encoding, as that
version of get_reqid() will return reqid if it is not ==
osd_reqid_t().

Fixes: ceph#15230
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas liewegas merged commit eeaab88 into ceph:master Mar 27, 2016
@liewegas liewegas deleted the wip-15230 branch March 27, 2016 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants