pacific: librbd: allow interrupted trash move request to be restarted

src/librbd/api/Trash.cc

@@ -25,6 +25,7 @@
 #include <json_spirit/json_spirit.h>
 #include "librbd/journal/DisabledPolicy.h"
 #include "librbd/image/ListWatchersRequest.h"
+#include <experimental/map>
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -108,6 +109,48 @@ int enable_mirroring(IoCtx &io_ctx, const std::string &image_id) {
   return 0;
 }
 
+int list_trash_image_specs(
+    librados::IoCtx &io_ctx,
+    std::map<std::string, cls::rbd::TrashImageSpec>* trash_image_specs,
+    bool exclude_user_remove_source) {
+  CephContext *cct((CephContext *)io_ctx.cct());
+  ldout(cct, 20) << "list_trash_image_specs " << &io_ctx << dendl;
+
+  bool more_entries;
+  uint32_t max_read = 1024;
+  std::string last_read;
+  do {
+    std::map<string, cls::rbd::TrashImageSpec> trash_entries;
+    int r = cls_client::trash_list(&io_ctx, last_read, max_read,
+                                   &trash_entries);
+    if (r < 0 && r != -ENOENT) {
+      lderr(cct) << "error listing rbd trash entries: " << cpp_strerror(r)
+                 << dendl;
+      return r;
+    } else if (r == -ENOENT) {
+      break;
+    }
+
+    if (trash_entries.empty()) {
+      break;
+    }
+
+    for (const auto &entry : trash_entries) {
+      if (exclude_user_remove_source &&
+          entry.second.source == cls::rbd::TRASH_IMAGE_SOURCE_REMOVING) {
+        continue;
+      }
+
+      trash_image_specs->insert({entry.first, entry.second});
+    }
+
+    last_read = trash_entries.rbegin()->first;
+    more_entries = (trash_entries.size() >= max_read);
+  } while (more_entries);
+
+  return 0;
+}
+
 } // anonymous namespace
 
 template <typename I>
@@ -246,15 +289,37 @@ int Trash<I>::move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source,
       return -EOPNOTSUPP;
     }
 
-    // image doesn't exist -- perhaps already in the trash since removing
-    // from the directory is the last step
-    return -ENOENT;
+    // search for an interrupted trash move request
+    std::map<std::string, cls::rbd::TrashImageSpec> trash_image_specs;
+    int r = list_trash_image_specs(io_ctx, &trash_image_specs, true);
+    if (r < 0) {
+      return r;
+    }
+
+    std::experimental::erase_if(
+      trash_image_specs, [image_name](const auto& pair) {
+        const auto& spec = pair.second;
+        return (spec.source != cls::rbd::TRASH_IMAGE_SOURCE_USER ||
+                spec.state != cls::rbd::TRASH_IMAGE_STATE_MOVING ||
+                spec.name != image_name);
+      });
+    if (trash_image_specs.empty()) {
+      return -ENOENT;
+    }
+
+    image_id = trash_image_specs.begin()->first;
+    ldout(cct, 15) << "derived image id " << image_id << " from existing "
+                   << "trash entry" << dendl;
   } else if (r < 0) {
     lderr(cct) << "failed to retrieve image id: " << cpp_strerror(r) << dendl;
     return r;
   }
 
-  ceph_assert(!image_name.empty() && !image_id.empty());
+  if (image_name.empty() || image_id.empty()) {
+    lderr(cct) << "invalid image name/id" << dendl;
+    return -EINVAL;
+  }
+
   return Trash<I>::move(io_ctx, source, image_name, image_id, delay);
 }
 
