From a107c47360ecdb8c09768ca9eab2341100245711 Mon Sep 17 00:00:00 2001
From: Ilya Dryomov <idryomov@gmail.com>
Date: Thu, 24 Sep 2020 10:50:24 +0200
Subject: [PATCH] rbd: make common options override krbd-specific options

ceph-csi has added support for passing custom map and unmap options via
mapOptions and unmapOptions storage class parameters.  However, it also
uses --read-only for implementing ROX (ReadOnlyMany) PVs.  If the user
supplies "mapOptions: rw", they will get around the intended read-only
restriction (at least on the block device).

ceph-csi could be patched to use "-o ro", but it actually makes sense
for common options to win over device type-specific equivalents.

Fixes: https://tracker.ceph.com/issues/47625
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 doc/man/8/rbd.rst              |  3 ++-
 qa/rbd/krbd_blkroset.t         |  9 +++++++++
 src/tools/rbd/action/Kernel.cc | 28 +++++++++++++++-------------
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/doc/man/8/rbd.rst b/doc/man/8/rbd.rst
index b81b6a7087d1f..cc920a1af2de3 100644
--- a/doc/man/8/rbd.rst
+++ b/doc/man/8/rbd.rst
@@ -762,7 +762,7 @@ Per client instance `rbd device map` options:
 
 Per mapping (block device) `rbd device map` options:
 
-* rw - Map the image read-write (default).
+* rw - Map the image read-write (default).  Overridden by --read-only.
 
 * ro - Map the image read-only.  Equivalent to --read-only.
 
@@ -772,6 +772,7 @@ Per mapping (block device) `rbd device map` options:
   discards (since 4.9).
 
 * exclusive - Disable automatic exclusive lock transitions (since 4.12).
+  Equivalent to --exclusive.
 
 * lock_timeout=x - A timeout on waiting for the acquisition of exclusive lock
   (since 4.17, default is 0 seconds, meaning no timeout).
diff --git a/qa/rbd/krbd_blkroset.t b/qa/rbd/krbd_blkroset.t
index 893fcc4d44eb7..e86c092f97003 100644
--- a/qa/rbd/krbd_blkroset.t
+++ b/qa/rbd/krbd_blkroset.t
@@ -343,6 +343,15 @@ rw -> ro with open_count > 0
   $ sudo rbd unmap $DEV
 
 
+"-o rw --read-only" should result in read-only mapping
+======================================================
+
+  $ DEV=$(sudo rbd map -o rw --read-only img)
+  $ blockdev --getro $DEV
+  1
+  $ sudo rbd unmap $DEV
+
+
 Teardown
 ========
 
diff --git a/src/tools/rbd/action/Kernel.cc b/src/tools/rbd/action/Kernel.cc
index dcbca088ff75b..deddcea3d01b5 100644
--- a/src/tools/rbd/action/Kernel.cc
+++ b/src/tools/rbd/action/Kernel.cc
@@ -499,30 +499,32 @@ int execute_map(const po::variables_map &vm,
   }
 
   MapOptions map_options;
-  if (vm["quiesce"].as<bool>()) {
-    std::cerr << "rbd: warning: quiesce is not supported" << std::endl;
+  if (vm.count("options")) {
+    for (auto &options : vm["options"].as<std::vector<std::string>>()) {
+      r = parse_map_options(options, &map_options);
+      if (r < 0) {
+        std::cerr << "rbd: couldn't parse map options" << std::endl;
+        return r;
+      }
+    }
   }
+
+  // parse options common to all device types after parsing krbd-specific
+  // options so that common options win (in particular "-o rw --read-only"
+  // should result in read-only mapping)
   if (vm["read-only"].as<bool>()) {
     put_map_option("rw", "ro", &map_options);
   }
   if (vm["exclusive"].as<bool>()) {
     put_map_option("exclusive", "exclusive", &map_options);
   }
-
+  if (vm["quiesce"].as<bool>()) {
+    std::cerr << "rbd: warning: quiesce is not supported" << std::endl;
+  }
   if (vm.count("quiesce-hook")) {
     std::cerr << "rbd: warning: quiesce-hook is not supported" << std::endl;
   }
 
-  if (vm.count("options")) {
-    for (auto &options : vm["options"].as<std::vector<std::string>>()) {
-      r = parse_map_options(options, &map_options);
-      if (r < 0) {
-        std::cerr << "rbd: couldn't parse map options" << std::endl;
-        return r;
-      }
-    }
-  }
-
   // connect to the cluster to get the default pool and the default map
   // options
   librados::Rados rados;
-- 
2.39.5