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

os/bluestore: make bdev label parsing error more meaningful and less noisy #20090

Merged
merged 2 commits into from Feb 1, 2018

Conversation

liewegas
Copy link
Member

No description provided.

This happens during the normal initialization of a new bluestore osd and it
is confusing for users.  Make it less noisy.

Signed-off-by: Sage Weil <sage@redhat.com>
If there is not a valid label, then the label is not found.  This is a
more reasonable error code than "Invalid argumnet".

Signed-off-by: Sage Weil <sage@redhat.com>
@@ -4200,10 +4200,10 @@ int BlueStore::_read_bdev_label(CephContext* cct, string path,
decode(expected_crc, p);
}
catch (buffer::error& e) {
derr << __func__ << " unable to decode label at offset " << p.get_off()
dout(2) << __func__ << " unable to decode label at offset " << p.get_off()
Copy link
Contributor

Choose a reason for hiding this comment

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

dout(1), still an error, not able to decode.

Copy link
Contributor

@varadakari varadakari left a comment

Choose a reason for hiding this comment

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

LGTM

@alfredodeza
Copy link
Contributor

There isn't a way to conclude that this is the first time the OSD is being created which means it will hit this error? Ideally it wouldn't just send anything out. My concern is that if we make it less noisy, we won't catch it when it is a real problem

@tchaikov
Copy link
Contributor

tchaikov commented Feb 1, 2018

@alfredodeza
Copy link
Contributor

Actual ticket is http://tracker.ceph.com/issues/22285

If there isn't a way to make this understand this is the first time starting up, then I guess we can just move forward with this. Anything is better than what we currently have that looks like an error :(

@liewegas
Copy link
Member Author

liewegas commented Feb 1, 2018

I think this is good as is!

@liewegas liewegas merged commit fbc4192 into ceph:master Feb 1, 2018
@alfredodeza
Copy link
Contributor

How can we get this backported to Luminous?

@alfredodeza
Copy link
Contributor

Never mind, backported to Luminous with #20326

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants