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

rbd: additional validation for striping parameters #6914

Merged
merged 1 commit into from Dec 18, 2015
Merged

rbd: additional validation for striping parameters #6914

merged 1 commit into from Dec 18, 2015

Conversation

x11562
Copy link

@x11562 x11562 commented Dec 14, 2015

stripe_unit is same to object size or stripe_count is equal to 1,the remainder for it is misleading.
checking for striping parameters should be as much as possible int the front of processing.

Signed-off-by: Na Xie xie.na@h3c.com

@@ -772,11 +772,6 @@ int set_stripe_unit_count(cls_method_context_t hctx, bufferlist *in, bufferlist
CLS_ERR("failed to read the order off of disk: %s", cpp_strerror(r).c_str());
return r;
}
if ((1ull << order) % stripe_unit || stripe_unit > (1ull << order)) {
Copy link
Member

Choose a reason for hiding this comment

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

We should keep this check too, since librbd will still use it when not called from /usr/bin/rbd

@x11562
Copy link
Author

x11562 commented Dec 15, 2015

@jdurgin thanks, I have modified them

@ghost
Copy link

ghost commented Dec 15, 2015

@x11562 please ignore the bot failure, it was fixed by #6927. You can rebase and repush to get a new run.

stripe_unit is same to object size or stripe_count is equal to 1,the remainder for it is misleading.

Signed-off-by: Na Xie <xie.na@h3c.com>
@x11562
Copy link
Author

x11562 commented Dec 15, 2015

@dachary OK, it succeed

} else if ((stripe_unit || stripe_count) &&
(stripe_unit != (1ull << order) && stripe_count != 1)) {
} else if (stripe_unit || stripe_count) {
if ((1ull << order) % stripe_unit || stripe_unit >= (1ull << order)) {

Choose a reason for hiding this comment

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

What if stripe_unit == 1 && stripe_unit == 0? Divide by zero would result.

@x11562
Copy link
Author

x11562 commented Dec 16, 2015

@dillaman

  1. stripe_unit == 1 is not a good practice, but allowed.
  2. stripe_unit == 0,it is impossible.
if ((stripe_unit != 0 && stripe_count == 0) ||
      (stripe_unit == 0 && stripe_count != 0)) {
    std::cerr << "must specify both (or neither) of stripe-unit and stripe-count"
              << std::endl;
    return -EINVAL;
  } else if (stripe_unit || stripe_count) {
    if ((1ull << order) % stripe_unit || stripe_unit >= (1ull << order)) {
      std::cerr << "stripe unit is not a factor of the object size" << std::endl;
      return -EINVAL;
    }
    if (stripe_count == 1) {
      std::cerr << "stripe count not allowed to be 1" << std::endl;
      return -EINVAL;
    }
    features |= RBD_FEATURE_STRIPINGV2;
  } else {
    features &= ~RBD_FEATURE_STRIPINGV2;
  }

@dillaman
Copy link

Indeed -- it was cut off in the snippet.

👍

@x11562
Copy link
Author

x11562 commented Dec 16, 2015

@dillaman
yep

@dillaman dillaman changed the title rbd:check for striping parameters rbd: additional validation for striping parameters Dec 16, 2015
dillaman pushed a commit that referenced this pull request Dec 18, 2015
rbd: additional validation for striping parameters

Reviewed-by: Jason Dillaman <dillaman@redhat.com>
@dillaman dillaman merged commit 6eacdc5 into ceph:master Dec 18, 2015
@x11562 x11562 deleted the x11562 branch December 21, 2015 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants