librbd: store and retrieve snapshot metadata based on id

Snapshots are usually accessed by id internally, so change accessors
to take id instead of name. Keep a separate map of name -> id for
looking up human-specified names.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index b5c2db6e7895ee9205f084e73bfbd80399929f8b..06ae93b261def6f7671d3284575570d4979a4aaf 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -261,10 +261,10 @@ namespace librbd {
 
   int ImageCtx::snap_set(string in_snap_name)
   {
-    map<string, SnapInfo>::iterator it = snaps_by_name.find(in_snap_name);
-    if (it != snaps_by_name.end()) {
+    snap_t in_snap_id = get_snap_id(in_snap_name);
+    if (in_snap_id != CEPH_NOSNAP) {
+      snap_id = in_snap_id;
       snap_name = in_snap_name;
-      snap_id = it->second.id;
       snap_exists = true;
       data_ctx.snap_set_read(snap_id);
       return 0;
@@ -282,34 +282,40 @@ namespace librbd {
 
   snap_t ImageCtx::get_snap_id(string in_snap_name) const
   {
-    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(in_snap_name);
-    if (it != snaps_by_name.end())
-      return it->second.id;
+    map<string, snap_t>::const_iterator it =
+      snap_ids.find(in_snap_name);
+    if (it != snap_ids.end())
+      return it->second;
     return CEPH_NOSNAP;
   }
 
-  int ImageCtx::get_snap_name(snapid_t in_snap_id, string *out_snap_name) const
+  const SnapInfo* ImageCtx::get_snap_info(snap_t in_snap_id) const
   {
-    map<string, SnapInfo>::const_iterator it;
+    map<snap_t, SnapInfo>::const_iterator it =
+      snap_info.find(in_snap_id);
+    if (it != snap_info.end())
+      return &it->second;
+    return NULL;
+  }
 
-    for (it = snaps_by_name.begin(); it != snaps_by_name.end(); ++it) {
-      if (it->second.id == in_snap_id) {
-       *out_snap_name = it->first;
-       return 0;
-      }
+  int ImageCtx::get_snap_name(snap_t in_snap_id,
+                             string *out_snap_name) const
+  {
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info) {
+      *out_snap_name = info->name;
+      return 0;
     }
     return -ENOENT;
   }
 
-  int ImageCtx::get_parent_spec(snapid_t in_snap_id, parent_spec *out_pspec)
+  int ImageCtx::get_parent_spec(snap_t in_snap_id,
+                               parent_spec *out_pspec) const
   {
-    map<string, SnapInfo>::iterator it;
-
-    for (it = snaps_by_name.begin(); it != snaps_by_name.end(); ++it) {
-      if (it->second.id == in_snap_id) {
-       *out_pspec = it->second.parent.spec;
-       return 0;
-      }
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info) {
+      *out_pspec = info->parent.spec;
+      return 0;
     }
     return -ENOENT;
   }
@@ -352,24 +358,25 @@ namespace librbd {
     return num_periods * stripe_count;
   }
 
-  int ImageCtx::is_snap_protected(string in_snap_name, bool *is_protected) const
+  int ImageCtx::is_snap_protected(snap_t in_snap_id,
+                                 bool *is_protected) const
   {
-    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(in_snap_name);
-    if (it != snaps_by_name.end()) {
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info) {
       *is_protected =
-       (it->second.protection_status == RBD_PROTECTION_STATUS_PROTECTED);
+       (info->protection_status == RBD_PROTECTION_STATUS_PROTECTED);
       return 0;
     }
     return -ENOENT;
   }
 
-  int ImageCtx::is_snap_unprotected(string in_snap_name,
+  int ImageCtx::is_snap_unprotected(snap_t in_snap_id,
                                    bool *is_unprotected) const
   {
-    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(in_snap_name);
-    if (it != snaps_by_name.end()) {
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info) {
       *is_unprotected =
-       (it->second.protection_status == RBD_PROTECTION_STATUS_UNPROTECTED);
+       (info->protection_status == RBD_PROTECTION_STATUS_UNPROTECTED);
       return 0;
     }
     return -ENOENT;
@@ -381,8 +388,9 @@ namespace librbd {
                          uint8_t protection_status)
   {
     snaps.push_back(id);
-    SnapInfo info(id, in_size, features, parent, protection_status);
-    snaps_by_name.insert(pair<string, SnapInfo>(in_snap_name, info));
+    SnapInfo info(in_snap_name, in_size, features, parent, protection_status);
+    snap_info.insert(pair<snap_t, SnapInfo>(id, info));
+    snap_ids.insert(pair<string, snap_t>(in_snap_name, id));
   }
 
   uint64_t ImageCtx::get_image_size(snap_t in_snap_id) const
@@ -390,14 +398,12 @@ namespace librbd {
     if (in_snap_id == CEPH_NOSNAP) {
       return size;
     }
-    string in_snap_name;
-    int r = get_snap_name(in_snap_id, &in_snap_name);
-    if (r < 0)
-      return 0;
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(in_snap_name);
-    if (p == snaps_by_name.end())
-      return 0;
-    return p->second.size;
+
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info) {
+      return info->size;
+    }
+    return 0;
   }
 
   int ImageCtx::get_features(snap_t in_snap_id, uint64_t *out_features) const
@@ -406,77 +412,56 @@ namespace librbd {
       *out_features = features;
       return 0;
     }
-    string in_snap_name;
-    int r = get_snap_name(in_snap_id, &in_snap_name);
-    if (r < 0)
-      return r;
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(in_snap_name);
-    if (p == snaps_by_name.end())
-      return -ENOENT;
-    *out_features = p->second.features;
-    return 0;
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info) {
+      *out_features = info->features;
+      return 0;
+    }
+    return -ENOENT;
+  }
+
+  const parent_info* ImageCtx::get_parent_info(snap_t in_snap_id) const
+  {
+    if (in_snap_id == CEPH_NOSNAP)
+      return &parent_md;
+    const SnapInfo *info = get_snap_info(in_snap_id);
+    if (info)
+      return &info->parent;
+    return NULL;
   }
 
   int64_t ImageCtx::get_parent_pool_id(snap_t in_snap_id) const
   {
-    if (in_snap_id == CEPH_NOSNAP) {
-      return parent_md.spec.pool_id;
-    }
-    string in_snap_name;
-    int r = get_snap_name(in_snap_id, &in_snap_name);
-    if (r < 0)
-      return -1;
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(in_snap_name);
-    if (p == snaps_by_name.end())
-      return -1;
-    return p->second.parent.spec.pool_id;
+    const parent_info *info = get_parent_info(in_snap_id);
+    if (info)
+      return info->spec.pool_id;
+    return -1;
   }
 
   string ImageCtx::get_parent_image_id(snap_t in_snap_id) const
   {
-    if (in_snap_id == CEPH_NOSNAP) {
-      return parent_md.spec.image_id;
-    }
-    string in_snap_name;
-    int r = get_snap_name(in_snap_id, &in_snap_name);
-    if (r < 0)
-      return "";
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(in_snap_name);
-    if (p == snaps_by_name.end())
-      return "";
-    return p->second.parent.spec.image_id;
+    const parent_info *info = get_parent_info(in_snap_id);
+    if (info)
+      return info->spec.image_id;
+    return "";
   }
 
   uint64_t ImageCtx::get_parent_snap_id(snap_t in_snap_id) const
   {
-    if (in_snap_id == CEPH_NOSNAP) {
-      return parent_md.spec.snap_id;
-    }
-    string in_snap_name;
-    int r = get_snap_name(in_snap_id, &in_snap_name);
-    if (r < 0)
-      return CEPH_NOSNAP;
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(in_snap_name);
-    if (p == snaps_by_name.end())
-      return CEPH_NOSNAP;
-    return p->second.parent.spec.snap_id;
+    const parent_info *info = get_parent_info(in_snap_id);
+    if (info)
+      return info->spec.snap_id;
+    return CEPH_NOSNAP;
   }
 
   int ImageCtx::get_parent_overlap(snap_t in_snap_id, uint64_t *overlap) const
   {
-    if (in_snap_id == CEPH_NOSNAP) {
-      *overlap = parent_md.overlap;
+    const parent_info *info = get_parent_info(in_snap_id);
+    if (info) {
+      *overlap = info->overlap;
       return 0;
     }
-    string in_snap_name;
-    int r = get_snap_name(in_snap_id, &in_snap_name);
-    if (r < 0)
-      return r;
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(in_snap_name);
-    if (p == snaps_by_name.end())
-      return -ENOENT;
-    *overlap = p->second.parent.overlap;
-    return 0;
+    return -ENOENT;
   }
 
   void ImageCtx::aio_read_from_cache(object_t o, bufferlist *bl, size_t len,

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index 83ed0449904a3eed9ff093c29fa25e18b2d68130..2c56461ab41aac54d1075f0f304732b14601affa 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -38,7 +38,8 @@ namespace librbd {
     ::SnapContext snapc;
     std::vector<librados::snap_t> snaps; // this mirrors snapc.snaps, but is in
                                         // a format librados can understand
-    std::map<std::string, SnapInfo> snaps_by_name;
+    std::map<librados::snap_t, SnapInfo> snap_info;
+    std::map<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
@@ -106,10 +107,15 @@ namespace librbd {
     int snap_set(std::string in_snap_name);
     void snap_unset();
     librados::snap_t get_snap_id(std::string in_snap_name) const;
-    int get_snap_name(snapid_t snap_id, std::string *out_snap_name) const;
-    int get_parent_spec(snapid_t snap_id, parent_spec *pspec);
-    int is_snap_protected(string in_snap_name, bool *is_protected) const;
-    int is_snap_unprotected(string in_snap_name, bool *is_unprotected) 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;
+    int get_parent_spec(librados::snap_t in_snap_id,
+                       parent_spec *pspec) const;
+    int is_snap_protected(librados::snap_t in_snap_id,
+                         bool *is_protected) const;
+    int is_snap_unprotected(librados::snap_t in_snap_id,
+                           bool *is_unprotected) const;
 
     uint64_t get_current_size() const;
     uint64_t get_object_size() const;
@@ -125,6 +131,7 @@ namespace librbd {
     uint64_t get_image_size(librados::snap_t in_snap_id) const;
     int get_features(librados::snap_t in_snap_id,
                     uint64_t *out_features) const;
+    const parent_info* get_parent_info(librados::snap_t in_snap_id) const;
     int64_t get_parent_pool_id(librados::snap_t in_snap_id) const;
     std::string get_parent_image_id(librados::snap_t in_snap_id) const;
     uint64_t get_parent_snap_id(librados::snap_t in_snap_id) const;

diff --git a/src/librbd/SnapInfo.h b/src/librbd/SnapInfo.h
index 44dd4cf208e76a57a268ced892d02d7f253cf1d0..fd973536d872a4dac050b2fde3472922f977f82e 100644 (file)
--- a/src/librbd/SnapInfo.h
+++ b/src/librbd/SnapInfo.h
@@ -13,14 +13,14 @@
 namespace librbd {
 
   struct SnapInfo {
-    librados::snap_t id;
+    std::string name;
     uint64_t size;
     uint64_t features;
     parent_info parent;
     uint8_t protection_status;
-    SnapInfo(librados::snap_t _id, uint64_t _size, uint64_t _features,
+    SnapInfo(std::string _name, uint64_t _size, uint64_t _features,
             parent_info _parent, uint8_t _protection_status) :
-      id(_id), size(_size), features(_features), parent(_parent),
+      name(_name), size(_size), features(_features), parent(_parent),
       protection_status(_protection_status) {}
   };
 }

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index b18eb932b81b59b30e82e35888244f50151407a1..a696612d262c131974cd9c8c94cf701bd78cc9a1 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -475,16 +475,16 @@ namespace librbd {
                              snapid_t oursnap_id)
   {
     if (pspec.pool_id != -1) {
-      map<string, SnapInfo>::iterator it;
-      for (it = ictx->snaps_by_name.begin();
-          it != ictx->snaps_by_name.end(); ++it) {
+      map<snap_t, SnapInfo>::iterator it;
+      for (it = ictx->snap_info.begin();
+          it != ictx->snap_info.end(); ++it) {
        // skip our snap id (if checking base image, CEPH_NOSNAP won't match)
-       if (it->second.id == oursnap_id)
+       if (it->first == oursnap_id)
          continue;
        if (it->second.parent.spec == pspec)
          break;
       }
-      if (it == ictx->snaps_by_name.end())
+      if (it == ictx->snap_info.end())
        return -ENOENT;
     }
     return 0;
@@ -569,7 +569,7 @@ namespace librbd {
       return -ENOENT;
 
     bool is_protected;
-    r = ictx->is_snap_protected(snap_name, &is_protected);
+    r = ictx->is_snap_protected(snap_id, &is_protected);
     if (r < 0)
       return r;
 
@@ -612,7 +612,7 @@ namespace librbd {
       return -ENOENT;
 
     bool is_unprotected;
-    r = ictx->is_snap_unprotected(snap_name, &is_unprotected);
+    r = ictx->is_snap_unprotected(snap_id, &is_unprotected);
     if (r < 0)
       return r;
 
@@ -695,8 +695,11 @@ reprotect_and_return_err:
       return r;
 
     RWLock::RLocker l(ictx->snap_lock);
+    snap_t snap_id = ictx->get_snap_id(snap_name);
+    if (snap_id == CEPH_NOSNAP)
+      return -ENOENT;
     bool is_unprotected;
-    r = ictx->is_snap_unprotected(snap_name, &is_unprotected);
+    r = ictx->is_snap_unprotected(snap_id, &is_unprotected);
     // consider both PROTECTED or UNPROTECTING to be 'protected',
     // since in either state they can't be deleted
     *is_protected = !is_unprotected;
@@ -975,7 +978,7 @@ reprotect_and_return_err:
     p_imctx->snap_lock.get_read();
     p_imctx->get_features(p_imctx->snap_id, &p_features);
     size = p_imctx->get_image_size(p_imctx->snap_id);
-    p_imctx->is_snap_protected(p_imctx->snap_name, &snap_protected);
+    p_imctx->is_snap_protected(p_imctx->snap_id, &snap_protected);
     p_imctx->snap_lock.put_read();
     p_imctx->md_lock.put_read();
 
@@ -1027,7 +1030,7 @@ reprotect_and_return_err:
 
     if (r == 0) {
       p_imctx->snap_lock.get_read();
-      r = p_imctx->is_snap_protected(p_imctx->snap_name, &snap_protected);
+      r = p_imctx->is_snap_protected(p_imctx->snap_id, &snap_protected);
       p_imctx->snap_lock.put_read();
     }
     if (r < 0 || !snap_protected) {
@@ -1544,11 +1547,11 @@ reprotect_and_return_err:
     bufferlist bl, bl2;
 
     RWLock::RLocker l(ictx->snap_lock);
-    for (map<string, SnapInfo>::iterator it = ictx->snaps_by_name.begin();
-        it != ictx->snaps_by_name.end(); ++it) {
+    for (map<snap_t, SnapInfo>::iterator it = ictx->snap_info.begin();
+        it != ictx->snap_info.end(); ++it) {
       snap_info_t info;
-      info.name = it->first;
-      info.id = it->second.id;
+      info.name = it->second.name;
+      info.id = it->first;
       info.size = it->second.size;
       snaps.push_back(info);
     }
@@ -1565,7 +1568,7 @@ reprotect_and_return_err:
       return r;
 
     RWLock::RLocker l(ictx->snap_lock);
-    return ictx->snaps_by_name.count(snap_name);
+    return ictx->get_snap_id(snap_name) != CEPH_NOSNAP;
   }
 
 
@@ -1788,7 +1791,8 @@ reprotect_and_return_err:
        }
 
        ictx->snaps.clear();
-       ictx->snaps_by_name.clear();
+       ictx->snap_info.clear();
+       ictx->snap_ids.clear();
        for (size_t i = 0; i < new_snapc.snaps.size(); ++i) {
          uint64_t features = ictx->old_format ? 0 : snap_features[i];
          uint8_t protection_status = ictx->old_format ?