librados: cleanly define SNAP_HEAD, SNAP_DIR constants

We were using the internal CEPH_NOSNAP and CEPH_SNAPDIR constants, and
defining a clone_info_t::HEAD (with a different value).  The docs were
referrring to the internal constant names.

Instead, define librados constants (C and C++) with the same values as the
internal types.

Note that this changes the clone_info_t::HEAD value from -1 to -2 so that
it now matches the internal type.

Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/doc/PendingReleaseNotes b/doc/PendingReleaseNotes
new file mode 100644 (file)
index 0000000..7adb635
--- /dev/null
+++ b/doc/PendingReleaseNotes
@@ -0,0 +1,4 @@
+
+* The librados::clone_info_t::HEAD constant has been replaced with librados::SNAP_HEAD, with
+  a different value that matches the internal constants.  This only affects the brand-new
+  list_snaps() interface that was introduced in v0.59.
\ No newline at end of file

diff --git a/src/include/rados/librados.h b/src/include/rados/librados.h
index b561f0c38b6c7ded98c4840e82e3ac408f756cae..7a8148dd3dd37d24a626738623ba8a5ea9aa3254 100644 (file)
--- a/src/include/rados/librados.h
+++ b/src/include/rados/librados.h
@@ -51,6 +51,12 @@ enum {
 /** @endcond */
 /** @} */
 
+/*
+ * snap id contants
+ */
+#define LIBRADOS_SNAP_HEAD  ((uint64_t)(-2))
+#define LIBRADOS_SNAP_DIR   ((uint64_t)(-1))
+
 /**
  * @typedef rados_t
  *
@@ -727,7 +733,7 @@ int rados_rollback(rados_ioctx_t io, const char *oid,
  * snapshot.
  *
  * @param io the io context to change
- * @param snap the id of the snapshot to set, or CEPH_NOSNAP for no
+ * @param snap the id of the snapshot to set, or LIBRADOS_SNAP_HEAD for no
  * snapshot (i.e. normal operation)
  */
 void rados_ioctx_snap_set_read(rados_ioctx_t io, rados_snap_t snap);
@@ -1343,7 +1349,7 @@ void rados_aio_release(rados_completion_t c);
  * @param len length of the data, in bytes
  * @param off byte offset in the object to begin writing at
  * @returns 0 on success, -EROFS if the io context specifies a snap_seq
- * other than CEPH_NOSNAP
+ * other than LIBRADOS_SNAP_HEAD
  */
 int rados_aio_write(rados_ioctx_t io, const char *oid,
                    rados_completion_t completion,
@@ -1363,7 +1369,7 @@ int rados_aio_write(rados_ioctx_t io, const char *oid,
  * @param buf the data to append
  * @param len length of buf (in bytes)
  * @returns 0 on success, -EROFS if the io context specifies a snap_seq
- * other than CEPH_NOSNAP
+ * other than LIBRADOS_SNAP_HEAD
  */
 int rados_aio_append(rados_ioctx_t io, const char *oid,
                     rados_completion_t completion,
@@ -1385,7 +1391,7 @@ int rados_aio_append(rados_ioctx_t io, const char *oid,
  * @param buf data to write
  * @param len length of the data, in bytes
  * @returns 0 on success, -EROFS if the io context specifies a snap_seq
- * other than CEPH_NOSNAP
+ * other than LIBRADOS_SNAP_HEAD
  */
 int rados_aio_write_full(rados_ioctx_t io, const char *oid,
                         rados_completion_t completion,
@@ -1403,7 +1409,7 @@ int rados_aio_write_full(rados_ioctx_t io, const char *oid,
  * @param oid the name of the object
  * @param completion what to do when the remove is safe and complete
  * @returns 0 on success, -EROFS if the io context specifies a snap_seq
- * other than CEPH_NOSNAP
+ * other than LIBRADOS_SNAP_HEAD
  */
 int rados_aio_remove(rados_ioctx_t io, const char *oid,
                     rados_completion_t completion);

diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp
index 2bb50f110fcbdac7b84d52ae2475be059e7afabe..dcb540801e65d1f9f73aba2d6319d8ec2bc9b4fc 100644 (file)
--- a/src/include/rados/librados.hpp
+++ b/src/include/rados/librados.hpp
@@ -333,12 +333,12 @@ namespace librados
      * list snapshot clones associated with a logical object
      *
      * This will include a record for each version of the object,
-     * include the "HEAD" (which will have a cloneid of
-     * librados::clone_info_t::HEAD).  Each clone includes a vector
-     * of snap ids for which it is defined to exist.
+     * include the "HEAD" (which will have a cloneid of SNAP_HEAD).
+     * Each clone includes a vector of snap ids for which it is
+     * defined to exist.
      *
      * NOTE: this operation must be submitted from an IoCtx with a
-     * read snapid of CEPH_SNAPDIR for reliable results.
+     * read snapid of SNAP_DIR for reliable results.
      *
      * @param out_snaps [out] pointer to resulting snap_set_t
      * @param prval [out] place error code in prval upon completion
@@ -538,7 +538,7 @@ namespace librados
      * @param oid the name of the object
      * @param completion what to do when the remove is safe and complete
      * @returns 0 on success, -EROFS if the io context specifies a snap_seq
-     * other than CEPH_NOSNAP
+     * other than SNAP_HEAD
      */
     int aio_remove(const std::string& oid, AioCompletion *c);
 

diff --git a/src/include/rados/rados_types.hpp b/src/include/rados/rados_types.hpp
index b61bdcb7fbf8bf641c12542671ea654b10b11f7d..4a233e6d2cac13801721cd9613825df9923e1f05 100644 (file)
--- a/src/include/rados/rados_types.hpp
+++ b/src/include/rados/rados_types.hpp
@@ -9,8 +9,12 @@ namespace librados {
 
 typedef uint64_t snap_t;
 
+enum {
+  SNAP_HEAD = (uint64_t)(-2),
+  SNAP_DIR = (uint64_t)(-1)
+};
+
 struct clone_info_t {
-  static const snap_t HEAD = ((snap_t)-1);
   snap_t cloneid;
   std::vector<snap_t> snaps;          // ascending
   std::vector< std::pair<uint64_t,uint64_t> > overlap;  // with next newest

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index b389d01e8aa6f2faa2910268fd9a768e99d48a2e..cb806e9082ae2558b5cb4c74b12b80d882e2438a 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2387,7 +2387,7 @@ reprotect_and_return_err:
        bool end_exists;
        calc_snap_set_diff(ictx->cct, snap_set,
                           from_snap_id,
-                          end_snap_id == CEPH_NOSNAP ? librados::clone_info_t::HEAD : end_snap_id,
+                          end_snap_id,
                           &diff, &end_exists);
        ldout(ictx->cct, 20) << "  diff " << diff << " end_exists=" << end_exists << dendl;
        if (diff.empty())

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index c8fb561ed994fdff571da3c26b83dd99e61b2312..425654e5a960dbd88722ac8f641edb537ad21626 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -2373,7 +2373,7 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
         if (ssc->snapset.head_exists) {
           assert(obs.exists);
           clone_info ci;
-          ci.cloneid = clone_info::HEAD;
+          ci.cloneid = CEPH_NOSNAP;
 
           //Size for HEAD is oi.size
           ci.size = oi.size;

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index f88dac031eb8560e8326fcd016b523dd9c84e7d2..f012f1e8025d23a27867a6c3528bf5b297708e91 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -20,8 +20,6 @@ extern "C" {
 #include "PG.h"
 #include "OSDMap.h"
 
-const snapid_t clone_info::HEAD((uint64_t)-1);
-
 // -- osd_reqid_t --
 void osd_reqid_t::encode(bufferlist &bl) const
 {

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index 7889d94fb5adf1f5f4ed9e58fe894c13deeff66e..17973db9dc615f5fab24ecf3e8f3750e389666a2 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -2114,8 +2114,6 @@ struct obj_list_watch_response_t {
 WRITE_CLASS_ENCODER(obj_list_watch_response_t)
 
 struct clone_info {
-  static const snapid_t HEAD;
-
   snapid_t cloneid;
   vector<snapid_t> snaps;  // ascending
   vector< pair<uint64_t,uint64_t> > overlap;
@@ -2140,7 +2138,7 @@ struct clone_info {
     DECODE_FINISH(bl);
   }
   void dump(Formatter *f) const {
-    if (cloneid == HEAD)
+    if (cloneid == CEPH_NOSNAP)
       f->dump_string("cloneid", "HEAD");
     else
       f->dump_unsigned("cloneid", cloneid.val);
@@ -2171,7 +2169,7 @@ struct clone_info {
     o.back()->overlap.push_back(pair<uint64_t,uint64_t>(8192,4096));
     o.back()->size = 16384;
     o.push_back(new clone_info);
-    o.back()->cloneid = HEAD;
+    o.back()->cloneid = CEPH_NOSNAP;
     o.back()->size = 32768;
   }
 };
@@ -2220,7 +2218,7 @@ struct obj_list_snap_response_t {
     cl.overlap.push_back(pair<uint64_t,uint64_t>(8192,4096));
     cl.size = 16384;
     o.back()->clones.push_back(cl);
-    cl.cloneid = clone_info::HEAD;
+    cl.cloneid = CEPH_NOSNAP;
     cl.snaps.clear();
     cl.overlap.clear();
     cl.size = 32768;

diff --git a/src/osdc/snap_set_diff.cc b/src/osdc/snap_set_diff.cc
index 4292b89d868d960906fed9be55941d8894aba155..2fb2dd9835890144f3fb7489708de1dc84b2f4eb 100644 (file)
--- a/src/osdc/snap_set_diff.cc
+++ b/src/osdc/snap_set_diff.cc
@@ -31,7 +31,7 @@ void calc_snap_set_diff(CephContext *cct, const librados::snap_set_t& snap_set,
     // include itself in the snaps list
     librados::snap_t a, b;
     b = r->cloneid;
-    if (b == librados::clone_info_t::HEAD) {
+    if (b == librados::SNAP_HEAD) {
       // head is valid starting from right after the last seen seq
       a = snap_set.seq + 1;
     } else {

diff --git a/src/rados.cc b/src/rados.cc
index 61265003578ec313e54551df2589e75f5d932bc8..4f02563ae1048f41490a1a0bd6d8d18280a4d23d 100644 (file)
--- a/src/rados.cc
+++ b/src/rados.cc
@@ -2061,7 +2061,7 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
 
       if (formatter) formatter->open_object_section("clone");
 
-      if (ci->cloneid == clone_info_t::HEAD) {
+      if (ci->cloneid == librados::SNAP_HEAD) {
         if (formatter)
           formatter->dump_string("id", "head");
         else
@@ -2110,7 +2110,7 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
         cout << "\t" << ci->size;
       }
 
-      if (ci->cloneid != clone_info_t::HEAD) {
+      if (ci->cloneid != librados::SNAP_HEAD) {
         if (formatter)
           formatter->open_array_section("overlaps");
         else

diff --git a/src/test/librados/snapshots.cc b/src/test/librados/snapshots.cc
index 63e9bb0a6924ed6f59e1268da616f67d4bfad15a..fe830ea8ba61c390d4549a0e875cf0269a1aad0d 100644 (file)
--- a/src/test/librados/snapshots.cc
+++ b/src/test/librados/snapshots.cc
@@ -332,7 +332,7 @@ TEST(LibRadosSnapshots, SelfManagedSnapRollbackPP) {
 
   snap_set_t ss;
 
-  snap_t head = clone_info_t::HEAD;
+  snap_t head = SNAP_HEAD;
   ASSERT_EQ(0, ioctx.list_snaps("foo", &ss));
   ASSERT_EQ(1u, ss.clones.size());
   ASSERT_EQ(head, ss.clones[0].cloneid);
@@ -415,7 +415,7 @@ TEST(LibRadosSnapshots, SelfManagedSnapOverlapPP) {
   ASSERT_EQ((int)sizeof(buf), ioctx.write("foo", bl1, sizeof(buf), bufsize*8));
 
   snap_set_t ss;
-  snap_t head = clone_info_t::HEAD;
+  snap_t head = SNAP_HEAD;
   ASSERT_EQ(0, ioctx.list_snaps("foo", &ss));
   ASSERT_EQ(1u, ss.clones.size());
   ASSERT_EQ(head, ss.clones[0].cloneid);