From 8c9872f7da2b4505a0f6454b042e3ffb62bc1d19 Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <idryomov@gmail.com>
Date: Thu, 14 Jul 2022 14:19:06 +0200
Subject: [PATCH] rbd: don't default empty pool name unless namespace is
 specified

Commit 96f05a7956b3 ("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 <idryomov@gmail.com>
(cherry picked from commit b2137e205862e6cfc316c11036266da65a78d26d)
---
 qa/workunits/rbd/cli_generic.sh | 56 +++++++++++++++++++++++++++++++++
 src/tools/rbd/action/Perf.cc    | 12 +++++--
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh
index a1723509586e2..2b3de518af637 100755
--- a/qa/workunits/rbd/cli_generic.sh
+++ b/qa/workunits/rbd/cli_generic.sh
@@ -1352,6 +1352,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
@@ -1374,5 +1429,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
index c38e02780650d..fdef1965f8aaa 100644
--- 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) {
-- 
2.39.5