@@ -285,41 +350,23 @@ template <typename I>
 int Trash<I>::list(IoCtx &io_ctx, vector<trash_image_info_t> &entries,
                    bool exclude_user_remove_source) {
   CephContext *cct((CephContext *)io_ctx.cct());
-  ldout(cct, 20) << "trash_list " << &io_ctx << dendl;
-
-  bool more_entries;
-  uint32_t max_read = 1024;
-  std::string last_read = "";
-  do {
-    map<string, cls::rbd::TrashImageSpec> trash_entries;
-    int r = cls_client::trash_list(&io_ctx, last_read, max_read,
-                                   &trash_entries);
-    if (r < 0 && r != -ENOENT) {
-      lderr(cct) << "error listing rbd trash entries: " << cpp_strerror(r)
-                 << dendl;
-      return r;
-    } else if (r == -ENOENT) {
-      break;
-    }
+  ldout(cct, 20) << __func__ << " " << &io_ctx << dendl;
 
-    if (trash_entries.empty()) {
-      break;
-    }
+  std::map<std::string, cls::rbd::TrashImageSpec> trash_image_specs;
+  int r = list_trash_image_specs(io_ctx, &trash_image_specs,
+                                 exclude_user_remove_source);
+  if (r < 0) {
+    return r;
+  }
 
-    for (const auto &entry : trash_entries) {
-      rbd_trash_image_source_t source =
-          static_cast<rbd_trash_image_source_t>(entry.second.source);
-      if (exclude_user_remove_source &&
-          source == RBD_TRASH_IMAGE_SOURCE_REMOVING) {
-        continue;
-      }
-      entries.push_back({entry.first, entry.second.name, source,
-                         entry.second.deletion_time.sec(),
-                         entry.second.deferment_end_time.sec()});
-    }
-    last_read = trash_entries.rbegin()->first;
-    more_entries = (trash_entries.size() >= max_read);
-  } while (more_entries);
+  entries.reserve(trash_image_specs.size());
+  for (const auto& [image_id, spec] : trash_image_specs) {
+    rbd_trash_image_source_t source =
+        static_cast<rbd_trash_image_source_t>(spec.source);
+    entries.push_back({image_id, spec.name, source,
+                       spec.deletion_time.sec(),
+                       spec.deferment_end_time.sec()});
+  }
 
   return 0;
 }
@@ -495,6 +542,10 @@ int Trash<I>::purge(IoCtx& io_ctx, time_t expire_ts,
                       << "using it crashed. Try again after closing/unmapping "
                       << "it or waiting 30s for the crashed client to timeout."
                       << dendl;
+      } else if (r == -EUCLEAN) {
+        ldout(cct, 5) << "Image is not in the expected state. Ensure moving "
+                      << "the image to the trash completed successfully."
+                      << dendl;
       } else if (r == -EMLINK) {
         ldout(cct, 5) << "Remove the image from the group and try again."
                       << dendl;
@@ -529,8 +580,12 @@ int Trash<I>::remove(IoCtx &io_ctx, const std::string &image_id, bool force,
     lderr(cct) << "error: deferment time has not expired." << dendl;
     return -EPERM;
   }
-  if (trash_spec.state != cls::rbd::TRASH_IMAGE_STATE_NORMAL &&
-      trash_spec.state != cls::rbd::TRASH_IMAGE_STATE_REMOVING) {
+  if (trash_spec.state == cls::rbd::TRASH_IMAGE_STATE_MOVING) {
+    lderr(cct) << "error: image is pending moving to the trash."
+               << dendl;
+    return -EUCLEAN;
+  } else if (trash_spec.state != cls::rbd::TRASH_IMAGE_STATE_NORMAL &&
+             trash_spec.state != cls::rbd::TRASH_IMAGE_STATE_REMOVING) {
     lderr(cct) << "error: image is pending restoration." << dendl;
     return -EBUSY;
   }

src/librbd/trash/MoveRequest.cc

@@ -101,7 +101,10 @@ template <typename I>
 void MoveRequest<I>::handle_directory_remove(int r) {
   ldout(m_cct, 10) << "r=" << r << dendl;
 
-  if (r < 0 && r != -ENOENT) {
+  if (r == -ENOENT) {
+    r = 0;
+  }
+  if (r < 0) {
     lderr(m_cct) << "failed to remove image from directory: " << cpp_strerror(r)
                  << dendl;
   }

src/tools/rbd/action/Trash.cc

@@ -147,6 +147,9 @@ int execute_remove(const po::variables_map &vm,
       std::cerr << "rbd: image has snapshots - these must be deleted"
                 << " with 'rbd snap purge' before the image can be removed."
                 << std::endl;
+    } else if (r == -EUCLEAN) {
+      std::cerr << "rbd: error: image not fully moved to trash."
+                << std::endl;
     } else if (r == -EBUSY) {
       std::cerr << "rbd: error: image still has watchers"
                 << std::endl