Project

General

Profile

Bug #14365

unsafe handle_config_change() methods

Added by Josh Durgin about 3 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
01/14/2016
Due date:
% Done:

0%

Source:
other
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
Labels (FS):
Pull request ID:

Description

The handle_conf_change() methods in these files look potentially unsafe (modifying data without locks):

client/Client.cc
mds/StrayManager.cc

Associated revisions

Revision 94d1aa26 (diff)
Added by John Spray about 3 years ago

client: take lock in handle_conf_change

Fixes: #14365
Signed-off-by: John Spray <>

Revision 17e310f5 (diff)
Added by John Spray about 3 years ago

tasks/cephfs: add TestConfigCommands

Get some coverage on the otherwise rarely touched
injectargs and `config set` interfaces.

Fixes: #14365
Signed-off-by: John Spray <>

History

#1 Updated by Greg Farnum about 3 years ago

  • Assignee set to John Spray

I think these should be okay, but it's easy to fix. John, can you establish that we don't need locks or else set them up? (Not sure if I'm missing something in the stray manager's locking path.)

#2 Updated by John Spray about 3 years ago

  • Status changed from New to In Progress

#3 Updated by John Spray about 3 years ago

Turns out there's a pre-existing lock cycle issue similar to #14374 here, we just never noticed because live config changes are untested for the most part.

#4 Updated by John Spray about 3 years ago

Also, MDSDaemon::handle_conf_change was broken because it was taking mds_lock, when mds_lock is already taken in the handle_command path that's calling into injectargs.

#5 Updated by John Spray about 3 years ago

  • Status changed from In Progress to Need Review

#6 Updated by Greg Farnum about 3 years ago

  • Status changed from Need Review to Resolved

Also available in: Atom PDF