diff --git a/docs/usage/cli.en.md b/docs/usage/cli.en.md index 0b700725..aa110507 100644 --- a/docs/usage/cli.en.md +++ b/docs/usage/cli.en.md @@ -470,6 +470,8 @@ These, however, can still be modified with -f|--force. See [create-pool](#create-pool) for parameter descriptions. +Renaming with --name fails if another pool already has the new name. + Examples: `vitastor-cli modify-pool pool_A --name pool_B` diff --git a/docs/usage/cli.ru.md b/docs/usage/cli.ru.md index e4787d15..0f422085 100644 --- a/docs/usage/cli.ru.md +++ b/docs/usage/cli.ru.md @@ -487,6 +487,8 @@ OSD PARENT UP SIZE USED% TAGS WEIGHT BLOCK BITMAP Описания параметров смотрите в [create-pool](#create-pool). +Переименование с --name завершается ошибкой, если у другого пула уже есть такое имя. + Примеры: `vitastor-cli modify-pool pool_A --name pool_B` diff --git a/src/cmd/cli.cpp b/src/cmd/cli.cpp index 28ba05af..8352292b 100644 --- a/src/cmd/cli.cpp +++ b/src/cmd/cli.cpp @@ -235,6 +235,7 @@ static const char* help_text = " [--immediate_commit ] [--pg_stripe_size ]\n" " These, however, can still be modified with -f|--force.\n" " See create-pool for parameter descriptions.\n" + " Renaming with --name fails if another pool already has the new name.\n" " Examples:\n" " vitastor-cli modify-pool pool_A --name pool_B\n" " vitastor-cli modify-pool 2 --pg_size 4 -n 128\n" diff --git a/src/cmd/cli_pool_modify.cpp b/src/cmd/cli_pool_modify.cpp index 5930116b..5e4e433f 100644 --- a/src/cmd/cli_pool_modify.cpp +++ b/src/cmd/cli_pool_modify.cpp @@ -117,6 +117,22 @@ resume_1: return; } + if (new_cfg["name"] != old_cfg["name"]) + { + for (auto & pce: kv.value.object_items()) + { + if (pce.second["name"] == new_cfg["name"]) + { + result = (cli_result_t){ + .err = EEXIST, + .text = "Pool with name \""+new_cfg["name"].string_value()+"\" already exists (ID "+pce.first+")", + }; + state = 100; + return; + } + } + } + if (new_cfg.find("used_for_app") != new_cfg.end() && !force) { // Check that pool doesn't have images diff --git a/tests/run_tests.sh b/tests/run_tests.sh index d72fcf78..7a1e9d55 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -18,6 +18,8 @@ SCHEME=ec ./test_change_pg_count.sh ./test_create_nomaxid.sh +./test_modify_pool_name.sh + ./test_etcd_fail.sh ANTIETCD=1 ./test_etcd_fail.sh diff --git a/tests/test_modify_pool_name.sh b/tests/test_modify_pool_name.sh new file mode 100755 index 00000000..88938226 --- /dev/null +++ b/tests/test_modify_pool_name.sh @@ -0,0 +1,34 @@ +#!/bin/bash -ex + +# Test that modify-pool --name refuses to take the name of another pool + +. `dirname $0`/common.sh + +wait_etcd + +POOL='"scheme":"replicated","pg_size":2,"pg_minsize":1,"pg_count":1,"failure_domain":"osd"' +$ETCDCTL put /vitastor/config/pools '{"1":{"name":"pool1",'$POOL'},"2":{"name":"pool2",'$POOL'}}' + +for p in pool2 2; do + r=0 + $VITASTOR_CLI modify-pool $p --name pool1 || r=$? + if [[ $r != 17 ]]; then + format_error "modify-pool $p --name pool1 should fail with EEXIST (17), but exited with $r" + fi +done + +names=$($ETCDCTL get --print-value-only /vitastor/config/pools | jq -c '[ .[].name ]') +if [[ "$names" != '["pool1","pool2"]' ]]; then + format_error "Pool names should stay pool1, pool2, but they are $names" +fi + +# A free name and the pool's own name are still accepted +$VITASTOR_CLI modify-pool pool2 --name pool3 +$VITASTOR_CLI modify-pool pool3 --name pool3 --pg_count 2 + +cfg=$($ETCDCTL get --print-value-only /vitastor/config/pools | jq -c '[ .[] | [ .name, .pg_count ] ]') +if [[ "$cfg" != '[["pool1",1],["pool3",2]]' ]]; then + format_error "Pools should be pool1 (1 PG) and pool3 (2 PGs), but they are $cfg" +fi + +format_green OK