Compare commits

...

1 Commits

Author SHA1 Message Date
a763d2d467 Refuse renaming a pool to the name of another pool in vitastor-cli
modify-pool --name did not check the new name against other pools, so
two pools could end up with the same name, and commands addressing a
pool by name then picked either of them. Fail with EEXIST, as
create-pool does. The check uses the pool list that the CAS write is
based on, so a concurrent rename or create is caught by the retry.
2026-10-20 10:57:35 +03:00
6 changed files with 57 additions and 0 deletions

View File

@@ -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`

View File

@@ -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`

View File

@@ -235,6 +235,7 @@ static const char* help_text =
" [--immediate_commit <all|small|none>] [--pg_stripe_size <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"

View File

@@ -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

View File

@@ -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

34
tests/test_modify_pool_name.sh Executable file
View File

@@ -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