rbd: don't default empty pool name unless namespace is specified

Commit 96f05a7 ("rbd: delay determination of default pool name") broke "rbd perf image iostat" and "rbd perf image iotop" GLOBAL_POOL_KEY support (the ability to blend all rbd pools together into a single view). Fixes: https://tracker.ceph.com/issues/56561 Signed-off-by: Ilya Dryomov <[email protected]>
drunkard · Jul 16, 2022 · b2137e2 · b2137e2
1 parent 367bf22
commit b2137e2
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 2 deletions.
diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh
@@ -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
@@ -1380,5 +1435,6 @@ test_thick_provision
 test_namespace
 test_trash_purge_schedule
 test_mirror_snapshot_schedule
+test_perf_image_iostat
 
 echo OK
diff --git a/src/tools/rbd/action/Perf.cc b/src/tools/rbd/action/Perf.cc
@@ -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
@@ -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) {