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: fix huge (>4GB) bluefs reads #34503

Merged
merged 1 commit into from May 1, 2020

Conversation

ifed01
Copy link
Contributor

@ifed01 ifed01 commented Apr 9, 2020

This fixes huge (>4GB) reads handling at BlueFS. Originally BlueFS reads returned 32-bit signed integer indicating amount of read data. Which got overflow for 4+GB lengths.

Partially fixes: https://tracker.ceph.com/issues/40300
Signed-off-by: Igor Fedotov ifedotov@suse.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@tchaikov
Copy link
Contributor

@ifed01 needs rebase

@ifed01 ifed01 force-pushed the wip-ifed-fix-huge-bluefs-reads branch from da52f60 to 7090ea0 Compare April 13, 2020 10:25
@ifed01
Copy link
Contributor Author

ifed01 commented Apr 13, 2020

@tchaikov - thanks, fixed.

@ifed01
Copy link
Contributor Author

ifed01 commented Apr 27, 2020

Ping @aclamk

Copy link
Contributor

@aclamk aclamk left a comment

Choose a reason for hiding this comment

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

Good!

unique_ptr<char> huge_buf(new char[h->file->fnode.size]);
auto l = h->file->fnode.size;
int64_t r = fs.read(h, 0, l, NULL, huge_buf.get());
ASSERT_EQ(r, l);
delete h;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip: it could be good to actually check h->file->fnode.size is as it should be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, added.

Signed-off-by: Igor Fedotov <ifedotov@suse.com>
@tchaikov tchaikov merged commit befb2ab into ceph:master May 1, 2020
@ifed01 ifed01 deleted the wip-ifed-fix-huge-bluefs-reads branch May 1, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants