librbd: Add snapshot namespace to snapshot-name index in ImageCtx

The essence of this huge change is to have two keys for indexing
snap ids in ImageCtx. It used to be a map of (snap_name -> snap_id)
now it's (snap_namespace, snap_name) -> snap_id. Therefore now snapshots
can have the same name if they are in different namespaces.
All the remaining changes are a consequece of this change of
ImageCtx.snap_ids field. The only exception is: we assume that you can't
rename snapshots from GroupSnapshotNamespaces. So rename operation
always assumes UserSnapshotNamespace.

Signed-off-by: Victor Denisov <denisovenator@gmail.com>

src/cls/rbd/cls_rbd_types.h

@@ -237,6 +237,10 @@ struct UserSnapshotNamespace {
     return true;
   }
 
+  inline bool operator<(const UserSnapshotNamespace& usn) const {
+    return false;
+  }
+
 };
 
 std::ostream& operator<<(std::ostream& os, const UserSnapshotNamespace& ns);
@@ -267,6 +271,16 @@ struct GroupSnapshotNamespace {
 	   snapshot_id == gsn.snapshot_id;
   }
 
+  inline bool operator<(const GroupSnapshotNamespace& gsn) const {
+    if (group_pool < gsn.group_pool) {
+      return true;
+    } else if (group_id < gsn.group_id) {
+      return true;
+    } else {
+      return snapshot_id < gsn.snapshot_id;
+    }
+  }
+
 };
 
 std::ostream& operator<<(std::ostream& os, const GroupSnapshotNamespace& ns);
@@ -282,6 +296,10 @@ struct UnknownSnapshotNamespace {
   inline bool operator==(const UnknownSnapshotNamespace& gsn) const {
     return true;
   }
+
+  inline bool operator<(const UnknownSnapshotNamespace& usn) const {
+    return false;
+  }
 };
 
 std::ostream& operator<<(std::ostream& os, const UnknownSnapshotNamespace& ns);

src/librbd/ImageCtx.cc

@@ -412,12 +412,14 @@ struct C_InvalidateCache : public Context {
     return flags;
   }
 
-  int ImageCtx::snap_set(string in_snap_name)
+  int ImageCtx::snap_set(cls::rbd::SnapshotNamespace in_snap_namespace,
+			 string in_snap_name)
   {
     assert(snap_lock.is_wlocked());
-    snap_t in_snap_id = get_snap_id(in_snap_name);
+    snap_t in_snap_id = get_snap_id(in_snap_namespace, in_snap_name);
     if (in_snap_id != CEPH_NOSNAP) {
       snap_id = in_snap_id;
+      snap_namespace = in_snap_namespace;
       snap_name = in_snap_name;
       snap_exists = true;
       data_ctx.snap_set_read(snap_id);
@@ -430,16 +432,17 @@ struct C_InvalidateCache : public Context {
   {
     assert(snap_lock.is_wlocked());
     snap_id = CEPH_NOSNAP;
+    snap_namespace = {};
     snap_name = "";
     snap_exists = true;
     data_ctx.snap_set_read(snap_id);
   }
 
-  snap_t ImageCtx::get_snap_id(string in_snap_name) const
+  snap_t ImageCtx::get_snap_id(cls::rbd::SnapshotNamespace in_snap_namespace,
+			       string in_snap_name) const
   {
     assert(snap_lock.is_locked());
-    map<string, snap_t>::const_iterator it =
-      snap_ids.find(in_snap_name);
+    auto it = snap_ids.find({in_snap_namespace, in_snap_name});
     if (it != snap_ids.end())
       return it->second;
     return CEPH_NOSNAP;
@@ -548,8 +551,8 @@ struct C_InvalidateCache : public Context {
     return -ENOENT;
   }
 
-  void ImageCtx::add_snap(string in_snap_name,
-			  cls::rbd::SnapshotNamespace in_snap_namespace,
+  void ImageCtx::add_snap(cls::rbd::SnapshotNamespace in_snap_namespace,
+			  string in_snap_name,
 			  snap_t id, uint64_t in_size,
 			  const ParentInfo &parent, uint8_t protection_status,
                           uint64_t flags, utime_t timestamp)
@@ -558,16 +561,18 @@ struct C_InvalidateCache : public Context {
     snaps.push_back(id);
     SnapInfo info(in_snap_name, in_snap_namespace,
 		  in_size, parent, protection_status, flags, timestamp);
-    snap_info.insert(pair<snap_t, SnapInfo>(id, info));
-    snap_ids.insert(pair<string, snap_t>(in_snap_name, id));
+    snap_info.insert({id, info});
+    snap_ids.insert({{in_snap_namespace, in_snap_name}, id});
   }
 
-  void ImageCtx::rm_snap(string in_snap_name, snap_t id)
+  void ImageCtx::rm_snap(cls::rbd::SnapshotNamespace in_snap_namespace,
+			 string in_snap_name,
+			 snap_t id)
   {
     assert(snap_lock.is_wlocked());
     snaps.erase(std::remove(snaps.begin(), snaps.end(), id), snaps.end());
     snap_info.erase(id);
-    snap_ids.erase(in_snap_name);
+    snap_ids.erase({in_snap_namespace, in_snap_name});
   }
 
   uint64_t ImageCtx::get_image_size(snap_t in_snap_id) const

src/librbd/ImageCtx.h

@@ -68,7 +68,7 @@ namespace librbd {
     std::vector<librados::snap_t> snaps; // this mirrors snapc.snaps, but is in
                                         // a format librados can understand
     std::map<librados::snap_t, SnapInfo> snap_info;
-    std::map<std::string, librados::snap_t> snap_ids;
+    std::map<std::pair<cls::rbd::SnapshotNamespace, std::string>, librados::snap_t> snap_ids;
     uint64_t snap_id;
     bool snap_exists; // false if our snap_id was deleted
     // whether the image was opened read-only. cannot be changed after opening
@@ -81,6 +81,7 @@ namespace librbd {
     std::string lock_tag;
 
     std::string name;
+    cls::rbd::SnapshotNamespace snap_namespace;
     std::string snap_name;
     IoCtx data_ctx, md_ctx;
     ImageWatcher<ImageCtx> *image_watcher;
@@ -227,9 +228,11 @@ namespace librbd {
     void perf_stop();
     void set_read_flag(unsigned flag);
     int get_read_flags(librados::snap_t snap_id);
-    int snap_set(std::string in_snap_name);
+    int snap_set(cls::rbd::SnapshotNamespace in_snap_namespace,
+		 std::string in_snap_name);
     void snap_unset();
-    librados::snap_t get_snap_id(std::string in_snap_name) const;
+    librados::snap_t get_snap_id(cls::rbd::SnapshotNamespace in_snap_namespace,
+				 std::string in_snap_name) const;
     const SnapInfo* get_snap_info(librados::snap_t in_snap_id) const;
     int get_snap_name(librados::snap_t in_snap_id,
 		      std::string *out_snap_name) const;
@@ -249,12 +252,14 @@ namespace librbd {
     uint64_t get_stripe_count() const;
     uint64_t get_stripe_period() const;
 
-    void add_snap(std::string in_snap_name,
-		  cls::rbd::SnapshotNamespace in_snap_namespace,
+    void add_snap(cls::rbd::SnapshotNamespace in_snap_namespace,
+		  std::string in_snap_name,
 		  librados::snap_t id,
 		  uint64_t in_size, const ParentInfo &parent,
 		  uint8_t protection_status, uint64_t flags, utime_t timestamp);
-    void rm_snap(std::string in_snap_name, librados::snap_t id);
+    void rm_snap(cls::rbd::SnapshotNamespace in_snap_namespace,
+		 std::string in_snap_name,
+		 librados::snap_t id);
     uint64_t get_image_size(librados::snap_t in_snap_id) const;
     uint64_t get_object_count(librados::snap_t in_snap_id) const;
     bool test_features(uint64_t test_features) const;

src/librbd/ImageState.cc

@@ -381,11 +381,14 @@ ImageState<I>::find_pending_refresh() const {
 }
 
 template <typename I>
-void ImageState<I>::snap_set(const std::string &snap_name, Context *on_finish) {
+void ImageState<I>::snap_set(const cls::rbd::SnapshotNamespace &snap_namespace,
+			     const std::string &snap_name,
+			     Context *on_finish) {
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 20) << __func__ << ": snap_name=" << snap_name << dendl;
 
   Action action(ACTION_TYPE_SET_SNAP);
+  action.snap_namespace = snap_namespace;
   action.snap_name = snap_name;
 
   m_lock.Lock();
@@ -694,7 +697,7 @@ void ImageState<I>::send_set_snap_unlock() {
     *m_image_ctx, create_context_callback<
       ImageState<I>, &ImageState<I>::handle_set_snap>(this));
   image::SetSnapRequest<I> *req = image::SetSnapRequest<I>::create(
-    *m_image_ctx, action_contexts.first.snap_name, ctx);
+    *m_image_ctx, action_contexts.first.snap_namespace, action_contexts.first.snap_name, ctx);
 
   m_lock.Unlock();
   req->send();

src/librbd/ImageState.h

@@ -9,6 +9,7 @@
 #include <list>
 #include <string>
 #include <utility>
+#include "cls/rbd/cls_rbd_types.h"
 
 class Context;
 class RWLock;
@@ -39,7 +40,9 @@ class ImageState {
   int refresh_if_required();
   void refresh(Context *on_finish);
 
-  void snap_set(const std::string &snap_name, Context *on_finish);
+  void snap_set(const cls::rbd::SnapshotNamespace &snap_namespace,
+		const std::string &snap_name,
+		Context *on_finish);
 
   void prepare_lock(Context *on_ready);
   void handle_prepare_lock_complete();
@@ -72,6 +75,7 @@ class ImageState {
   struct Action {
     ActionType action_type;
     uint64_t refresh_seq = 0;
+    cls::rbd::SnapshotNamespace snap_namespace;
     std::string snap_name;
     Context *on_ready = nullptr;
 
@@ -85,7 +89,7 @@ class ImageState {
       case ACTION_TYPE_REFRESH:
         return (refresh_seq == action.refresh_seq);
       case ACTION_TYPE_SET_SNAP:
-        return snap_name == action.snap_name;
+        return (snap_name == action.snap_name) && (snap_namespace == action.snap_namespace);
       case ACTION_TYPE_LOCK:
         return false;
       default:

src/librbd/ImageWatcher.cc

@@ -150,20 +150,20 @@ void ImageWatcher<I>::notify_resize(uint64_t request_id, uint64_t size,
 }
 
 template <typename I>
-void ImageWatcher<I>::notify_snap_create(const std::string &snap_name,
-					 const cls::rbd::SnapshotNamespace &snap_namespace,
+void ImageWatcher<I>::notify_snap_create(const cls::rbd::SnapshotNamespace &snap_namespace,
+					 const std::string &snap_name,
                                          Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.exclusive_lock &&
          !m_image_ctx.exclusive_lock->is_lock_owner());
 
-  notify_lock_owner(SnapCreatePayload(snap_name, snap_namespace), on_finish);
+  notify_lock_owner(SnapCreatePayload(snap_namespace, snap_name), on_finish);
 }
 
 template <typename I>
 void ImageWatcher<I>::notify_snap_rename(const snapid_t &src_snap_id,
 				         const std::string &dst_snap_name,
-                                      Context *on_finish) {
+					 Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.exclusive_lock &&
          !m_image_ctx.exclusive_lock->is_lock_owner());
@@ -172,33 +172,36 @@ void ImageWatcher<I>::notify_snap_rename(const snapid_t &src_snap_id,
 }
 
 template <typename I>
-void ImageWatcher<I>::notify_snap_remove(const std::string &snap_name,
+void ImageWatcher<I>::notify_snap_remove(const cls::rbd::SnapshotNamespace &snap_namespace,
+					 const std::string &snap_name,
                                          Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.exclusive_lock &&
          !m_image_ctx.exclusive_lock->is_lock_owner());
 
-  notify_lock_owner(SnapRemovePayload(snap_name), on_finish);
+  notify_lock_owner(SnapRemovePayload(snap_namespace, snap_name), on_finish);
 }
 
 template <typename I>
-void ImageWatcher<I>::notify_snap_protect(const std::string &snap_name,
+void ImageWatcher<I>::notify_snap_protect(const cls::rbd::SnapshotNamespace &snap_namespace,
+					  const std::string &snap_name,
                                           Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.exclusive_lock &&
          !m_image_ctx.exclusive_lock->is_lock_owner());
 
-  notify_lock_owner(SnapProtectPayload(snap_name), on_finish);
+  notify_lock_owner(SnapProtectPayload(snap_namespace, snap_name), on_finish);
 }
 
 template <typename I>
-void ImageWatcher<I>::notify_snap_unprotect(const std::string &snap_name,
+void ImageWatcher<I>::notify_snap_unprotect(const cls::rbd::SnapshotNamespace &snap_namespace,
+					    const std::string &snap_name,
                                             Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.exclusive_lock &&
          !m_image_ctx.exclusive_lock->is_lock_owner());
 
-  notify_lock_owner(SnapUnprotectPayload(snap_name), on_finish);
+  notify_lock_owner(SnapUnprotectPayload(snap_namespace, snap_name), on_finish);
 }
 
 template <typename I>
@@ -700,8 +703,8 @@ bool ImageWatcher<I>::handle_payload(const SnapCreatePayload &payload,
       ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: "
 			         << payload.snap_name << dendl;
 
-      m_image_ctx.operations->execute_snap_create(payload.snap_name,
-						  payload.snap_namespace,
+      m_image_ctx.operations->execute_snap_create(payload.snap_namespace,
+						  payload.snap_name,
                                                   new C_ResponseMessage(ack_ctx),
                                                   0, false);
       return false;
@@ -744,7 +747,8 @@ bool ImageWatcher<I>::handle_payload(const SnapRemovePayload &payload,
       ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: "
 			         << payload.snap_name << dendl;
 
-      m_image_ctx.operations->execute_snap_remove(payload.snap_name,
+      m_image_ctx.operations->execute_snap_remove(payload.snap_namespace,
+						  payload.snap_name,
                                                   new C_ResponseMessage(ack_ctx));
       return false;
     } else if (r < 0) {
@@ -764,7 +768,8 @@ bool ImageWatcher<I>::handle_payload(const SnapProtectPayload& payload,
       ldout(m_image_ctx.cct, 10) << this << " remote snap_protect request: "
                                  << payload.snap_name << dendl;
 
-      m_image_ctx.operations->execute_snap_protect(payload.snap_name,
+      m_image_ctx.operations->execute_snap_protect(payload.snap_namespace,
+						   payload.snap_name,
                                                    new C_ResponseMessage(ack_ctx));
       return false;
     } else if (r < 0) {
@@ -784,7 +789,8 @@ bool ImageWatcher<I>::handle_payload(const SnapUnprotectPayload& payload,
       ldout(m_image_ctx.cct, 10) << this << " remote snap_unprotect request: "
                                  << payload.snap_name << dendl;
 
-      m_image_ctx.operations->execute_snap_unprotect(payload.snap_name,
+      m_image_ctx.operations->execute_snap_unprotect(payload.snap_namespace,
+						     payload.snap_name,
                                                      new C_ResponseMessage(ack_ctx));
       return false;
     } else if (r < 0) {

src/librbd/ImageWatcher.h

@@ -40,15 +40,21 @@ class ImageWatcher : public Watcher {
                       Context *on_finish);
   void notify_resize(uint64_t request_id, uint64_t size, bool allow_shrink,
                      ProgressContext &prog_ctx, Context *on_finish);
-  void notify_snap_create(const std::string &snap_name,
-			  const cls::rbd::SnapshotNamespace &snap_namespace,
+  void notify_snap_create(const cls::rbd::SnapshotNamespace &snap_namespace,
+			  const std::string &snap_name,
 			  Context *on_finish);
   void notify_snap_rename(const snapid_t &src_snap_id,
                           const std::string &dst_snap_name,
                           Context *on_finish);
-  void notify_snap_remove(const std::string &snap_name, Context *on_finish);
-  void notify_snap_protect(const std::string &snap_name, Context *on_finish);
-  void notify_snap_unprotect(const std::string &snap_name, Context *on_finish);
+  void notify_snap_remove(const cls::rbd::SnapshotNamespace &snap_namespace,
+			  const std::string &snap_name,
+			  Context *on_finish);
+  void notify_snap_protect(const cls::rbd::SnapshotNamespace &snap_namespace,
+			   const std::string &snap_name,
+			   Context *on_finish);
+  void notify_snap_unprotect(const cls::rbd::SnapshotNamespace &snap_namespace,
+			     const std::string &snap_name,
+			     Context *on_finish);
   void notify_rebuild_object_map(uint64_t request_id,
                                  ProgressContext &prog_ctx, Context *on_finish);
   void notify_rename(const std::string &image_name, Context *on_finish);