Merge pull request ceph#57697 from petrutlucian94/sv_wait

rbd-wnbd: wait for the disk cleanup to complete

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

Author: idryomov

src/common/win32/service.cc

@@ -86,6 +86,8 @@ void ServiceBase::shutdown(bool ignore_errors)
   DWORD original_state = status.dwCurrentState;
   set_status(SERVICE_STOP_PENDING);
 
+  dout(0) << "Shutdown requested." << dendl;
+
   int err = shutdown_hook();
   if (err) {
     derr << "Shutdown service hook failed. Error code: " << err << dendl;
@@ -108,6 +110,8 @@ void ServiceBase::stop()
   DWORD original_state = status.dwCurrentState;
   set_status(SERVICE_STOP_PENDING);
 
+  dout(0) << "Service stop requested." << dendl;
+
   int err = stop_hook();
   if (err) {
     derr << "Service stop hook failed. Error code: " << err << dendl;

src/tools/rbd_wnbd/rbd_mapping.cc

@@ -232,6 +232,12 @@ int RbdMappingDispatcher::create(Config& cfg)
     return -EEXIST;
   }
 
+  if (stop_requested) {
+    derr << "service stop requested, refusing to create new mapping."
+         << dendl;
+    return -ESHUTDOWN;
+  }
+
   auto rbd_mapping = std::make_shared<RbdMapping>(
     cfg, client_cache,
     std::bind(
@@ -282,3 +288,129 @@ void RbdMappingDispatcher::disconnect_cbk(std::string devpath, int ret)
     mappings.erase(devpath);
   }
 }
+
+int RbdMappingDispatcher::stop(
+  bool hard_disconnect,
+  int soft_disconnect_timeout,
+  int worker_count)
+{
+  stop_requested = true;
+
+  // Although not generally recommended, soft_disconnect_timeout can be 0,
+  // which means infinite timeout.
+  ceph_assert(soft_disconnect_timeout >= 0);
+  ceph_assert(worker_count > 0);
+
+  std::atomic<int> err = 0;
+
+  boost::asio::thread_pool pool(worker_count);
+  LARGE_INTEGER start_t, counter_freq;
+  QueryPerformanceFrequency(&counter_freq);
+  QueryPerformanceCounter(&start_t);
+
+  // We're initiating the disk removal through libwnbd, which uses PNP
+  // to notify the storage stack that the disk is going to be removed
+  // and waits for pending operations to complete.
+  //
+  // Once ready, the WNBD driver notifies rbd-wnbd that the disk has been
+  // disconnected.
+  auto mapped_devpaths = get_mapped_devpaths();
+  for (const auto& devpath: mapped_devpaths) {
+    boost::asio::post(pool,
+      [devpath, start_t, counter_freq, soft_disconnect_timeout,
+       hard_disconnect, &err]()
+      {
+        LARGE_INTEGER curr_t, elapsed_ms;
+        QueryPerformanceCounter(&curr_t);
+        elapsed_ms.QuadPart = curr_t.QuadPart - start_t.QuadPart;
+        elapsed_ms.QuadPart *= 1000;
+        elapsed_ms.QuadPart /= counter_freq.QuadPart;
+
+        int64_t time_left_ms = std::max(
+          (int64_t)0,
+          soft_disconnect_timeout * 1000 - elapsed_ms.QuadPart);
+
+        dout(1) << "Removing mapping: " << devpath
+                << ". Timeout: " << time_left_ms
+                << "ms. Hard disconnect: " << hard_disconnect
+                << dendl;
+
+        WNBD_REMOVE_OPTIONS remove_options = {0};
+        remove_options.Flags.HardRemove = hard_disconnect || !time_left_ms;
+        remove_options.Flags.HardRemoveFallback = true;
+        remove_options.SoftRemoveTimeoutMs = time_left_ms;
+        remove_options.SoftRemoveRetryIntervalMs = SOFT_REMOVE_RETRY_INTERVAL * 1000;
+
+        // This is asynchronous, it may take a few seconds for the disk to be
+        // removed. We'll perform the wait outside the loop to speed up the
+        // process.
+        int r = WnbdRemoveEx(devpath.c_str(), &remove_options);
+        if (r && r != ERROR_FILE_NOT_FOUND) {
+          err = -EINVAL;
+          derr << "Could not initiate mapping removal: " << devpath
+               << ". Error: " << win32_strerror(r) << dendl;
+        } else {
+          dout(1) << "Successfully initiated mapping removal: " << devpath << dendl;
+        }
+      });
+  }
+  pool.join();
+
+  if (err) {
+    derr << "Could not initiate removal of all mappings. Error: " << err << dendl;
+    return err;
+  }
+
+  // Wait for the cleanup to complete on the rbd-wnbd service side.
+  return wait_for_mappings_removal(10000);
+}
+
+std::vector<std::string> RbdMappingDispatcher::get_mapped_devpaths() {
+  std::vector<std::string> out;
+  std::unique_lock l{map_mutex};
+
+  for (auto it = mappings.begin(); it != mappings.end(); it++) {
+    out.push_back(it->first);
+  }
+
+  return out;
+}
+
+int RbdMappingDispatcher::get_mappings_count() {
+  std::unique_lock l{map_mutex};
+  return mappings.size();
+}
+
+int RbdMappingDispatcher::wait_for_mappings_removal(int timeout_ms) {
+  LARGE_INTEGER start_t, counter_freq;
+  QueryPerformanceFrequency(&counter_freq);
+  QueryPerformanceCounter(&start_t);
+
+  int time_left_ms = timeout_ms;
+  int mappings_count = get_mappings_count();
+
+  while (mappings_count && time_left_ms > 0) {
+    dout(1) << "Waiting for " << mappings_count << " mapping(s) to be removed. "
+            << "Time left: " << time_left_ms << "ms." << dendl;
+    Sleep(2000);
+
+    LARGE_INTEGER curr_t, elapsed_ms;
+    QueryPerformanceCounter(&curr_t);
+    elapsed_ms.QuadPart = curr_t.QuadPart - start_t.QuadPart;
+    elapsed_ms.QuadPart *= 1000;
+    elapsed_ms.QuadPart /= counter_freq.QuadPart;
+
+    time_left_ms = timeout_ms - elapsed_ms.QuadPart;
+    mappings_count = get_mappings_count();
+  }
+
+  if (mappings_count) {
+    derr << "Timed out waiting for disk mappings to be removed. "
+         << "Remaining mapping(s): " << mappings_count << dendl;
+    return -ETIMEDOUT;
+  } else {
+    dout(1) << "Successfully removed all mappings." << dendl;
+  }
+
+  return 0;
+}

src/tools/rbd_wnbd/rbd_mapping.h

@@ -107,7 +107,12 @@ class RbdMappingDispatcher
   std::map<std::string, std::shared_ptr<RbdMapping>> mappings;
   ceph::mutex map_mutex = ceph::make_mutex("RbdMappingDispatcher::MapMutex");
 
+  std::atomic<bool> stop_requested = false;
+
   void disconnect_cbk(std::string devpath, int ret);
+  int wait_for_mappings_removal(int timeout_ms);
+  std::vector<std::string> get_mapped_devpaths();
+  int get_mappings_count();
 
 public:
   RbdMappingDispatcher(RadosClientCache& _client_cache)
@@ -116,4 +121,7 @@ class RbdMappingDispatcher
 
   int create(Config& cfg);
   std::shared_ptr<RbdMapping> get_mapping(std::string& devpath);
+  int stop(bool hard_disconnect,
+           int soft_disconnect_timeout,
+           int worker_count);
 };

src/tools/rbd_wnbd/rbd_wnbd.cc

@@ -482,74 +482,6 @@ int restart_registered_mappings(
   return err;
 }
 
-int disconnect_all_mappings(
-  bool unregister,
-  bool hard_disconnect,
-  int soft_disconnect_timeout,
-  int worker_count)
-{
-  // Although not generally recommended, soft_disconnect_timeout can be 0,
-  // which means infinite timeout.
-  ceph_assert(soft_disconnect_timeout >= 0);
-  ceph_assert(worker_count > 0);
-  int64_t timeout_ms = soft_disconnect_timeout * 1000;
-
-  Config cfg;
-  WNBDActiveDiskIterator iterator;
-  int r;
-  std::atomic<int> err = 0;
-
-  boost::asio::thread_pool pool(worker_count);
-  LARGE_INTEGER start_t, counter_freq;
-  QueryPerformanceFrequency(&counter_freq);
-  QueryPerformanceCounter(&start_t);
-  while (iterator.get(&cfg)) {
-    boost::asio::post(pool,
-      [cfg, start_t, counter_freq, timeout_ms,
-       hard_disconnect, unregister, &err]() mutable
-      {
-        LARGE_INTEGER curr_t, elapsed_ms;
-        QueryPerformanceCounter(&curr_t);
-        elapsed_ms.QuadPart = curr_t.QuadPart - start_t.QuadPart;
-        elapsed_ms.QuadPart *= 1000;
-        elapsed_ms.QuadPart /= counter_freq.QuadPart;
-
-        int64_t time_left_ms = max((int64_t)0, timeout_ms - elapsed_ms.QuadPart);
-
-        cfg.hard_disconnect = hard_disconnect || !time_left_ms;
-        cfg.hard_disconnect_fallback = true;
-        cfg.soft_disconnect_timeout = time_left_ms / 1000;
-
-        dout(1) << "Removing mapping: " << cfg.devpath
-                << ". Timeout: " << cfg.soft_disconnect_timeout
-                << "s. Hard disconnect: " << cfg.hard_disconnect
-                << dendl;
-
-        int r = do_unmap(&cfg, unregister);
-        if (r) {
-          err = r;
-          derr << "Could not remove mapping: " << cfg.devpath
-               << ". Error: " << r << dendl;
-        } else {
-          dout(1) << "Successfully removed mapping: " << cfg.devpath << dendl;
-        }
-      });
-  }
-  pool.join();
-
-  r = iterator.get_error();
-  if (r == -ENOENT) {
-    dout(0) << __func__ << ": wnbd adapter unavailable, "
-            << "assuming that no wnbd mappings exist." << dendl;
-    err = 0;
-  } else if (r) {
-    derr << "Could not fetch all mappings. Error: " << r << dendl;
-    err = r;
-  }
-
-  return err;
-}
-
 class RBDService : public ServiceBase {
   private:
     bool hard_disconnect;
@@ -565,7 +497,7 @@ class RBDService : public ServiceBase {
     ceph::mutex start_hook_lock = ceph::make_mutex("RBDService::StartLocker");
     ceph::mutex stop_hook_lock = ceph::make_mutex("RBDService::ShutdownLocker");
     bool started = false;
-    std::atomic<bool> stop_requsted = false;
+    std::atomic<bool> stop_requested = false;
 
   public:
     RBDService(bool _hard_disconnect,
@@ -743,7 +675,7 @@ class RBDService : public ServiceBase {
       dout(0) << "monitoring wnbd adapter state changes" << dendl;
       // The event watcher will wait at most WMI_EVENT_TIMEOUT (2s)
       // and exit the loop if the service is being stopped.
-      while (!stop_requsted) {
+      while (!stop_requested) {
         IWbemClassObject* object;
         ULONG returned = 0;
 
@@ -826,10 +758,10 @@ class RBDService : public ServiceBase {
     int stop_hook() override {
       std::unique_lock l{stop_hook_lock};
 
-      stop_requsted = true;
+      stop_requested = true;
 
-      int r = disconnect_all_mappings(
-        false, hard_disconnect, soft_disconnect_timeout, thread_count);
+      int r = mapping_dispatcher.stop(
+        hard_disconnect, soft_disconnect_timeout, thread_count);
 
       if (adapter_monitor_thread.joinable()) {
         dout(10) << "waiting for wnbd event monitor thread" << dendl;

src/tools/rbd_wnbd/rbd_wnbd.h

@@ -55,11 +55,6 @@ typedef struct {
 bool is_process_running(DWORD pid);
 void unmap_at_exit();
 
-int disconnect_all_mappings(
-  bool unregister,
-  bool hard_disconnect,
-  int soft_disconnect_timeout,
-  int worker_count);
 int restart_registered_mappings(
   int worker_count, int total_timeout, int image_map_timeout);
 int map_device_using_same_process(std::string command_line);