Skip to content

Commit

Permalink
Merge pull request #47106 from idryomov/wip-56561
Browse files Browse the repository at this point in the history 
rbd: don't default empty pool name unless namespace is specified

Reviewed-by: Christopher Hoffman <choffman@redhat.com>
Reviewed-by: Mykola Golub <mgolub@suse.com>
  • Loading branch information
@idryomov
idryomov committed Jul 18, 2022
2 parents 279d37e + 365300f commit 1a1fea5
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 28 deletions.
56 changes: 56 additions & 0 deletions qa/workunits/rbd/cli_generic.sh
View file
Expand Up @@ -1358,6 +1358,61 @@ test_mirror_snapshot_schedule() {
ceph osd pool rm rbd2 rbd2 --yes-i-really-really-mean-it
}

test_perf_image_iostat() {
echo "testing perf image iostat..."
remove_images

ceph osd pool create rbd1 8
rbd pool init rbd1
rbd namespace create rbd1/ns
ceph osd pool create rbd2 8
rbd pool init rbd2
rbd namespace create rbd2/ns

IMAGE_SPECS=("test1" "rbd1/test2" "rbd1/ns/test3" "rbd2/test4" "rbd2/ns/test5")
for spec in "${IMAGE_SPECS[@]}"; do
# ensure all images are created without a separate data pool
# as we filter iostat by specific pool specs below
rbd create $RBD_CREATE_ARGS --size 10G --rbd-default-data-pool '' $spec
done

BENCH_PIDS=()
for spec in "${IMAGE_SPECS[@]}"; do
rbd bench --io-type write --io-pattern rand --io-total 10G --io-threads 1 \
--rbd-cache false $spec >/dev/null 2>&1 &
BENCH_PIDS+=($!)
done

# test specifying pool spec via spec syntax
test "$(rbd perf image iostat --format json rbd1 |
jq -r 'map(.image) | sort | join(" ")')" = 'test2'
test "$(rbd perf image iostat --format json rbd1/ns |
jq -r 'map(.image) | sort | join(" ")')" = 'test3'
test "$(rbd perf image iostat --format json --rbd-default-pool rbd1 /ns |
jq -r 'map(.image) | sort | join(" ")')" = 'test3'

# test specifying pool spec via options
test "$(rbd perf image iostat --format json --pool rbd2 |
jq -r 'map(.image) | sort | join(" ")')" = 'test4'
test "$(rbd perf image iostat --format json --pool rbd2 --namespace ns |
jq -r 'map(.image) | sort | join(" ")')" = 'test5'
test "$(rbd perf image iostat --format json --rbd-default-pool rbd2 --namespace ns |
jq -r 'map(.image) | sort | join(" ")')" = 'test5'

# test omitting pool spec (-> GLOBAL_POOL_KEY)
test "$(rbd perf image iostat --format json |
jq -r 'map(.image) | sort | join(" ")')" = 'test1 test2 test3 test4 test5'

for pid in "${BENCH_PIDS[@]}"; do
kill $pid
done
wait

remove_images
ceph osd pool rm rbd2 rbd2 --yes-i-really-really-mean-it
ceph osd pool rm rbd1 rbd1 --yes-i-really-really-mean-it
}

test_pool_image_args
test_rename
test_ls
Expand All @@ -1380,5 +1435,6 @@ test_thick_provision
test_namespace
test_trash_purge_schedule
test_mirror_snapshot_schedule
test_perf_image_iostat

echo OK
3 changes: 1 addition & 2 deletions src/tools/rbd/Utils.cc
View file
Expand Up @@ -197,8 +197,7 @@ std::string get_default_pool_name() {
}

