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

mon: validate states transmitted in beacons #10428

Merged
merged 1 commit into from Jul 28, 2016
Merged

Conversation

jcsp
Copy link
Contributor

@jcsp jcsp commented Jul 25, 2016

Since FSMap was added, the state of a daemon can lead
to an entirely invalid map, but we were letting daemons
send any state they wanted.

Especially, we must not allow standby daemons to set
any state other than STANDBY.

Fixes: http://tracker.ceph.com/issues/16592
Signed-off-by: John Spray john.spray@redhat.com

@jcsp jcsp added bug-fix cephfs Ceph File System labels Jul 25, 2016
@gregsfortytwo
Copy link
Member

Actual build error here:

/home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/MDSMonitor.cc: In member function ‘bool MDSMonitor::prepare_beacon(MonOpRequestRef)’:
/home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/MDSMonitor.cc:660:16: error: expected primary-expression before ‘<<’ token
<< ceph_mds_state_name(info.state) << " -> "

@jcsp
Copy link
Contributor Author

jcsp commented Jul 27, 2016

Oops, updated.

@jcsp jcsp assigned jcsp and gregsfortytwo and unassigned jcsp Jul 27, 2016
@gregsfortytwo
Copy link
Member

Reviewed-by:

@gregsfortytwo gregsfortytwo removed their assignment Jul 27, 2016
@@ -646,6 +646,23 @@ bool MDSMonitor::prepare_beacon(MonOpRequestRef op)
m->get_name(), fsmap.get_epoch(), state, seq,
CEPH_FEATURES_SUPPORTED_DEFAULT));
} else {
if (info.state == MDSMap::STATE_STANDBY && state != info.state) {
Copy link
Member

Choose a reason for hiding this comment

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

These ifs should probably be an else if to be consistent I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, updated to flatten these conditions into the outer series of else ifs

@batrick
Copy link
Member

batrick commented Jul 27, 2016

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>

Since FSMap was added, the state of a daemon can lead
to an entirely invalid map, but we were letting daemons
send any state they wanted.

Especially, we must not allow standby daemons to set
any state other than STANDBY.

Fixes: http://tracker.ceph.com/issues/16592
Signed-off-by: John Spray <john.spray@redhat.com>
@jcsp jcsp merged commit d8d1483 into ceph:master Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants