Merge pull request #7215 from trociny/wip-rbd-nbd-fixes

rbd-nbd: fix up return code handling

Reviewed-by: Jason Dillaman <dillaman@redhat.com>

qa/workunits/rbd/rbd-nbd.sh

@@ -1,42 +1,102 @@
 #!/bin/bash -ex
 
-pool=rbd
-gen=$pool/gen
-data=testfile
-size=64
-dev=/dev/nbd0
+. $(dirname $0)/../ceph-helpers.sh
 
-mkdir -p rbd_nbd_test
-pushd rbd_nbd_test
+POOL=rbd
+IMAGE=testrbdnbd$$
+SUDO=sudo
+SIZE=64
+DATA=
+DEV=
+
+setup()
+{
+    trap cleanup INT TERM EXIT
+    TEMPDIR=`mktemp -d`
+    DATA=${TEMPDIR}/data
+    dd if=/dev/urandom of=${DATA} bs=1M count=${SIZE}
+    rbd --dest-pool ${POOL} --no-progress import ${DATA} ${IMAGE}
+
+    if [ `id -u` = 0 ]
+    then
+	SUDO=
+    fi
+}
+
+function cleanup()
+{
+    set +e
+    rm -Rf ${TMPDIR}
+    if [ -n "${DEV}" ]
+    then
+	${SUDO} rbd-nbd unmap ${DEV}
+    fi
+    if rbd -p ${POOL} status ${IMAGE} 2>/dev/null; then
+	for s in 0.1 0.2 0.4 0.8 1.6 3.2 6.4 12.8; do
+	    sleep $s
+	    rbd -p ${POOL} status ${IMAGE} | grep 'Watchers: none' && break
+	done
+	rbd -p ${POOL} remove ${IMAGE}
+    fi
+}
 
 function expect_false()
 {
   if "$@"; then return 1; else return 0; fi
 }
 
-rbd remove $gen || true
-sudo rbd-nbd unmap $dev || true
+#
+# main
+#
+
+setup
+
+# exit status test
+expect_false rbd-nbd
+expect_false rbd-nbd INVALIDCMD
+if [ `id -u` -ne 0 ]
+then
+    expect_false rbd-nbd map ${IMAGE}
+fi
+expect_false ${SUDO} rbd-nbd map INVALIDIMAGE
+expect_false ${SUDO} rbd-nbd --device INVALIDDEV map ${IMAGE}
+
+# map test using the first unused device
+DEV=`${SUDO} rbd-nbd map ${POOL}/${IMAGE}`
+${SUDO} rbd-nbd list-mapped | grep "^${DEV}$"
+
+# map test specifying the device
+expect_false ${SUDO} rbd-nbd --device ${DEV} map ${POOL}/${IMAGE}
+dev1=${DEV}
+${SUDO} rbd-nbd unmap ${DEV}
+${SUDO} rbd-nbd list-mapped | expect_false grep "^${DEV}$"
+DEV=
+# XXX: race possible when the device is reused by other process
+DEV=`${SUDO} rbd-nbd --device ${dev1} map ${POOL}/${IMAGE}`
+[ "${DEV}" = "${dev1}" ]
+${SUDO} rbd-nbd list-mapped | grep "^${DEV}$"
 
 #read test
-dd if=/dev/urandom of=$data bs=1M count=$size
-rbd --no-progress import $data $gen
-sudo rbd-nbd --device $dev map $gen
-[ "`dd if=$data bs=1M | md5sum`" != "`sudo dd if=$dev bs=1M | md5sum`" ] && false
+[ "`dd if=${DATA} bs=1M | md5sum`" = "`${SUDO} dd if=${DEV} bs=1M | md5sum`" ]
 
 #write test
-dd if=/dev/urandom of=$data bs=1M count=$size
-sudo dd if=$data of=$dev bs=1M
+dd if=/dev/urandom of=${DATA} bs=1M count=${SIZE}
+${SUDO} dd if=${DATA} of=${DEV} bs=1M
 sync
-[ "`dd if=$data bs=1M | md5sum`" != "`rbd --no-progress export $gen - | md5sum`" ] && false
+[ "`dd if=${DATA} bs=1M | md5sum`" = "`rbd -p ${POOL} --no-progress export ${IMAGE} - | md5sum`" ]
 
 #trim test
-sudo mkfs.ext4 $dev # better idea?
+provisioned=`rbd -p ${POOL} --format xml du ${IMAGE} |
+  $XMLSTARLET sel -t -m "//stats/images/image/provisioned_size" -v .`
+used=`rbd -p ${POOL} --format xml du ${IMAGE} |
+  $XMLSTARLET sel -t -m "//stats/images/image/used_size" -v .`
+[ "${used}" -eq "${provisioned}" ]
+${SUDO} mkfs.ext4 -E discard ${DEV} # better idea?
 sync
-info=`rbd du $gen | tail -n 1`
-[ "`echo $info | awk '{print $2}'`" == "`echo $info | awk '{print $3}'`" ] && false
-
-sudo rbd-nbd unmap $dev
-popd
-rm -rf rbd_nbd_test
+provisioned=`rbd -p ${POOL} --format xml du ${IMAGE} |
+  $XMLSTARLET sel -t -m "//stats/images/image/provisioned_size" -v .`
+used=`rbd -p ${POOL} --format xml du ${IMAGE} |
+  $XMLSTARLET sel -t -m "//stats/images/image/used_size" -v .`
+[ "${used}" -lt "${provisioned}" ]
 
 echo OK

src/tools/rbd_nbd/rbd-nbd.cc

@@ -186,7 +186,8 @@ class NBDServer
 
     if (ret < 0) {
       ctx->reply.error = htonl(-ret);
-    } else if (ret != static_cast<int>(ctx->request.len)) {
+    } else if ((ctx->command == NBD_CMD_WRITE || ctx->command == NBD_CMD_READ)
+	       && ret != static_cast<int>(ctx->request.len)) {
       derr << __func__ << ": " << *ctx << ": unexpected return value: " << ret
 	   << " (" << ctx->request.len << " expected)" << dendl;
       ctx->reply.error = htonl(EIO);
@@ -485,15 +486,15 @@ static int do_map()
 
   if (global_init_prefork(g_ceph_context) >= 0) {
     std::string err;
-    if (forker.prefork(err) < 0) {
+    r = forker.prefork(err);
+    if (r < 0) {
       cerr << err << std::endl;
-      return EXIT_FAILURE;
+      return r;
     }
 
     if (forker.is_parent()) {
-      if (forker.parent_wait(err) < 0) {
-	cerr << err << std::endl;
-	return EXIT_FAILURE;
+      if (forker.parent_wait(err) != 0) {
+	return -ENXIO;
       }
       return 0;
     }