From fca5ef8aba28e5b3d75c56558766e05b2d07f523 Mon Sep 17 00:00:00 2001
From: Marc Schoechlin <marc.schoechlin@vico-research.com>
Date: Fri, 5 Oct 2018 13:05:12 +0200
Subject: [PATCH] common: fix for broken rbdmap parameter parsing

This improves the broken parameter parsing discussed in #36327

Test expression for the parsing function:

PARAMS="options='lock_on_read,queue_depth=1024',id=archiv,keyring=/etc/ceph/ceph.client.archiv.keyring"
echo -e " ORIGINAL PARAM: >>>$PARAMS<<<\nCONVERTED PARAM: >>>$(create_cmd_params "$PARAMS")<<<"

PARAMS="id=archiv,keyring=/etc/ceph/ceph.client.archiv.keyring,options='lock_on_read,queue_depth=1024'"
echo -e " ORIGINAL PARAM: >>>$PARAMS<<<\nCONVERTED PARAM: >>>$(create_cmd_params "$PARAMS")<<<"

PARAMS=""
echo -e " ORIGINAL PARAM: >>>$PARAMS<<<\nCONVERTED PARAM: >>>$(create_cmd_params "$PARAMS")<<<"

PARAMS=",keyring=/etc/ceph/ceph.client.archiv.keyring,options='lock_on_read,queue_depth=1024',id=archiv,"
echo -e " ORIGINAL PARAM: >>>$PARAMS<<<\nCONVERTED PARAM: >>>$(create_cmd_params "$PARAMS")<<<"

PARAMS="'keyring'=/etc/ceph/ceph.client.archiv.keyring,options='lock_on_read,queue_depth=1024',id=archiv,"
echo -e " ORIGINAL PARAM: >>>$PARAMS<<<\nCONVERTED PARAM: >>>$(create_cmd_params "$PARAMS")<<<"

PARAMS="--keyring=/etc/ceph/ceph.client.archiv.keyring,options='lock_on_read,queue_depth=1024',id=archiv, # a comment"
echo -e " ORIGINAL PARAM: >>>$PARAMS<<<\nCONVERTED PARAM: >>>$(create_cmd_params "$PARAMS")<<<"

Fixes: https://tracker.ceph.com/issues/36327
Signed-off-by: Marc Schoechlin <ms@256bit.org]>
---
 doc/man/8/rbdmap.rst |  9 +++++--
 src/rbdmap           | 57 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/doc/man/8/rbdmap.rst b/doc/man/8/rbdmap.rst
index ba8001ff83fb4..e6980ab7e326c 100644
--- a/doc/man/8/rbdmap.rst
+++ b/doc/man/8/rbdmap.rst
@@ -46,6 +46,8 @@ This will cause the script to issue an ``rbd map`` command like the following::
     rbd map POOLNAME/IMAGENAME --PARAM1 VAL1 --PARAM2 VAL2 
 
 (See the ``rbd`` manpage for a full list of possible options.)
+For parameters and values which contain commas or equality signs, a simple
+apostrophe can be used to prevent replacing them.
 
 When run as ``rbdmap map``, the script parses the configuration file, and for
 each RBD image specified attempts to first map the image (using the ``rbd map``
@@ -77,11 +79,12 @@ sequence.)
 Examples
 ========
 
-Example ``/etc/ceph/rbdmap`` for two RBD images called "bar1" and "bar2", both
-in pool "foopool"::
+Example ``/etc/ceph/rbdmap`` for three RBD images called "bar1", "bar2" and "bar3", 
+which are in pool "foopool"::
 
     foopool/bar1    id=admin,keyring=/etc/ceph/ceph.client.admin.keyring
     foopool/bar2    id=admin,keyring=/etc/ceph/ceph.client.admin.keyring
+    foopool/bar3    id=admin,keyring=/etc/ceph/ceph.client.admin.keyring,options='lock_on_read,queue_depth=1024'
 
 Each line in the file contains two strings: the image spec and the options to
 be passed to ``rbd map``. These two lines get transformed into the following
@@ -89,12 +92,14 @@ commands::
 
     rbd map foopool/bar1 --id admin --keyring /etc/ceph/ceph.client.admin.keyring
     rbd map foopool/bar2 --id admin --keyring /etc/ceph/ceph.client.admin.keyring
+    rbd map foopool/bar2 --id admin --keyring /etc/ceph/ceph.client.admin.keyring --options lock_on_read,queue_depth=1024
 
 If the images had XFS filesystems on them, the corresponding ``/etc/fstab``
 entries might look like this::
 
     /dev/rbd/foopool/bar1 /mnt/bar1 xfs noauto 0 0
     /dev/rbd/foopool/bar2 /mnt/bar2 xfs noauto 0 0
+    /dev/rbd/foopool/bar3 /mnt/bar3 xfs noauto 0 0
 
 After creating the images and populating the ``/etc/ceph/rbdmap`` file, making
 the images get automatically mapped and mounted at boot is just a matter of
diff --git a/src/rbdmap b/src/rbdmap
index 2b13517491f2c..fa0cf7b821c62 100755
--- a/src/rbdmap
+++ b/src/rbdmap
@@ -1,5 +1,52 @@
 #!/usr/bin/env bash
 
+create_cmd_params() {
+	local PARAMS="$1"
+	local CMDPARAMS=""
+	local STATE="START"
+	for (( i=0; i<${#PARAMS}; i++ )); do
+		CHAR="${PARAMS:$i:1}"
+		case $CHAR in
+			"#")
+				break
+				;;
+			"'")
+				if [ "$STATE" == "INQUOTE" ];then
+					STATE="NORMAL"
+				else
+					STATE="INQUOTE"
+				fi
+				;;
+			"=")
+				if [ "$STATE" == "INQUOTE" ]; then
+					CMDPARAMS="${CMDPARAMS}${CHAR}"
+				else
+					CMDPARAMS="${CMDPARAMS} "
+				fi
+				;;
+			",")
+				if [ "$STATE" == "INQUOTE" ]; then
+					CMDPARAMS="${CMDPARAMS}${CHAR}"
+				elif [ "$STATE" == "START" ]; then
+					STATE="NORMAL"
+					CMDPARAMS="${CMDPARAMS} --"
+				else
+					CMDPARAMS="${CMDPARAMS} --"
+				fi
+				;;
+			*)
+				if [ "$STATE" == "START" ];then
+					STATE="NORMAL"
+					CMDPARAMS="${CMDPARAMS}--${CHAR}"
+				else
+					CMDPARAMS="${CMDPARAMS}${CHAR}"
+				fi
+				;;
+		esac
+	done
+	echo -n "$CMDPARAMS"
+}
+
 do_map() {
 	# Read /etc/rbdtab to create non-existant mapping
 	RET=0
@@ -14,16 +61,12 @@ do_map() {
 			DEV=rbd/$DEV
 			;;
 		esac
+
+		CMDPARAMS="$(create_cmd_params "${PARAMS}")"
 		logger -p "daemon.debug" -t rbdmap "Mapping '${DEV}'"
 		newrbd=""
 		MAP_RV=""
-		OIFS=$IFS
-		IFS=','
-		CMDPARAMS=""
-		for PARAM in ${PARAMS[@]}; do
-			CMDPARAMS="$CMDPARAMS --$(echo $PARAM | tr '=' ' ')"
-		done
-		IFS=$OIFS
+
 		if [ -b /dev/rbd/$DEV ]; then
 			MAP_RV="$(readlink -f /dev/rbd/$DEV)"
 		else
-- 
2.39.5