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: removed hardcoded default pool #15518

Merged
merged 5 commits into from Jun 8, 2017

Conversation

Projects
None yet
2 participants
@dillaman
Copy link
Contributor

dillaman commented Jun 6, 2017

No description provided.

@trociny trociny self-assigned this Jun 7, 2017

@@ -1390,6 +1390,7 @@ OPTION(rbd_mirroring_replay_delay, OPT_INT, 0) // time-delay in seconds for rbd-
* rbd_create3()/RBD::create3() and rbd_clone2/RBD::clone2() are only
* affected by rbd_default_order.
*/
OPTION(rbd_default_pool, OPT_STR, "rbd") // default pool for storing images

This comment has been minimized.

Copy link
@trociny

trociny Jun 7, 2017

Contributor

Placing this option here might be confusing. The comment above says "rbd_create()/RBD::create() are affected by all of these options", while actually they are not affected by "rbd_default_pool".

This comment has been minimized.

Copy link
@trociny

trociny Jun 7, 2017

Contributor

Just an idea and might be done as a separate PR. We could add a validator for this option.

std::string default_pool_name;
get_default_pool_name(&default_pool_name);
if (r == -ENOENT && pool_name == default_pool_name) {
std::cerr << "rbd: error opening default pool '" << pool_name << std::endl

This comment has been minimized.

Copy link
@trociny

trociny Jun 7, 2017

Contributor

The closing quote (') after pool_name is missing.

return 0;
}

boost::regex pattern("^[^@/]*?$");

This comment has been minimized.

Copy link
@trociny

trociny Jun 7, 2017

Contributor

This also matches an empty string, making it a valid pool name.

std::string pool_name;
int get_default_pool_name(std::string *pool_name) {
std::string default_pool_name = g_ceph_context->_conf->rbd_default_pool;
int r = validate_pool_name(default_pool_name, SPEC_VALIDATION_FULL);

This comment has been minimized.

Copy link
@trociny

trociny Jun 7, 2017

Contributor

If we added the config option validator we could rely on it here.

@dillaman dillaman force-pushed the dillaman:wip-rbd-missing-default-pool branch 2 times, most recently from 38ac74e to 37b06aa Jun 7, 2017

@dillaman

This comment has been minimized.

Copy link
Contributor Author

dillaman commented Jun 7, 2017

@trociny update pushed

boost::regex pattern("^[^@/]+$");
if (!boost::regex_match (*value, pattern)) {
*error_message = "ignoring invalid RBD default pool";
*value = "rbd";

This comment has been minimized.

Copy link
@trociny

trociny Jun 7, 2017

Contributor

May be we should somehow mention int the error message that 'rbd' is used instead? Because now when I have

% ./bin/rbd --rbd-default-pool '/sjsj' create test2 --size 128M 
parse error setting 'rbd_default_pool' to '/sjsj' (ignoring invalid RBD default pool)

It is not clear that the image is actually created in rbd pool.

This comment has been minimized.

Copy link
@dillaman

dillaman Jun 7, 2017

Author Contributor

@trociny tweaked the message

dillaman added some commits Jun 6, 2017

common: new 'rbd_default_pool' configuration option
The 'rbd' pool will no longer be automatically created. Allow
the user to specify a custom default RBD pool name.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
pybind/rbd: module should depend on librbd not rbd CLI
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
rbd-replay: removed default rbd pool name
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
rbd-nbd: removed default rbd pool name
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
rbd: removed hardcoded default rbd pool name
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

@dillaman dillaman force-pushed the dillaman:wip-rbd-missing-default-pool branch from 37b06aa to e1cfaed Jun 7, 2017

@trociny

trociny approved these changes Jun 8, 2017

Copy link
Contributor

trociny left a comment

LGTM

@trociny trociny merged commit eac34cc into ceph:master Jun 8, 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

@dillaman dillaman deleted the dillaman:wip-rbd-missing-default-pool branch Jun 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.