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

osd: constify OpRequest::get_req(); fix a few cases of operator<< vs mutated message races #13545

Merged
merged 28 commits into from Feb 24, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Member

liewegas commented Feb 20, 2017

Try to use const in as many places as possible. In cases where a Message we are processing is really mutated, annotate it by using a different OpRequest accessor (get_nonconst_req()) and make sure that it cannot race with the Message::print() method (used by operator<< and, most importantly, the asok 'ops' command, which may run at any time and must break due to any of our mutations).

I tried to order this so that it would build in sequence but didn't go back and verify. It should be close.

liewegas added some commits Feb 16, 2017

messages/MOSDPGPull: make pulls vector private
Signed-off-by: Sage Weil <sage@redhat.com>
osd/ECBackend: remove no-op set_priority calls
Signed-off-by: Sage Weil <sage@redhat.com>
osd: avoid require_mon_peer stray put()
For the handle_pg_create caller we cannot put the ref; do it in the callers
instead.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/OpRequest: add get_nonconst_req()
(Soon we'll make get_req() const.)

Signed-off-by: Sage Weil <sage@redhat.com>
common/RefCountedObject: allow refs to const objects
Make nref mutable, and make a const and non-const get() variant.

Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify a bit
Signed-off-by: Sage Weil <sage@redhat.com>
msg/Message: add const variant for get_data()
Signed-off-by: Sage Weil <sage@redhat.com>
os/ObjectStore: make setattrs take const attrset
Signed-off-by: Sage Weil <sage@redhat.com>
osd: add some const to MOSDPGLog handling; document non-constness of log
The PGLog (merge) code stills the pg_log_t entries, but operator<< (called
by the message printer) doesn't look at it.  Document.

Signed-off-by: Sage Weil <sage@redhat.com>
osd: make message check helpers const
Signed-off-by: Sage Weil <sage@redhat.com>
osd/Session: const Message* for check_backoff
Signed-off-by: Sage Weil <sage@redhat.com>
osd/ReplicatedBackend: constify
In a few places we copy a bufferlist instead of stealing it.  This is
fast (it just came off the wire and is one buffer).

Signed-off-by: Sage Weil <sage@redhat.com>
osd/ECBackend: constify
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: take const refs to pg info where possible
Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify handle_pg_notify
Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify handle_pg_remove
Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify handle_pg_query
Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify handle_pg_create
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PrimaryLogPG: non-const helpers where possible
Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify handle_pg_info
Signed-off-by: Sage Weil <sage@redhat.com>
osd: constify misc messages
Signed-off-by: Sage Weil <sage@redhat.com>
osd: cast const Message*'s where we can
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: cast const messages where we can
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PGBackend: take const where we can
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PrimaryLogPG: annotate non-const MOSDOp casts
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PrimaryLogPG: cast const Message*'s where possible
Signed-off-by: Sage Weil <sage@redhat.com>
osd/OpRequest: make get_req() return const
Anybody who needs a mutable pointer should be calling get_nonconst_ref()
and justify themselves.

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

@liewegas liewegas merged commit b0a3a2c into ceph:master Feb 24, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment