Merge pull request ceph#50593 from pkalever/fix-feature-disable

rbd-nbd: fix stuck with disable request

Reviewed-by: Mykola Golub <[email protected]>
Reviewed-by: Ilya Dryomov <[email protected]>

Author: idryomov

qa/workunits/rbd/rbd-nbd.sh

@@ -472,6 +472,16 @@ DEV=
 rbd feature disable ${POOL}/${IMAGE} journaling
 rbd config image rm ${POOL}/${IMAGE} rbd_discard_granularity_bytes
 
+# test that disabling a feature so that the op is proxied to rbd-nbd
+# (arranged here by blkdiscard before "rbd feature disable") doesn't hang
+DEV=`_sudo rbd device --device-type nbd map ${POOL}/${IMAGE}`
+get_pid ${POOL}
+rbd feature enable ${POOL}/${IMAGE} journaling
+_sudo blkdiscard --offset 0 --length 4096 ${DEV}
+rbd feature disable ${POOL}/${IMAGE} journaling
+unmap_device ${DEV} ${PID}
+DEV=
+
 # test that rbd_op_threads setting takes effect
 EXPECTED=`ceph-conf --show-config-value librados_thread_count`
 DEV=`_sudo rbd device --device-type nbd map ${POOL}/${IMAGE}`

src/test/librbd/fsx.cc

@@ -1162,6 +1162,8 @@ int
 krbd_resize(struct rbd_ctx *ctx, uint64_t size)
 {
 	int ret;
+	int count = 0;
+	uint64_t effective_size;
 
 	ceph_assert(size % truncbdy == 0);
 
@@ -1183,7 +1185,29 @@ krbd_resize(struct rbd_ctx *ctx, uint64_t size)
 	if (ret < 0)
 		return ret;
 
-	return __librbd_resize(ctx, size);
+	ret = __librbd_resize(ctx, size);
+	if (ret < 0)
+		return ret;
+
+	for (;;) {
+		ret = krbd_get_size(ctx, &effective_size);
+		if (ret < 0)
+			return ret;
+
+		if (effective_size == size)
+			break;
+
+		if (count++ >= 15) {
+			prt("BLKGETSIZE64 size error: expected 0x%llx, actual 0x%llx\n",
+			    (unsigned long long)size,
+			    (unsigned long long)effective_size);
+			return -EINVAL;
+		}
+
+		usleep(count * 250 * 1000);
+	}
+
+	return 0;
 }
 
 int

src/tools/rbd_nbd/rbd-nbd.cc

@@ -738,7 +738,67 @@ class NBDWatchCtx : public librbd::UpdateWatchCtx
   bool use_netlink;
   librados::IoCtx &io_ctx;
   librbd::Image ℑ
-  unsigned long size;
+  uint64_t size;
+  std::thread handle_notify_thread;
+  ceph::condition_variable cond;
+  ceph::mutex lock = ceph::make_mutex("NBDWatchCtx::Locker");
+  bool notify = false;
+  bool terminated = false;
+
+  bool wait_notify() {
+    dout(10) << __func__ << dendl;
+
+    std::unique_lock locker{lock};
+    cond.wait(locker, [this] { return notify || terminated; });
+
+    if (terminated) {
+      return false;
+    }
+
+    dout(10) << __func__ << ": got notify request" << dendl;
+    notify = false;
+    return true;
+  }
+
+  void handle_notify_entry() {
+    dout(10) << __func__ << dendl;
+
+    while (wait_notify()) {
+      uint64_t new_size;
+      int ret = image.size(&new_size);
+      if (ret < 0) {
+        derr << "getting image size failed: " << cpp_strerror(ret) << dendl;
+        continue;
+      }
+      if (new_size == size) {
+        continue;
+      }
+      dout(5) << "resize detected" << dendl;
+      if (ioctl(fd, BLKFLSBUF, NULL) < 0) {
+        derr << "invalidate page cache failed: " << cpp_strerror(errno)
+             << dendl;
+      }
+      if (use_netlink) {
+        ret = netlink_resize(nbd_index, new_size);
+      } else {
+        ret = ioctl(fd, NBD_SET_SIZE, new_size);
+        if (ret < 0) {
+          derr << "resize failed: " << cpp_strerror(errno) << dendl;
+        }
+      }
+      if (!ret) {
+        size = new_size;
+      }
+      if (ioctl(fd, BLKRRPART, NULL) < 0) {
+        derr << "rescan of partition table failed: " << cpp_strerror(errno)
+             << dendl;
+      }
+      if (image.invalidate_cache() < 0) {
+        derr << "invalidate rbd cache failed" << dendl;
+      }
+    }
+  }
+
 public:
   NBDWatchCtx(int _fd,
               int _nbd_index,
@@ -752,41 +812,31 @@ class NBDWatchCtx : public librbd::UpdateWatchCtx
     , io_ctx(_io_ctx)
     , image(_image)
     , size(_size)
-  { }
+  {
+    handle_notify_thread = make_named_thread("rbd_handle_notify",
+                                             &NBDWatchCtx::handle_notify_entry,
+                                             this);
+  }
 
-  ~NBDWatchCtx() override {}
+  ~NBDWatchCtx() override
+  {
+    dout(10) << __func__ << ": terminating" << dendl;
+    std::unique_lock locker{lock};
+    terminated = true;
+    cond.notify_all();
+    locker.unlock();
+
+    handle_notify_thread.join();
+    dout(10) << __func__ << ": finish" << dendl;
+  }
 
   void handle_notify() override
   {
-    librbd::image_info_t info;
-    if (image.stat(info, sizeof(info)) == 0) {
-      unsigned long new_size = info.size;
-      int ret;
-
-      if (new_size != size) {
-        dout(5) << "resize detected" << dendl;
-        if (ioctl(fd, BLKFLSBUF, NULL) < 0)
-          derr << "invalidate page cache failed: " << cpp_strerror(errno)
-               << dendl;
-	if (use_netlink) {
-	  ret = netlink_resize(nbd_index, new_size);
-	} else {
-          ret = ioctl(fd, NBD_SET_SIZE, new_size);
-          if (ret < 0)
-            derr << "resize failed: " << cpp_strerror(errno) << dendl;
-	}
-
-        if (!ret)
-          size = new_size;
+    dout(10) << __func__ << dendl;
 
-        if (ioctl(fd, BLKRRPART, NULL) < 0) {
-          derr << "rescan of partition table failed: " << cpp_strerror(errno)
-               << dendl;
-        }
-        if (image.invalidate_cache() < 0)
-          derr << "invalidate rbd cache failed" << dendl;
-      }
-    }
+    std::unique_lock locker{lock};
+    notify = true;
+    cond.notify_all();
   }
 };