From: Mykola Golub <mgolub@suse.com>
Date: Sun, 4 Oct 2020 17:41:53 +0000 (+0100)
Subject: rbd-nbd: make unmap/detach wait for rbd-nbd process to terminate
X-Git-Tag: v16.1.0~754^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3043d801686120e1589f84f140f308ff37c0c1cc;p=ceph.git

rbd-nbd: make unmap/detach wait for rbd-nbd process to terminate

Signed-off-by: Mykola Golub <mgolub@suse.com>
---

diff --git a/qa/workunits/rbd/rbd-nbd.sh b/qa/workunits/rbd/rbd-nbd.sh
index fce36921214..879094dc92b 100755
--- a/qa/workunits/rbd/rbd-nbd.sh
+++ b/qa/workunits/rbd/rbd-nbd.sh
@@ -97,7 +97,7 @@ 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}"
+    test -n "${PID}" || return 1
     ps -p ${PID} -C rbd-nbd
 }
 
@@ -105,15 +105,13 @@ unmap_device()
 {
     local dev=$1
     local pid=$2
+
     _sudo rbd-nbd unmap ${dev}
+    rbd-nbd list-mapped | expect_false grep "^${pid}\\b" || return 1
+    ps -C rbd-nbd | expect_false grep "^${pid}\\b" || return 1
 
-    for s in 0.5 1 2 4 8 16 32; do
-	sleep ${s}
-        rbd-nbd list-mapped | expect_false grep "^${pid}\\b" &&
-            ps -C rbd-nbd | expect_false grep "^${pid}\\b" &&
-            return 0
-    done
-    return 1
+    # workaround possible race between unmap and following map
+    sleep 0.5
 }
 
 #
@@ -320,17 +318,8 @@ DEV=`_sudo rbd-nbd map --try-netlink ${POOL}/${IMAGE}`
 get_pid
 _sudo mount ${DEV} ${TEMPDIR}/mnt
 _sudo rbd-nbd detach ${POOL}/${IMAGE}
-attached=
-for i in `seq 10`; do
-    if _sudo rbd-nbd attach --device ${DEV} ${POOL}/${IMAGE}; then
-        attached=1
-        break
-    fi
-    rbd-nbd list-mapped
-    ps auxww | grep rbd-nbd
-    sleep 1
-done
-test "${attached}" = 1
+expect_false get_pid
+_sudo rbd-nbd attach --device ${DEV} ${POOL}/${IMAGE}
 get_pid
 ls ${TEMPDIR}/mnt/
 dd if=${TEMPDIR}/mnt/test of=/dev/null bs=1M count=1
diff --git a/src/tools/rbd_nbd/rbd-nbd.cc b/src/tools/rbd_nbd/rbd-nbd.cc
index 13ae55fe88b..80737de19b5 100644
--- a/src/tools/rbd_nbd/rbd-nbd.cc
+++ b/src/tools/rbd_nbd/rbd-nbd.cc
@@ -1420,6 +1420,57 @@ static void run_server(Preforker& forker, NBDServer *server, bool netlink_used)
   shutdown_async_signal_handler();
 }
 
+// Eventually it should be replaced with glibc' pidfd_open
+// when it is widely available.
+static int
+pidfd_open(pid_t pid, unsigned int)
+{
+  std::string path = "/proc/" + stringify(pid);
+  int fd = open(path.c_str(), O_RDONLY);
+  if (fd == -1 && errno == ENOENT) {
+    errno = ESRCH;
+  }
+
+  return fd;
+}
+
+static int wait_for_terminate(int pid, int timeout)
+{
+  int fd = pidfd_open(pid, 0);
+  if (fd == -1) {
+    if (errno == -ESRCH) {
+      return 0;
+    }
+    int r = -errno;
+    cerr << "rbd-nbd: pidfd_open(" << pid << ") failed: "
+         << cpp_strerror(r) << std::endl;
+    return r;
+  }
+
+  struct pollfd poll_fds[1];
+  memset(poll_fds, 0, sizeof(struct pollfd));
+  poll_fds[0].fd = fd;
+  poll_fds[0].events = POLLIN;
+
+  int r = poll(poll_fds, 1, timeout * 1000);
+  if (r == -1) {
+    r = -errno;
+    cerr << "rbd-nbd: failed to poll rbd-nbd process: " << cpp_strerror(r)
+         << std::endl;
+    goto done;
+  }
+
+  if ((poll_fds[0].revents & POLLIN) == 0) {
+    cerr << "rbd-nbd: waiting for process exit timed out" << std::endl;
+    r = -ETIMEDOUT;
+  }
+
+done:
+  close(fd);
+
+  return r;
+}
+
 static int do_map(int argc, const char *argv[], Config *cfg, bool reconnect)
 {
   int r;
@@ -1625,14 +1676,14 @@ close_ret:
 static int do_detach(Config *cfg)
 {
   int r = kill(cfg->pid, SIGTERM);
-  if (r != 0) {
+  if (r == -1) {
     r = -errno;
     cerr << "rbd-nbd: failed to terminate " << cfg->pid << ": "
          << cpp_strerror(r) << std::endl;
     return r;
   }
 
-  return 0;
+  return wait_for_terminate(cfg->pid, cfg->timeout);
 }
 
 static int do_unmap(Config *cfg)
@@ -1659,7 +1710,12 @@ static int do_unmap(Config *cfg)
   }
 
   close(nbd);
-  return r;
+
+  if (r < 0) {
+    return r;
+  }
+
+  return wait_for_terminate(cfg->pid, cfg->timeout);
 }
 
 static int parse_imgpath(const std::string &imgpath, Config *cfg,