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: do not try to set device class before luminous #16706

Merged
merged 2 commits into from Aug 1, 2017

Conversation

Projects
None yet
5 participants
@jdurgin
Member

jdurgin commented Jul 31, 2017

This avoids crashing when older monitors do not support it.

Fixes: http://tracker.ceph.com/issues/20850
Signed-off-by: Josh Durgin jdurgin@redhat.com

if (osdmap.require_osd_release >= CEPH_RELEASE_LUMINOUS) {
r = update_crush_device_class();
if (r < 0) {
derr << __func__ <<" unable to update_crush_device_class: "

This comment has been minimized.

@tchaikov

tchaikov Jul 31, 2017

Contributor

add a space before "

@tchaikov tchaikov added the needs-qa label Jul 31, 2017

@ceph-jenkins

This comment has been minimized.

Collaborator

ceph-jenkins commented Jul 31, 2017

make check failed

@jdurgin jdurgin changed the title from osd: do not try to set device class before luminous to DNM: osd: do not try to set device class before luminous Jul 31, 2017

<< cpp_strerror(r) << dendl;
osd_lock.Lock();
goto monout;
if (osdmap->require_osd_release >= CEPH_RELEASE_LUMINOUS) {

This comment has been minimized.

@xiexingguo

xiexingguo Aug 1, 2017

Member

This seems not to be an ideal fix. For newly created osds even currently running on a Luminous cluster, the above condition will prevent them from automatically setting device classes as they should still hold empty osdmaps by now...
I'd instead suggest:
(1) simply ignore the result of update_crush_device_class(), we can still manually 'set-device-class' later;
(2) wait until monitor upgrade is done and then retry (update_crush_device_class())

jdurgin added some commits Jul 31, 2017

osd: do not try to set device class before luminous
This avoids crashing when older monitors do not support it.

Fixes: http://tracker.ceph.com/issues/20850
Signed-off-by: Josh Durgin <jdurgin@redhat.com>
mon/OSDMonitor: make setting require-osd-release idempotent
Signed-off-by: Josh Durgin <jdurgin@redhat.com>

@jdurgin jdurgin changed the title from DNM: osd: do not try to set device class before luminous to osd: do not try to set device class before luminous Aug 1, 2017

@liewegas liewegas merged commit 90d2fc2 into ceph:master Aug 1, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@jdurgin jdurgin deleted the jdurgin:wip-20850 branch Aug 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment