Print vitastor-cli help for ls-osds and user-modify aliases

vitastor-cli accepts ls-osds and user-modify, but --help does not list
them, so "vitastor-cli --help ls-osds" printed "Unknown command". Add
them to the help and to the docs, which also lacked the pgs alias in the
Russian version. Test that --help knows every name the CLI accepts.

docs/usage/cli.en.md

@@ -243,16 +243,15 @@ Flatten a layer, i.e. merge data and detach it from parents.
 
 ## rm-data
 
-`vitastor-cli rm-data --pool <id|name> --inode <inode> [--wait-list] [--min-offset <offset>]`
+`vitastor-cli rm-data --pool <pool> --inode <inode> [--wait-list] [--min-offset <offset>]`
 
-Remove inode data without changing metadata. `<inode>` may be decimal or 0x hex.
+Remove inode data without changing metadata.
 
 ```
 --wait-list   Retrieve full objects listings before starting to remove objects.
               Requires more memory, but allows to show correct removal progress.
 --min-offset  Purge only data starting with specified offset.
 --max-offset  Purge only data before specified offset.
-              Offsets are in bytes or with a K/M/G/T unit suffix, decimal or 0x hex.
 --client_wait_up_timeout 16  Timeout for waiting until PGs are up in seconds.
 ```
 
@@ -356,7 +355,7 @@ host     stump
 
 ## ls-osd
 
-`vitastor-cli osds|ls-osd|osd-ls [-l|--long]`
+`vitastor-cli osds|ls-osds|ls-osd|osd-ls [-l|--long]`
 
 Show current OSDs as list, optionally with I/O statistics if -l is specified.
 
@@ -509,7 +508,7 @@ the Vitastor authorization model.
 
 ## modify-user
 
-`vitastor-cli modify-user --groups <group1,group2,...> <username>`
+`vitastor-cli modify-user|user-modify --groups <group1,group2,...> <username>`
 
 Create or update a user with the given name (matching TLS certificate CN).
 

docs/usage/cli.ru.md

@@ -252,16 +252,15 @@ vitastor-cli dd [iimg=<image> | if=<file>] [oimg=<image> | of=<file>] [bs=1M] \
 
 ## rm-data
 
-`vitastor-cli rm-data --pool <id|name> --inode <inode> [--wait-list] [--min-offset <offset>]`
+`vitastor-cli rm-data --pool <pool> --inode <inode> [--wait-list] [--min-offset <offset>]`
 
-Удалить данные инода, не меняя метаданные образов. `<inode>` можно задать в десятичном виде или в hex с 0x.
+Удалить данные инода, не меняя метаданные образов.
 
 ```
 --wait-list   Сначала запросить полный листинг объектов, а потом начать удалять.
               Требует больше памяти, но позволяет правильно печатать прогресс удаления.
 --min-offset  Удалять только данные, начиная с заданного смещения.
 --max-offset  Удалять только данные до (исключительно) заданного смещения.
-              Смещения задаются в байтах или с суффиксом K/M/G/T, в десятичном виде или в hex с 0x.
 --client_wait_up_timeout 16  Время ожидания поднятия PG в секундах.
 ```
 
@@ -374,7 +373,7 @@ host     stump
 
 ## ls-osd
 
-`vitastor-cli osds|ls-osd|osd-ls [-l|--long]`
+`vitastor-cli osds|ls-osds|ls-osd|osd-ls [-l|--long]`
 
 Показать список OSD, со статистикой ввода-вывода, если установлено -l.
 
@@ -404,7 +403,7 @@ OSD  PARENT            UP    SIZE  USED%    TAGS          WEIGHT  BLOCK  BITMAP
 
 ## pg-list
 
-`vitastor-cli pg-list|pg-ls|list-pg|ls-pg|ls-pgs [OPTIONS] [state1+state2] [^state3] [...]`
+`vitastor-cli pg-list|pg-ls|list-pg|ls-pg|ls-pgs|pgs [OPTIONS] [state1+state2] [^state3] [...]`
 
 Вывести список PG с состояними, удовлетворяющими любому из переданных фильтров (^ или !
 в начале фильтра означает отрицание). Опции:
@@ -528,7 +527,7 @@ OSD  PARENT            UP    SIZE  USED%    TAGS          WEIGHT  BLOCK  BITMAP
 
 ## modify-user
 
-`vitastor-cli modify-user --groups <group1,group2,...> <username>`
+`vitastor-cli modify-user|user-modify --groups <group1,group2,...> <username>`
 
 Создать или изменить пользователя с указанным именем (совпадающим с CN TLS-сертификата).
 

src/cmd/cli.cpp

@@ -116,13 +116,12 @@ static const char* help_text =
     "vitastor-cli flatten <layer>\n"
     "  Flatten a layer, i.e. merge data and detach it from parents.\n"
     "\n"
-    "vitastor-cli rm-data --pool <id|name> --inode <inode> [--wait-list] [--min-offset <offset>]\n"
-    "  Remove inode data without changing metadata. <inode> may be decimal or 0x hex.\n"
+    "vitastor-cli rm-data --pool <pool> --inode <inode> [--wait-list] [--min-offset <offset>]\n"
+    "  Remove inode data without changing metadata.\n"
     "  --wait-list   Retrieve full objects listings before starting to remove objects.\n"
     "                Requires more memory, but allows to show correct removal progress.\n"
     "  --min-offset  Purge only data starting with specified offset.\n"
     "  --max-offset  Purge only data before specified offset.\n"
-    "                Offsets are in bytes or with a K/M/G/T unit suffix, decimal or 0x hex.\n"
     "  --client_wait_up_timeout 16  Timeout for waiting until PGs are up in seconds.\n"
     "\n"
     "vitastor-cli merge-data <from> <to> [--target <target>]\n"
@@ -180,7 +179,7 @@ static const char* help_text =
     "vitastor-cli osd-tree [-l|--long]\n"
     "  Show current OSD tree, optionally with I/O statistics if -l is specified.\n"
     "\n"
-    "vitastor-cli osds|ls-osd|osd-ls [-l|--long]\n"
+    "vitastor-cli osds|ls-osds|ls-osd|osd-ls [-l|--long]\n"
     "  Show current OSDs as list, optionally with I/O statistics if -l is specified.\n"
     "\n"
     "vitastor-cli modify-osd [--tags tag1,tag2,...] [--reweight <number>] [--noout true/false] <osd_number>\n"
@@ -254,7 +253,7 @@ static const char* help_text =
     "vitastor-cli ls-users|user-ls|ls-user|list-users [<name> ...]\n"
     "  List users (only with specified names if passed).\n"
     "\n"
-    "vitastor-cli modify-user --type <type> --groups group1,group2,... <username>\n"
+    "vitastor-cli modify-user|user-modify --type <type> --groups group1,group2,... <username>\n"
     "  Create or update user permissions. User names match CN of their certificates.\n"
     "  --type TYPE      Set user type: client or admin. Default is client.\n"
     "  --groups GROUPS  Set user's groups.\n"

src/cmd/cli_rm_data.cpp

@@ -1,7 +1,6 @@
 // Copyright (c) Vitaliy Filippov, 2019+
 // License: VNPL-1.1 (see README.md for details)
 
-#include <ctype.h>
 #include "cli.h"
 #include "cluster_client.h"
 
@@ -24,7 +23,6 @@ struct rm_inode_t
 {
     uint64_t inode = 0;
     pool_id_t pool_id = 0;
-    std::string pool_name;
     uint64_t min_offset = 0;
     uint64_t max_offset = 0;
     bool down_ok = false;
@@ -304,25 +302,6 @@ struct rm_inode_t
             goto resume_1;
         if (state == 100)
             return;
-        if (pool_name != "")
-        {
-            pool_id = 0;
-            for (auto & pp: parent->cli->st_cli->pool_config)
-            {
-                if (pp.second.name == pool_name)
-                {
-                    pool_id = pp.first;
-                    break;
-                }
-            }
-            if (!pool_id)
-            {
-                result = (cli_result_t){ .err = ENOENT, .text = "Pool "+pool_name+" does not exist" };
-                state = 100;
-                return;
-            }
-            inode = INODE_WITH_POOL(pool_id, inode);
-        }
         if (!pool_id)
         {
             result = (cli_result_t){ .err = EINVAL, .text = "Pool is not specified" };
@@ -338,103 +317,20 @@ struct rm_inode_t
     }
 };
 
-// Decimal or 0x hex, without sign, octal or overflow
-static bool parse_rm_number(std::string s, uint64_t & value)
-{
-    int base = 10;
-    if (s.size() > 2 && s[0] == '0' && (s[1] == 'x' || s[1] == 'X'))
-    {
-        s = s.substr(2);
-        base = 16;
-    }
-    if (s == "" || s.find_first_not_of(base == 16 ? "0123456789abcdefABCDEF" : "0123456789") != std::string::npos)
-        return false;
-    errno = 0;
-    value = strtoull(s.c_str(), NULL, base);
-    return !errno;
-}
-
-// Command-line values are strings with optional K/M/G/T suffixes, internal callers pass numbers
-static bool parse_rm_offset(const json11::Json & value, uint64_t & offset)
-{
-    if (!value.is_string())
-    {
-        offset = value.uint64_value();
-        return true;
-    }
-    std::string s = value.string_value();
-    int shift = 0;
-    switch (s.size() ? tolower(s.back()) : 0)
-    {
-        case 'k': shift = 10; break;
-        case 'm': shift = 20; break;
-        case 'g': shift = 30; break;
-        case 't': shift = 40; break;
-    }
-    if (shift)
-        s.pop_back();
-    if (!parse_rm_number(s, offset) || offset > (UINT64_MAX >> shift))
-        return false;
-    offset <<= shift;
-    return true;
-}
-
-static bool parse_rm_inode(const json11::Json & value, uint64_t & inode)
-{
-    if (!value.is_string())
-    {
-        inode = value.uint64_value();
-        return true;
-    }
-    return parse_rm_number(value.string_value(), inode);
-}
-
-// A non-numeric string is a pool name, resolved after loading the pool configuration
-static bool parse_rm_pool(const json11::Json & value, uint64_t & pool_id, std::string & pool_name)
-{
-    pool_id = value.uint64_value();
-    if (value.is_string() && !pool_id && value.string_value() != "0")
-        pool_name = value.string_value();
-    return pool_id < POOL_ID_MAX && (!value.is_string() || value.string_value() != "");
-}
-
 std::function<bool(cli_result_t &)> cli_tool_t::start_rm_data(json11::Json cfg)
 {
-    uint64_t inode = 0, pool_id = 0, min_offset = 0, max_offset = 0;
-    std::string pool_name, error;
-    if (!parse_rm_inode(cfg["inode"], inode))
-        error = "Invalid --inode value: "+cfg["inode"].as_string();
-    else if (!parse_rm_pool(cfg["pool"], pool_id, pool_name))
-        error = "Invalid --pool value: "+cfg["pool"].as_string();
-    else if (!parse_rm_offset(cfg["min_offset"], min_offset))
-        error = "Invalid --min-offset value: "+cfg["min_offset"].as_string();
-    else if (!parse_rm_offset(cfg["max_offset"], max_offset))
-        error = "Invalid --max-offset value: "+cfg["max_offset"].as_string();
-    else if (!max_offset && cfg["max_offset"].is_string())
-        error = "--max-offset 0 is invalid, omit it to remove data up to the end";
-    else if (max_offset && min_offset >= max_offset)
-        error = "--min-offset must be less than --max-offset";
-    if (error != "")
-    {
-        return [error](cli_result_t & result)
-        {
-            result = (cli_result_t){ .err = EINVAL, .text = error };
-            return true;
-        };
-    }
     auto remover = new rm_inode_t();
     remover->parent = this;
-    remover->inode = inode;
-    remover->pool_id = pool_id;
-    remover->pool_name = pool_name;
+    remover->inode = cfg["inode"].uint64_value();
+    remover->pool_id = cfg["pool"].uint64_value();
     if (remover->pool_id)
     {
         remover->inode = (remover->inode & (((uint64_t)1 << (64-POOL_ID_BITS)) - 1)) | (((uint64_t)remover->pool_id) << (64-POOL_ID_BITS));
     }
     remover->down_ok = cfg["down_ok"].bool_value();
     remover->pool_id = INODE_POOL(remover->inode);
-    remover->min_offset = min_offset;
-    remover->max_offset = max_offset;
+    remover->min_offset = cfg["min_offset"].uint64_value();
+    remover->max_offset = cfg["max_offset"].uint64_value();
     return [remover](cli_result_t & result)
     {
         remover->loop();

tests/run_tests.sh

@@ -95,6 +95,8 @@ OLD=1 SCHEME=ec IMMEDIATE_COMMIT=1 ./test_rebalance_verify.sh
 
 ./test_dd.sh
 
+./test_cli_help.sh
+
 ./test_root_node.sh
 
 ./test_switch_primary.sh

tests/test_cli_help.sh

@@ -0,0 +1,14 @@
+#!/bin/bash -ex
+
+# Test that vitastor-cli --help knows every command name and alias the CLI accepts
+
+. `dirname $0`/common.sh
+
+for cmd in $(grep -o 'cmd\[0\] == "[^"]*"' src/cmd/cli.cpp | sed 's/.*== "//; s/"$//'); do
+    build/src/cmd/vitastor-cli --help $cmd > ./testdata/help.txt 2>&1 || true
+    if grep -q '^Unknown command' ./testdata/help.txt; then
+        format_error "vitastor-cli --help $cmd does not know the command"
+    fi
+done
+
+format_green OK

tests/test_rm.sh

@@ -24,132 +24,4 @@ sz=$(stat -c %s ./testdata/bin/testimg.bin)
 
 [[ "$sz" = 67108864 ]] || format_error "File size is $sz bytes after resize to 64 M"
 
-# test rm-data offset parsing and validation
-
-$VITASTOR_CLI create -s 32M rmimg
-dd if=/dev/urandom of=./testdata/bin/rmimg.bin bs=1M count=32
-$VITASTOR_CLI dd iodepth=4 bs=1M if=./testdata/bin/rmimg.bin oimg=rmimg
-
-check_rmimg()
-{
-    rm -f ./testdata/bin/check.bin
-    $VITASTOR_CLI dd iodepth=4 bs=1M conv=nosparse iimg=rmimg of=./testdata/bin/check.bin
-    cmp ./testdata/bin/check.bin ./testdata/bin/rmimg.bin || format_error "$1"
-}
-
-zero_rmimg()
-{
-    dd if=/dev/zero of=./testdata/bin/rmimg.bin bs=1M seek=$1 count=$2 conv=notrunc
-}
-
-check_rmimg "Image data differs after write"
-
-RM_DATA="$VITASTOR_CLI rm-data --pool 1 --inode $($VITASTOR_CLI ls --json rmimg | jq -r '.[0].inode_num')"
-
-# Bad offsets and empty ranges must fail without removing anything
-while IFS='|' read -r -u 3 args err; do
-    if out=$($RM_DATA $args 2>&1); then
-        format_error "rm-data $args should fail"
-    fi
-    [[ "$out" = "$err" ]] || format_error "rm-data $args failed with an unexpected error: $out"
-    check_rmimg "rm-data $args removed data"
-done 3<<EOF
---min-offset 8M --max-offset 10X|Invalid --max-offset value: 10X
---min-offset 8M --max-offset 1.5G|Invalid --max-offset value: 1.5G
---min-offset 8M --max-offset 0x|Invalid --max-offset value: 0x
---min-offset 8X --max-offset 16M|Invalid --min-offset value: 8X
---min-offset 1.5M|Invalid --min-offset value: 1.5M
---min-offset 8M --max-offset -8M|Invalid --max-offset value: -8M
---min-offset -1|Invalid --min-offset value: -1
---min-offset 16777217T|Invalid --min-offset value: 16777217T
---min-offset 8M --max-offset 16777217T|Invalid --max-offset value: 16777217T
---min-offset 18446744073709551616|Invalid --min-offset value: 18446744073709551616
---min-offset 8M --max-offset 0x10000000000000000|Invalid --max-offset value: 0x10000000000000000
---min-offset 16M --max-offset 8M|--min-offset must be less than --max-offset
---min-offset 8M --max-offset 8M|--min-offset must be less than --max-offset
---min-offset 16M --max-offset 0|--max-offset 0 is invalid, omit it to remove data up to the end
---max-offset 0M|--max-offset 0 is invalid, omit it to remove data up to the end
-EOF
-
-$RM_DATA --min-offset 010M --max-offset 012M
-zero_rmimg 10 2
-check_rmimg "rm-data --min-offset 010M --max-offset 012M removed wrong data"
-
-$RM_DATA --min-offset 8M --max-offset 10M
-zero_rmimg 8 2
-check_rmimg "rm-data --min-offset 8M --max-offset 10M removed wrong data"
-
-$RM_DATA --min-offset $((20*1024*1024)) --max-offset $((24*1024*1024))
-zero_rmimg 20 4
-check_rmimg "rm-data with offsets in bytes removed wrong data"
-
-$RM_DATA --min-offset 0x1a00000 --max-offset 0x1c00000
-zero_rmimg 26 2
-check_rmimg "rm-data with hex offsets removed wrong data"
-
-$RM_DATA --min-offset 30M
-zero_rmimg 30 2
-check_rmimg "rm-data --min-offset 30M removed wrong data"
-
-# test rm-data pool names and hex inode numbers
-
-$VITASTOR_CLI create-pool testpool2 -s 2 -n 4 --failure_domain osd
-wait_pool_up 60 2 2 4
-
-$VITASTOR_CLI create -s 32M -p testpool2 rmimg2
-dd if=/dev/urandom of=./testdata/bin/rmimg2.bin bs=1M count=32
-$VITASTOR_CLI dd iodepth=4 bs=1M if=./testdata/bin/rmimg2.bin oimg=rmimg2
-
-zero_rmimg2()
-{
-    dd if=/dev/zero of=./testdata/bin/rmimg2.bin bs=1M seek=$1 count=$2 conv=notrunc
-}
-
-check_rmimg2()
-{
-    check_rmimg "$1"
-    rm -f ./testdata/bin/check.bin
-    $VITASTOR_CLI dd iodepth=4 bs=1M conv=nosparse iimg=rmimg2 of=./testdata/bin/check.bin
-    cmp ./testdata/bin/check.bin ./testdata/bin/rmimg2.bin || format_error "$1"
-}
-
-check_rmimg2 "Image data differs after write"
-
-INODE1=$($VITASTOR_CLI ls --json rmimg | jq -r '.[0].inode_num')
-INODE2=$($VITASTOR_CLI ls --json rmimg2 | jq -r '.[0].inode_num')
-[[ "$($VITASTOR_CLI ls --json rmimg2 | jq -r '.[0].pool_id')" = 2 ]] || format_error "rmimg2 is not in pool 2"
-
-# Bad inode numbers and unknown pools must fail without removing anything
-while IFS='|' read -r -u 3 args err; do
-    if out=$($VITASTOR_CLI rm-data $args 2>&1); then
-        format_error "rm-data $args should fail"
-    fi
-    [[ "$out" = "$err" ]] || format_error "rm-data $args failed with an unexpected error: $out"
-    check_rmimg2 "rm-data $args removed data"
-done 3<<EOF
---pool 2 --inode ${INODE2}x|Invalid --inode value: ${INODE2}x
---pool 2 --inode 0x|Invalid --inode value: 0x
---pool 2 --inode 0x0x$INODE2|Invalid --inode value: 0x0x$INODE2
---pool 2 --inode -1|Invalid --inode value: -1
---pool 70000 --inode $INODE2|Invalid --pool value: 70000
---pool nosuchpool --inode $(( (1<<48) | INODE1 ))|Pool nosuchpool does not exist
-EOF
-
-$VITASTOR_CLI rm-data --pool testpool2 --inode $INODE2 --min-offset 8M --max-offset 12M
-zero_rmimg2 8 4
-check_rmimg2 "rm-data --pool testpool2 --inode $INODE2 removed wrong data"
-
-$VITASTOR_CLI rm-data --pool 2 --inode $(printf 0x%x $INODE2) --min-offset 16M --max-offset 20M
-zero_rmimg2 16 4
-check_rmimg2 "rm-data --pool 2 --inode $(printf 0x%x $INODE2) removed wrong data"
-
-$VITASTOR_CLI rm-data --inode $(printf 0x%x $(( (2<<48) | INODE2 ))) --min-offset 20M --max-offset 24M
-zero_rmimg2 20 4
-check_rmimg2 "rm-data --inode $(printf 0x%x $(( (2<<48) | INODE2 ))) removed wrong data"
-
-# --pool overrides pool bits of --inode
-$VITASTOR_CLI rm-data --pool testpool --inode $(( (2<<48) | INODE1 )) --min-offset 0 --max-offset 4M
-zero_rmimg 0 4
-check_rmimg2 "rm-data --pool testpool --inode $(( (2<<48) | INODE1 )) removed wrong data"
-
 format_green OK