From f0c69761c8036a57319ead5cdf97cebb0f0fb5d7 Mon Sep 17 00:00:00 2001
From: Mykola Golub <mgolub@suse.com>
Date: Thu, 10 Sep 2020 14:08:42 +0100
Subject: [PATCH] qa/workunits/rbd: yet another attempt to improve rbd-nbd
 unmap

Previously it still could race when unmap_device returned success
because the device was not found in `rbd-nbd list-mapped` (the nbd
device was removed) but the test failed because the process was still
found in the ps table.

Fixes: https://tracker.ceph.com/issues/47394
Signed-off-by: Mykola Golub <mgolub@suse.com>
---
 qa/workunits/rbd/rbd-nbd.sh | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/qa/workunits/rbd/rbd-nbd.sh b/qa/workunits/rbd/rbd-nbd.sh
index ae4c093b004..c395de19680 100755
--- a/qa/workunits/rbd/rbd-nbd.sh
+++ b/qa/workunits/rbd/rbd-nbd.sh
@@ -98,18 +98,20 @@ function get_pid()
     PID=$(rbd-nbd --format xml list-mapped | $XMLSTARLET sel -t -v \
       "//devices/device[pool='${POOL}'][namespace='${ns}'][image='${IMAGE}'][device='${DEV}']/id")
     test -n "${PID}"
-    ps -p ${PID} -o cmd | grep rbd-nbd
+    ps -p ${PID} -C rbd-nbd
 }
 
 unmap_device()
 {
-    local unmap_dev=$1
-    local list_dev=${2:-$1}
-    _sudo rbd-nbd unmap ${unmap_dev}
+    local dev=$1
+    local pid=$2
+    _sudo rbd-nbd unmap ${dev}
 
     for s in 0.5 1 2 4 8 16 32; do
 	sleep ${s}
-        rbd-nbd list-mapped | expect_false grep "${list_dev} *$" && return 0
+        rbd-nbd list-mapped | expect_false grep "^${pid}\\b" &&
+            ! ps -p ${pid} -C rbd-nbd &&
+            return 0
     done
     return 1
 }
@@ -141,7 +143,7 @@ get_pid
 # map test specifying the device
 expect_false _sudo rbd-nbd --device ${DEV} map ${POOL}/${IMAGE}
 dev1=${DEV}
-unmap_device ${DEV}
+unmap_device ${DEV} ${PID}
 DEV=
 # XXX: race possible when the device is reused by other process
 DEV=`_sudo rbd-nbd --device ${dev1} map ${POOL}/${IMAGE}`
@@ -186,16 +188,16 @@ test -n "${blocks2}"
 test ${blocks2} -eq ${blocks}
 
 # read-only option test
-unmap_device ${DEV}
+unmap_device ${DEV} ${PID}
 DEV=`_sudo rbd-nbd map --read-only ${POOL}/${IMAGE}`
 PID=$(rbd-nbd list-mapped | awk -v pool=${POOL} -v img=${IMAGE} -v dev=${DEV} \
     '$2 == pool && $3 == img && $5 == dev {print $1}')
 test -n "${PID}"
-ps -p ${PID} -o cmd | grep rbd-nbd
+ps -p ${PID} -C rbd-nbd
 
 _sudo dd if=${DEV} of=/dev/null bs=1M
 expect_false _sudo dd if=${DATA} of=${DEV} bs=1M oflag=direct
-unmap_device ${DEV}
+unmap_device ${DEV} ${PID}
 
 # exclusive option test
 DEV=`_sudo rbd-nbd map --exclusive ${POOL}/${IMAGE}`
@@ -204,32 +206,29 @@ get_pid
 _sudo dd if=${DATA} of=${DEV} bs=1M oflag=direct
 expect_false timeout 10 \
 	rbd bench ${IMAGE} --io-type write --io-size=1024 --io-total=1024
-unmap_device ${DEV}
+unmap_device ${DEV} ${PID}
 DEV=
 rbd bench ${IMAGE} --io-type write --io-size=1024 --io-total=1024
 
 # unmap by image name test
 DEV=`_sudo rbd-nbd map ${POOL}/${IMAGE}`
 get_pid
-unmap_device ${IMAGE} ${DEV}
+unmap_device ${IMAGE} ${PID}
 DEV=
-ps -p ${PID} -o cmd | expect_false grep rbd-nbd
 
 # map/unmap snap test
 rbd snap create ${POOL}/${IMAGE}@snap
 DEV=`_sudo rbd-nbd map ${POOL}/${IMAGE}@snap`
 get_pid
-unmap_device "${IMAGE}@snap" ${DEV}
+unmap_device "${IMAGE}@snap" ${PID}
 DEV=
-ps -p ${PID} -o cmd | expect_false grep rbd-nbd
 
 # map/unmap namespace test
 rbd snap create ${POOL}/${NS}/${IMAGE}@snap
 DEV=`_sudo rbd-nbd map ${POOL}/${NS}/${IMAGE}@snap`
 get_pid ${NS}
-unmap_device "${POOL}/${NS}/${IMAGE}@snap" ${DEV}
+unmap_device "${POOL}/${NS}/${IMAGE}@snap" ${PID}
 DEV=
-ps -p ${PID} -o cmd | expect_false grep rbd-nbd
 
 # auto unmap test
 DEV=`_sudo rbd-nbd map ${POOL}/${IMAGE}`
@@ -244,6 +243,7 @@ rbd-nbd list-mapped | expect_false grep "^${PID} *${POOL} *${IMAGE}"
 # quiesce test
 QUIESCE_HOOK=${TEMPDIR}/quiesce.sh
 DEV=`_sudo rbd-nbd map --quiesce --quiesce-hook ${QUIESCE_HOOK} ${POOL}/${IMAGE}`
+get_pid
 
 # test it does not fail if the hook does not exists
 test ! -e ${QUIESCE_HOOK}
@@ -284,7 +284,7 @@ rbd snap create ${POOL}/${IMAGE}@quiesce4
 _sudo dd if=${DATA} of=${DEV} bs=1M count=1 oflag=direct
 
 # test rbd-nbd_quiesce hook that comes with distribution
-unmap_device ${DEV}
+unmap_device ${DEV} ${PID}
 LOG_FILE=${TEMPDIR}/rbd-nbd.log
 if [ -n "${CEPH_SRC}" ]; then
     QUIESCE_HOOK=${CEPH_SRC}/tools/rbd_nbd/rbd-nbd_quiesce
@@ -293,13 +293,14 @@ if [ -n "${CEPH_SRC}" ]; then
 else
     DEV=`_sudo rbd-nbd map --quiesce ${POOL}/${IMAGE} --log-file=${LOG_FILE}`
 fi
+get_pid
 _sudo mkfs ${DEV}
 mkdir ${TEMPDIR}/mnt
 _sudo mount ${DEV} ${TEMPDIR}/mnt
 rbd snap create ${POOL}/${IMAGE}@quiesce5
 _sudo dd if=${DATA} of=${TEMPDIR}/mnt/test bs=1M count=1 oflag=direct
 _sudo umount ${TEMPDIR}/mnt
-unmap_device ${DEV}
+unmap_device ${DEV} ${PID}
 cat ${LOG_FILE}
 expect_false grep 'quiesce failed' ${LOG_FILE}
 
-- 
2.39.5