krbd: separate event reaping from event processing

Move event processing into UdevMapHandler and UdevUnmapHandler
functors and replace wait_for_udev_{add,remove}() with a single
wait_for_mapping() template.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit c84f9e2f2df47361d7a928d0b25cb84ef332c055)

Conflicts:
	src/krbd.cc [ krbd_spec not in luminous ]

diff --git a/src/krbd.cc b/src/krbd.cc
index b2a58c8d64aa563049e43aa06a85cf1c3951c244..c68f38cefd5c33ecdc403c4d8233448b1f4a6f38 100644 (file)
--- a/src/krbd.cc
+++ b/src/krbd.cc
@@ -174,91 +174,107 @@ static int build_map_buf(CephContext *cct, const char *pool, const char *image,
   return 0;
 }
 
-static int wait_for_udev_add(struct udev_monitor *mon, const char *pool,
-                             const char *image, const char *snap,
-                             string *pname)
+template <typename F>
+static int wait_for_mapping(udev_monitor *mon, F udev_device_handler)
 {
-  struct udev_device *bus_dev = NULL;
-  std::vector<struct udev_device*> block_dev_vec;
+  struct pollfd fds[1];
+
+  fds[0].fd = udev_monitor_get_fd(mon);
+  fds[0].events = POLLIN;
 
-  /*
-   * Catch /sys/devices/rbd/<id>/ and wait for the corresponding
-   * block device to show up.  This is necessary because rbd devices
-   * and block devices aren't linked together in our sysfs layout.
-   */
   for (;;) {
-    struct pollfd fds[1];
     struct udev_device *dev;
 
-    fds[0].fd = udev_monitor_get_fd(mon);
-    fds[0].events = POLLIN;
     if (poll(fds, 1, -1) < 0) {
       ceph_abort();
     }
 
     dev = udev_monitor_receive_device(mon);
-    if (!dev)
+    if (!dev) {
       continue;
+    }
+    if (udev_device_handler(dev)) {
+      return 0;
+    }
+  }
+}
 
-    if (strcmp(udev_device_get_action(dev), "add") != 0)
-      goto next;
+class UdevMapHandler {
+public:
+  UdevMapHandler(const char *pool, const char *image, const char *snap,
+                 std::string *pdevnode) :
+      m_pool(pool), m_image(image), m_snap(snap), m_pdevnode(pdevnode) {}
 
-    if (strcmp(udev_device_get_subsystem(dev), "rbd") == 0) {
-      if (!bus_dev) {
-        const char *this_pool = udev_device_get_sysattr_value(dev, "pool");
-        const char *this_image = udev_device_get_sysattr_value(dev, "name");
-        const char *this_snap = udev_device_get_sysattr_value(dev,
-                                                              "current_snap");
-
-        if (this_pool && strcmp(this_pool, pool) == 0 &&
-            this_image && strcmp(this_image, image) == 0 &&
-            this_snap && strcmp(this_snap, snap) == 0) {
-          bus_dev = dev;
+  /*
+   * Catch /sys/devices/rbd/<id>/ and wait for the corresponding
+   * block device to show up.  This is necessary because rbd devices
+   * and block devices aren't linked together in our sysfs layout.
+   */
+  bool operator()(udev_device *dev) {
+    if (strcmp(udev_device_get_action(dev), "add")) {
+      goto next;
+    }
+    if (!strcmp(udev_device_get_subsystem(dev), "rbd")) {
+      if (!m_bus_dev) {
+        const char *pool = udev_device_get_sysattr_value(dev, "pool");
+        const char *image = udev_device_get_sysattr_value(dev, "name");
+        const char *snap = udev_device_get_sysattr_value(dev, "current_snap");
+
+        if (pool && strcmp(pool, m_pool) == 0 &&
+            image && strcmp(image, m_image) == 0 &&
+            snap && strcmp(snap, m_snap) == 0) {
+          m_bus_dev = dev;
           goto check;
         }
       }
-    } else if (strcmp(udev_device_get_subsystem(dev), "block") == 0) {
-      block_dev_vec.push_back(dev);
+    } else if (!strcmp(udev_device_get_subsystem(dev), "block")) {
+      m_block_devs.push_back(dev);
       goto check;
     }
 
 next:
     udev_device_unref(dev);
-    continue;
+    return false;
 
 check:
-    if (bus_dev && !block_dev_vec.empty()) {
-      const char *major = udev_device_get_sysattr_value(bus_dev, "major");
-      const char *minor = udev_device_get_sysattr_value(bus_dev, "minor");
+    if (m_bus_dev && !m_block_devs.empty()) {
+      const char *major = udev_device_get_sysattr_value(m_bus_dev, "major");
+      const char *minor = udev_device_get_sysattr_value(m_bus_dev, "minor");
       assert(!minor ^ have_minor_attr());
 
-      for (auto p : block_dev_vec) {
+      for (auto p : m_block_devs) {
         const char *this_major = udev_device_get_property_value(p, "MAJOR");
         const char *this_minor = udev_device_get_property_value(p, "MINOR");
 
         if (strcmp(this_major, major) == 0 &&
             (!minor || strcmp(this_minor, minor) == 0)) {
-          string name = get_kernel_rbd_name(udev_device_get_sysname(bus_dev));
+          string name = get_kernel_rbd_name(udev_device_get_sysname(m_bus_dev));
 
           assert(strcmp(udev_device_get_devnode(p), name.c_str()) == 0);
-          *pname = name;
-          goto done;
+          *m_pdevnode = name;
+          return true;
         }
       }
     }
+    return false;
   }
 
-done:
-  if (bus_dev) {
-    udev_device_unref(bus_dev);
-  }
-  
-  for (auto p : block_dev_vec) {
-    udev_device_unref(p);
+  ~UdevMapHandler() {
+    if (m_bus_dev) {
+      udev_device_unref(m_bus_dev);
+    }
+
+    for (auto p : m_block_devs) {
+      udev_device_unref(p);
+    }
   }
 
-  return 0;
-}
+private:
+  udev_device *m_bus_dev = nullptr;
+  std::vector<udev_device *> m_block_devs;
+  const char *m_pool, *m_image, *m_snap;
+  std::string *m_pdevnode;
+};
 
 static int do_map(struct udev *udev, const char *pool, const char *image,
                   const char *snap, const string& buf, string *pname)
@@ -288,7 +304,7 @@ static int do_map(struct udev *udev, const char *pool, const char *image,
     goto out_mon;
   }
 
-  r = wait_for_udev_add(mon, pool, image, snap, pname);
+  r = wait_for_mapping(mon, UdevMapHandler(pool, image, snap, pname));
   if (r < 0) {
     cerr << "rbd: wait failed" << std::endl;
     goto out_mon;
@@ -482,33 +498,24 @@ static string build_unmap_buf(const string& id, const char *options)
   return buf;
 }
 
-static int wait_for_udev_remove(struct udev_monitor *mon, dev_t devno)
-{
-  for (;;) {
-    struct pollfd fds[1];
-    struct udev_device *dev;
+class UdevUnmapHandler {
+public:
+  UdevUnmapHandler(dev_t devno) : m_devno(devno) {}
 
-    fds[0].fd = udev_monitor_get_fd(mon);
-    fds[0].events = POLLIN;
-    if (poll(fds, 1, -1) < 0) {
-      ceph_abort();
-    }
+  bool operator()(udev_device *dev) {
+    bool match = false;
 
-    dev = udev_monitor_receive_device(mon);
-    if (!dev)
-      continue;
-
-    if (strcmp(udev_device_get_action(dev), "remove") == 0 &&
-        udev_device_get_devnum(dev) == devno) {
-      udev_device_unref(dev);
-      break;
+    if (!strcmp(udev_device_get_action(dev), "remove") &&
+        udev_device_get_devnum(dev) == m_devno) {
+      match = true;
     }
-
     udev_device_unref(dev);
+    return match;
   }
 
-  return 0;
-}
+private:
+  dev_t m_devno;
+};
 
 static int do_unmap(struct udev *udev, dev_t devno, const string& buf)
 {
@@ -555,7 +562,7 @@ static int do_unmap(struct udev *udev, dev_t devno, const string& buf)
     }
   }
 
-  r = wait_for_udev_remove(mon, devno);
+  r = wait_for_mapping(mon, UdevUnmapHandler(devno));
   if (r < 0) {
     cerr << "rbd: wait failed" << std::endl;
     goto out_mon;