librbd: make ImageCtx methods take snap_id parameters

This makes it easier to use without racing with snap_set.
Requests in the cache, for example, store their snap_id
and may not be sent for a long time after being created.

Also rename the parameters for these methods so they
don't alias member variables.

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

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index cd0ef45edf24575c0c76ff5ee45a205b5f341f42..a6159e1625199a9c5ed18460a9e5ee4ee93c0e40 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -184,110 +184,134 @@ namespace librbd {
     data_ctx.snap_set_read(snap_id);
   }
 
-  snap_t ImageCtx::get_snap_id(string snap_name) const
+  snap_t ImageCtx::get_snap_id(string in_snap_name) const
   {
-    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(snap_name);
+    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(in_snap_name);
     if (it != snaps_by_name.end())
       return it->second.id;
     return CEPH_NOSNAP;
   }
 
-  int ImageCtx::get_snap_name(snapid_t snap_id, string *snap_name) const
+  int ImageCtx::get_snap_name(snapid_t in_snap_id, string *out_snap_name) const
   {
     map<string, SnapInfo>::const_iterator it;
 
     for (it = snaps_by_name.begin(); it != snaps_by_name.end(); it++) {
-      if (it->second.id == snap_id) {
-       *snap_name = it->first;
+      if (it->second.id == in_snap_id) {
+       *out_snap_name = it->first;
        return 0;
       }
     }
     return -ENOENT;
   }
 
-  int ImageCtx::get_snap_size(string snap_name, uint64_t *size) const
+  int ImageCtx::get_snap_size(string in_snap_name, uint64_t *out_size) const
   {
-    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(snap_name);
+    map<string, SnapInfo>::const_iterator it = snaps_by_name.find(in_snap_name);
     if (it != snaps_by_name.end()) {
-      *size = it->second.size;
+      *out_size = it->second.size;
       return 0;
     }
     return -ENOENT;
   }
 
-  void ImageCtx::add_snap(string snap_name, snap_t id, uint64_t size, uint64_t features,
-               cls_client::parent_info parent)
+  void ImageCtx::add_snap(string in_snap_name, snap_t id, uint64_t in_size,
+                         uint64_t features,
+                         cls_client::parent_info parent)
   {
     snaps.push_back(id);
-    SnapInfo info(id, size, features, parent);
-    snaps_by_name.insert(pair<string, SnapInfo>(snap_name, info));
+    SnapInfo info(id, in_size, features, parent);
+    snaps_by_name.insert(pair<string, SnapInfo>(in_snap_name, info));
   }
 
-  uint64_t ImageCtx::get_image_size() const
+  uint64_t ImageCtx::get_image_size(snap_t in_snap_id) const
   {
-    if (snap_name.length() == 0) {
+    if (in_snap_id == CEPH_NOSNAP) {
       return size;
-    } else {
-      map<string, SnapInfo>::const_iterator p = snaps_by_name.find(snap_name);
-      if (p == snaps_by_name.end())
-       return 0;
-      return p->second.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;
   }
 
-  int ImageCtx::get_features(uint64_t *out_features) const
+  int ImageCtx::get_features(snap_t in_snap_id, uint64_t *out_features) const
   {
-    if (snap_name.length() == 0) {
+    if (in_snap_id == CEPH_NOSNAP) {
       *out_features = features;
       return 0;
     }
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(snap_name);
+    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;
   }
 
-  int64_t ImageCtx::get_parent_pool_id() const
+  int64_t ImageCtx::get_parent_pool_id(snap_t in_snap_id) const
   {
-    if (snap_name.length() == 0) {
+    if (in_snap_id == CEPH_NOSNAP) {
       return parent_md.pool_id;
     }
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(snap_name);
+    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.pool_id;
   }
 
-  string ImageCtx::get_parent_image_id() const
+  string ImageCtx::get_parent_image_id(snap_t in_snap_id) const
   {
-    if (snap_name.length() == 0) {
+    if (in_snap_id == CEPH_NOSNAP) {
       return parent_md.image_id;
     }
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(snap_name);
+    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.image_id;
   }
 
-  uint64_t ImageCtx::get_parent_snap_id() const
+  uint64_t ImageCtx::get_parent_snap_id(snap_t in_snap_id) const
   {
-    if (snap_name.length() == 0) {
+    if (in_snap_id == CEPH_NOSNAP) {
       return parent_md.snap_id;
     }
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(snap_name);
+    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.snap_id;
   }
 
-  int ImageCtx::get_parent_overlap(uint64_t *overlap) const
+  int ImageCtx::get_parent_overlap(snap_t in_snap_id, uint64_t *overlap) const
   {
-    if (snap_name.length() == 0) {
+    if (in_snap_id == CEPH_NOSNAP) {
       *overlap = parent_md.overlap;
       return 0;
     }
-    map<string, SnapInfo>::const_iterator p = snaps_by_name.find(snap_name);
+    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;

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index 2467f9ddfdea92ed24dde4b389359add0bd50251..9405e8b9cabdccb16c274ebc6ee25cb7e3d56bb8 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -78,17 +78,21 @@ namespace librbd {
     void perf_stop();
     int snap_set(std::string in_snap_name);
     void snap_unset();
-    librados::snap_t get_snap_id(std::string snap_name) const;
-    int get_snap_name(snapid_t snap_id, std::string *snap_name) const;
-    int get_snap_size(std::string snap_name, uint64_t *size) const;
-    void add_snap(std::string snap_name, librados::snap_t id, uint64_t size,
-                 uint64_t features, cls_client::parent_info parent);
-    uint64_t get_image_size() const;
-    int get_features(uint64_t *out_features) const;
-    int64_t get_parent_pool_id() const;
-    std::string get_parent_image_id() const;
-    uint64_t get_parent_snap_id() const;
-    int get_parent_overlap(uint64_t *overlap) const;
+    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_snap_size(std::string in_snap_name, uint64_t *out_size) const;
+    void add_snap(std::string in_snap_name, librados::snap_t id,
+                 uint64_t in_size, uint64_t features,
+                 cls_client::parent_info parent);
+
+    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;
+    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;
+    int get_parent_overlap(librados::snap_t in_snap_id,
+                          uint64_t *overlap) const;
     void aio_read_from_cache(object_t o, bufferlist *bl, size_t len,
                             uint64_t off, Context *onfinish);
     void write_to_cache(object_t o, bufferlist& bl, size_t len, uint64_t off);

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 56fd4c590542b3651306cdd4980eeb93bfe796b0..c6960dac4cce5e356eb212ae5bfceab65e123268 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -93,7 +93,7 @@ namespace librbd {
   void image_info(ImageCtx *ictx, image_info_t& info, size_t infosize)
   {
     int obj_order = ictx->order;
-    info.size = ictx->get_image_size();
+    info.size = ictx->get_image_size(ictx->snap_id);
     info.obj_size = 1 << obj_order;
     info.num_objs = howmany(info.size, get_block_size(obj_order));
     info.order = obj_order;
@@ -570,7 +570,7 @@ namespace librbd {
       order = p_imctx->order;
     }
 
-    uint64_t size = p_imctx->get_image_size();
+    uint64_t size = p_imctx->get_image_size(p_imctx->snap_id);
     int remove_r;
     librbd::NoOpProgressContext no_op;
     ImageCtx *c_imctx = NULL;
@@ -754,7 +754,7 @@ namespace librbd {
     if (r < 0)
       return r;
     Mutex::Locker(ictx->lock);
-    return ictx->get_features(features);
+    return ictx->get_features(ictx->snap_id, features);
   }
 
   int get_overlap(ImageCtx *ictx, uint64_t *overlap)
@@ -763,7 +763,7 @@ namespace librbd {
     if (r < 0)
       return r;
     Mutex::Locker(ictx->lock);
-    return ictx->get_parent_overlap(overlap);
+    return ictx->get_parent_overlap(ictx->snap_id, overlap);
   }
 
   int open_parent(ImageCtx *ictx, ImageCtx **parent_ctx,
@@ -773,7 +773,11 @@ namespace librbd {
     assert(ictx->parent_md.pool_id >= 0);
     string pool_name;
     Rados rados(ictx->md_ctx);
-    int64_t pool_id = ictx->get_parent_pool_id();
+    ictx->lock.Lock();
+    int64_t pool_id = ictx->get_parent_pool_id(ictx->snap_id);
+    string parent_image_id = ictx->get_parent_image_id(ictx->snap_id);
+    snap_t parent_snap_id = ictx->get_parent_snap_id(ictx->snap_id);
+    ictx->lock.Unlock();
     if (pool_id < 0)
       return -ENOENT;
     int r = rados.pool_reverse_lookup(pool_id, &pool_name);
@@ -791,10 +795,10 @@ namespace librbd {
       return r;
     }
 
+
     if (parent_image_name) {
       r = cls_client::dir_get_name(&p_ioctx, RBD_DIRECTORY,
-                                  ictx->get_parent_image_id(),
-                                  parent_image_name);
+                                  parent_image_id, parent_image_name);
       if (r < 0) {
        lderr(ictx->cct) << "error getting parent image name: "
                         << cpp_strerror(r) << dendl;
@@ -804,8 +808,7 @@ namespace librbd {
 
     // since we don't know the image and snapshot name, set their ids and
     // reset the snap_name and snap_exists fields after we read the header
-    ImageCtx *parent = new ImageCtx("", ictx->get_parent_image_id(),
-                                   NULL, p_ioctx);
+    ImageCtx *parent = new ImageCtx("", parent_image_id, NULL, p_ioctx);
     r = open_image(parent, false);
     if (r < 0) {
       lderr(ictx->cct) << "error opening parent image: " << cpp_strerror(r)
@@ -813,7 +816,7 @@ namespace librbd {
       close_image(parent);
       return r;
     }
-    r = parent->get_snap_name(ictx->get_parent_snap_id(), &parent->snap_name);
+    r = parent->get_snap_name(parent_snap_id, &parent->snap_name);
     if (r < 0) {
       lderr(ictx->cct) << "parent snapshot does not exist" << dendl;
       close_image(parent);
@@ -837,7 +840,7 @@ namespace librbd {
       return r;
 
     Mutex::Locker l(ictx->lock);
-    if (ictx->get_parent_pool_id() < 0)
+    if (ictx->get_parent_pool_id(ictx->snap_id) < 0)
       return -ENOENT;
 
     // for parent snap_name, we need to open the parent ImageCtx, for which
@@ -1096,19 +1099,20 @@ namespace librbd {
     int r;
     if (ictx->parent) {
       uint64_t overlap;
-      r = ictx->get_parent_overlap(&overlap);
+      r = ictx->get_parent_overlap(ictx->snap_id, &overlap);
       if (r < 0)
        return r;
       if (!overlap ||
-         ictx->parent->md_ctx.get_id() != ictx->get_parent_pool_id() ||
-         ictx->parent->id != ictx->get_parent_image_id() ||
-         ictx->parent->snap_id != ictx->get_parent_snap_id()) {
+         ictx->parent->md_ctx.get_id() !=
+         ictx->get_parent_pool_id(ictx->snap_id) ||
+         ictx->parent->id != ictx->get_parent_image_id(ictx->snap_id) ||
+         ictx->parent->snap_id != ictx->get_parent_snap_id(ictx->snap_id)) {
        close_image(ictx->parent);
        ictx->parent = NULL;
       }
     }
 
-    if (ictx->get_parent_pool_id() > -1 && !ictx->parent) {
+    if (ictx->get_parent_pool_id(ictx->snap_id) > -1 && !ictx->parent) {
       r = open_parent(ictx, &(ictx->parent), NULL, NULL);
       if (r < 0) {
        lderr(ictx->cct) << "error opening parent snapshot: "
@@ -1269,7 +1273,7 @@ namespace librbd {
     // the current version, so we have to invalidate that too.
     ictx->invalidate_cache();
 
-    uint64_t new_size = ictx->get_image_size();
+    uint64_t new_size = ictx->get_image_size(ictx->snap_id);
     ictx->get_snap_size(snap_name, &new_size);
     ldout(cct, 2) << "resizing to snapshot size..." << dendl;
     NoOpProgressContext no_op;
@@ -1323,7 +1327,7 @@ namespace librbd {
   {
     CephContext *cct = (CephContext *)dest_md_ctx.cct();
     CopyProgressCtx cp(prog_ctx);
-    uint64_t src_size = ictx->get_image_size();
+    uint64_t src_size = ictx->get_image_size(ictx->snap_id);
     int64_t r;
 
     int order = ictx->order;
@@ -1864,7 +1868,7 @@ namespace librbd {
   int check_io(ImageCtx *ictx, uint64_t off, uint64_t len)
   {
     ictx->lock.Lock();
-    uint64_t image_size = ictx->get_image_size();
+    uint64_t image_size = ictx->get_image_size(ictx->snap_id);
     bool snap_exists = ictx->snap_exists;
     ictx->lock.Unlock();