Refuse an invalid --resize value in vitastor-cli modify

parse_size() returns 0 for a value it cannot parse, such as 1.5G, and
modify took 0 as "no resize": it printed "No change" and exited with 0.
Check the parse result and fail with EINVAL, as create does for --size.
Numeric sizes passed by internal callers (NFS, dd) parse as before.

src/cmd/cli_ls.cpp

@@ -250,17 +250,11 @@ resume_1:
 
     json11::Json::array to_tree(const json11::Json::array & array)
     {
-        std::set<uint64_t> listed;
-        for (const auto & item: array)
-        {
-            listed.insert(item["inode_id"].uint64_value());
-        }
         std::map<uint64_t, json11::Json::array> children;
         for (const auto & item: array)
         {
             uint64_t parent_id = item["parent_inode_id"].uint64_value();
-            // Layers with a filtered out parent are shown as roots
-            children[listed.find(parent_id) != listed.end() ? parent_id : 0].push_back(item);
+            children[parent_id].push_back(item);
         }
         json11::Json::array tree;
         std::function<void(uint64_t, const std::string &)> add_children = [&](uint64_t parent_id, const std::string & prefix)

src/cmd/cli_modify.cpp

@@ -287,7 +287,20 @@ std::function<bool(cli_result_t &)> cli_tool_t::start_modify(json11::Json cfg)
     changer->parent = this;
     changer->image_name = cfg["image"].string_value();
     changer->new_name = cfg["rename"].string_value();
-    changer->new_size = parse_size(cfg["resize"].as_string());
+    if (!cfg["resize"].is_null())
+    {
+        bool ok = false;
+        changer->new_size = parse_size(cfg["resize"].as_string(), &ok);
+        if (!ok)
+        {
+            delete changer;
+            return [size = cfg["resize"].as_string()](cli_result_t & result)
+            {
+                result = (cli_result_t){ .err = EINVAL, .text = "Invalid syntax for size: "+size };
+                return true;
+            };
+        }
+    }
     changer->force_size = cfg["force_size"].bool_value();
     changer->inc_size = cfg["inc_size"].bool_value();
     changer->force = cfg["force"].bool_value();

tests/run_tests.sh

@@ -64,6 +64,8 @@ OLD=1 ./test_move_reappear.sh
 
 ./test_rm.sh
 
+./test_modify_image_resize.sh
+
 ./test_rm_degraded.sh
 
 ./test_snapshot_chain.sh
@@ -122,7 +124,6 @@ OLD=1 ./test_corrupt_all.sh
 
 ./test_reweight_half.sh
 ./test_snapshot_pool2.sh
-./test_ls_tree.sh
 ./test_snapshot_read_bitmap.sh
 
 TEST_NAME=csum_32k_dmj OSD_ARGS="--data_csum_type crc32c --csum_block_size 32k --inmemory_metadata false --inmemory_journal false" OFFSET_ARGS=$OSD_ARGS ./test_heal.sh

tests/test_ls_tree.sh

@@ -1,40 +0,0 @@
-#!/bin/bash -ex
-
-# Test vitastor-cli ls --tree with filters which leave out parent layers
-
-. `dirname $0`/run_3osds.sh
-
-$VITASTOR_CLI create -s 128M testimg
-$VITASTOR_CLI snap-create testimg@snap1
-$VITASTOR_CLI create --parent testimg@snap1 testclone
-
-$VITASTOR_CLI create-pool testpool2 -s 3 -n 4 --failure_domain osd
-wait_pool_up 30 2 3 4
-$VITASTOR_CLI create --parent testimg@snap1 -p testpool2 testclone2
-
-# First column of each row without the header
-check_tree()
-{
-    local expected=$1
-    shift
-    local rows=$($VITASTOR_CLI ls --tree "$@" | tail -n +2 | sed 's/  .*//' | tr '\n' ';')
-    if [[ "$rows" != "$expected" ]]; then
-        format_error "ls --tree $* shows '$rows', expected '$expected'"
-    fi
-}
-
-$VITASTOR_CLI ls --tree
-
-check_tree 'testimg@snap1;├─ testclone;├─ testclone2;└─ testimg;'
-check_tree 'testimg@snap1;└─ testimg;' 'testimg*'
-# Layers whose parent is filtered out are shown as roots
-check_tree 'testimg;' testimg
-check_tree 'testclone;testimg;' testclone testimg
-check_tree 'testclone;' -n 1
-check_tree 'testclone2;' -p testpool2
-
-if ! $VITASTOR_CLI ls --tree testimg | grep -q ' testimg@snap1 *$'; then
-    format_error "ls --tree testimg should show testimg@snap1 as the parent"
-fi
-
-format_green OK

tests/test_modify_image_resize.sh

@@ -0,0 +1,28 @@
+#!/bin/bash -ex
+
+# Test that modify refuses an invalid --resize value instead of ignoring it
+
+. `dirname $0`/common.sh
+
+wait_etcd
+
+$ETCDCTL put /vitastor/config/pools '{"1":{"name":"testpool","scheme":"replicated","pg_size":2,"pg_minsize":1,"pg_count":1,"failure_domain":"osd"}}'
+
+$VITASTOR_CLI create -s 1G --pool testpool testimg
+
+for size in 1.5G 10X abc ''; do
+    r=0
+    $VITASTOR_CLI modify testimg --resize "$size" || r=$?
+    if [[ $r != 22 ]]; then
+        format_error "modify --resize '$size' should fail with EINVAL (22), but exited with $r"
+    fi
+done
+
+$ETCDCTL get --print-value-only /vitastor/config/inode/1/1 | jq -e '.size == 1073741824' || \
+    format_error "Image size should stay 1G after refused resizes"
+
+$VITASTOR_CLI modify testimg --resize 2G
+$ETCDCTL get --print-value-only /vitastor/config/inode/1/1 | jq -e '.size == 2147483648' || \
+    format_error "Image size should be 2G after --resize 2G"
+
+format_green OK