int get_pool_and_namespace_names(
const boost::program_options::variables_map &vm,
bool default_empty_pool_name, bool validate_pool_name,
const boost::program_options::variables_map &vm, bool validate_pool_name,
std::string* pool_name, std::string* namespace_name, size_t *arg_index) {
if (namespace_name != nullptr && vm.count(at::NAMESPACE_NAME)) {
*namespace_name = vm[at::NAMESPACE_NAME].as<std::string>();
Expand Down
3 changes: 1 addition & 2 deletions src/tools/rbd/Utils.h
View file
Expand Up @@ -105,8 +105,7 @@ void normalize_pool_name(std::string* pool_name);
std::string get_default_pool_name();

int get_pool_and_namespace_names(
const boost::program_options::variables_map &vm,
bool default_empty_pool_name, bool validate_pool_name,
const boost::program_options::variables_map &vm, bool validate_pool_name,
std::string* pool_name, std::string* namespace_name, size_t *arg_index);

int get_pool_image_snapshot_names(
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rbd/action/Group.cc
View file
Expand Up @@ -123,7 +123,7 @@ int execute_list(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rbd/action/List.cc
View file
Expand Up @@ -314,7 +314,7 @@ int execute(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down
22 changes: 11 additions & 11 deletions src/tools/rbd/action/MirrorPool.cc
View file
Expand Up @@ -826,7 +826,7 @@ int execute_peer_bootstrap_create(
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -885,7 +885,7 @@ int execute_peer_bootstrap_import(
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -978,7 +978,7 @@ int execute_peer_add(const po::variables_map &vm,
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1069,7 +1069,7 @@ int execute_peer_remove(const po::variables_map &vm,
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1117,7 +1117,7 @@ int execute_peer_set(const po::variables_map &vm,
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1288,7 +1288,7 @@ int execute_disable(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand All @@ -1311,7 +1311,7 @@ int execute_enable(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1367,7 +1367,7 @@ int execute_info(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1463,7 +1463,7 @@ int execute_status(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1653,7 +1653,7 @@ int execute_promote(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -1692,7 +1692,7 @@ int execute_demote(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down
6 changes: 3 additions & 3 deletions src/tools/rbd/action/Namespace.cc
View file
Expand Up @@ -30,7 +30,7 @@ int execute_create(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -69,7 +69,7 @@ int execute_remove(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -115,7 +115,7 @@ int execute_list(const po::variables_map &vm,
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, true, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, true, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down
16 changes: 12 additions & 4 deletions src/tools/rbd/action/Perf.cc
View file
Expand Up @@ -584,7 +584,7 @@ int execute_iostat(const po::variables_map &vm,
std::string pool;
std::string pool_namespace;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, false, false, &pool,
int r = utils::get_pool_and_namespace_names(vm, false, &pool,
&pool_namespace, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -621,7 +621,11 @@ int execute_iostat(const po::variables_map &vm,
return r;
}

utils::normalize_pool_name(&pool);
if (!pool_namespace.empty()) {
// default empty pool name only if namespace is specified to allow
// for an empty pool_spec (-> GLOBAL_POOL_KEY)
utils::normalize_pool_name(&pool);
}
std::string pool_spec = format_pool_spec(pool, pool_namespace);

// no point to refreshing faster than the stats period
Expand Down Expand Up @@ -667,7 +671,7 @@ int execute_iotop(const po::variables_map &vm,
std::string pool;
std::string pool_namespace;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, false, false, &pool,
int r = utils::get_pool_and_namespace_names(vm, false, &pool,
&pool_namespace, &arg_index);
if (r < 0) {
return r;
Expand All @@ -685,7 +689,11 @@ int execute_iotop(const po::variables_map &vm,
return r;
}

utils::normalize_pool_name(&pool);
if (!pool_namespace.empty()) {
// default empty pool name only if namespace is specified to allow
// for an empty pool_spec (-> GLOBAL_POOL_KEY)
utils::normalize_pool_name(&pool);
}
iotop::MainWindow mainWindow(rados, format_pool_spec(pool, pool_namespace));
r = mainWindow.run();
if (r < 0) {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/rbd/action/Pool.cc
View file
Expand Up @@ -29,7 +29,7 @@ int execute_init(const po::variables_map &vm,
const std::vector<std::string> &ceph_global_init_args) {
std::string pool_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
nullptr, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -68,7 +68,7 @@ int execute_stats(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/rbd/action/Trash.cc
View file
Expand Up @@ -357,7 +357,7 @@ int execute_list(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down Expand Up @@ -408,7 +408,7 @@ int execute_purge(const po::variables_map &vm,
std::string pool_name;
std::string namespace_name;
size_t arg_index = 0;
int r = utils::get_pool_and_namespace_names(vm, true, false, &pool_name,
int r = utils::get_pool_and_namespace_names(vm, false, &pool_name,
&namespace_name, &arg_index);
if (r < 0) {
return r;
Expand Down

0 comments on commit 1a1fea5

Please sign in to comment.