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

osd: bail out if transaction size overflows #10753

Merged
merged 2 commits into from Aug 21, 2016
Merged

Conversation

tchaikov
Copy link
Contributor

with a large MOSDMap message, the transaction size could be greater than
UINT_MAX. so fail early with error messages.

Fixes: http://tracker.ceph.com/issues/16982
Signed-off-by: Kefu Chai kchai@redhat.com

@athanatos
Copy link
Contributor

Let's talk about this in standup tomorrow.

@tchaikov tchaikov changed the title osd: bail out if transaction size overflows [DNM] osd: bail out if transaction size overflows Aug 18, 2016
@tchaikov
Copy link
Contributor Author

will check if we have a guard for file journal because the transaction size could be greater than the size of file journal!

@tchaikov tchaikov changed the title [DNM] osd: bail out if transaction size overflows osd: bail out if transaction size overflows Aug 19, 2016
@tchaikov
Copy link
Contributor Author

changelog

  • assert() if transaction size is greater than the journal size.


// store new maps: queue for disk and put in the osdmap cache
epoch_t start = MAX(superblock.newest_map + 1, first);
for (epoch_t e = start; e <= last; e++) {
if (txn_size > t.get_num_bytes()) {
derr << __func__ << " transaction size overflowed" << dendl;
assert(txn_size > t.get_num_bytes());
Copy link
Member

Choose a reason for hiding this comment

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

@tchaikov I think this assert has no effect(always true)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right. will fix.

with a large MOSDMap message, the transaction size could be greater than
UINT_MAX. so fail early with error messages.

Fixes: http://tracker.ceph.com/issues/16982
Signed-off-by: Kefu Chai <kchai@redhat.com>
if a transaction is too large to fit in the FileJournal's ring buffer,
we will wait. but if its size is larger than the max_size, it's likely
due to a bug or an invalid setting. in that case, we'd better fail
earlier.

Fixes: http://tracker.ceph.com/issues/16982
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

changelog

  • should assert() if assert(txn_size < t.get_num_bytes())
  • compare with header.max_size instead of max_size, the latter is the size of device not the size of journal.

@athanatos
Copy link
Contributor

lgtm, adding needs-qa

@tchaikov tchaikov merged commit 59db0ce into ceph:master Aug 21, 2016
@tchaikov tchaikov deleted the wip-16982 branch August 21, 2016 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants