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

tools/crushtool: replicated-rule API support #15011

Merged
merged 1 commit into from Jun 2, 2017

Conversation

Projects
None yet
3 participants
@xiexingguo
Member

xiexingguo commented May 9, 2017

This patch adds two interfaces to operate replicated crush rules,
namely "--create-simple-rule" and "--remove-rule".

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

@liewegas

This comment has been minimized.

Member

liewegas commented May 15, 2017

This looks good! Can you add test for this in test/cli/crushtool? Just crushtool -d between each step so we can see that it's working to create and then remove a rule...

if (add_rule) {
if (crush.rule_exists(rule_name)) {
cerr << "rule " << rule_name << " already exists" << std::endl;
return 0;

This comment has been minimized.

@tchaikov

tchaikov May 16, 2017

Contributor

why the ret code is 0, while the command fails? shall it be EXIT_FAILURE?

This comment has been minimized.

@xiexingguo

xiexingguo May 16, 2017

Member

yeah, fixed.

pg_pool_t::TYPE_REPLICATED, &err);
if (r < 0) {
cerr << err.str() << std::endl;
return r;

This comment has been minimized.

@tchaikov

tchaikov May 16, 2017

Contributor

i don't think we can use a negative number here, per http://pubs.opengroup.org/onlinepubs/009695399/functions/exit.html

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented May 17, 2017

Just crushtool -d between each step so we can see that it's working to create and then remove a rule...

@liewegas Add the test case. Looks okay now?

@@ -1,5 +1,7 @@
$ crushtool -i "$TESTDIR/simple.template" --add-item 0 1.0 device0 --loc host host0 --loc cluster cluster0 -o one > /dev/null
$ crushtool -i one --add-item 1 1.0 device1 --loc host host0 --loc cluster cluster0 -o two > /dev/null
$ crushtool -i two --create-simple-rule simple-rule cluster0 host firstn -o two > /dev/null

This comment has been minimized.

@liewegas

liewegas May 17, 2017

Member

can you crushtool -d ... after each of these new commands so that the we can see the rule (or lack of the rule) in the dump to confirm it is actually working?

This comment has been minimized.

@xiexingguo

xiexingguo May 18, 2017

Member

Done. Please review again...

@@ -1,5 +1,9 @@
$ crushtool -i "$TESTDIR/simple.template" --add-item 0 1.0 device0 --loc host host0 --loc cluster cluster0 -o one > /dev/null
$ crushtool -i one --add-item 1 1.0 device1 --loc host host0 --loc cluster cluster0 -o two > /dev/null
$ crushtool -i two --create-simple-rule simple-rule cluster0 host firstn -o two > /dev/null
$ crushtool -d two -o final

This comment has been minimized.

@liewegas

liewegas May 19, 2017

Member

I mean, leave off -o final, so that the output appears here, and therefore if we ever break the command(s) this test will fail to match the output.

This comment has been minimized.

@xiexingguo

xiexingguo May 22, 2017

Member

Repushed. Is this what you want?

This comment has been minimized.

@xiexingguo
$ crushtool -i two --create-simple-rule simple-rule cluster0 host firstn -o two > /dev/null
$ crushtool -d two -o final
$ cmp final "$TESTDIR/simple.template.two.newrule"
$ crushtool -i two --remove-rule simple-rule -o two > /dev/null

This comment has been minimized.

@liewegas

liewegas May 19, 2017

Member

and here

tools/crushtool: replicated-rule API support
This patch adds two interfaces to operate replicated crush rules,
namely "--create-simple-rule" and "--remove-rule".

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

@liewegas liewegas merged commit b735c5a into ceph:master Jun 2, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@xiexingguo xiexingguo deleted the xiexingguo:wip-crush-tool branch Jun 3, 2017

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