librbd, cls_rbd: close snapshot creation race with old format

If two clients created a snapshot at the same time, the one with the
higher snapshot id might be created first, so the lower snapshot id
would be added to the snapshot context and the snaphot seq would be
set to the lower one.

Instead of allowing this to happen, return -ESTALE if the snapshot id
is lower than the currently stored snapshot sequence number. On the
client side, get a new id and retry if this error is encountered.

Backport: argonaut
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>

diff --git a/src/cls_rbd.cc b/src/cls_rbd.cc
index c03f99bdb5ead7f1bc75d0971158976e25017eb1..275ecd36bfe1abf0e926bec2f0a061b94bfec4ef 100644 (file)
--- a/src/cls_rbd.cc
+++ b/src/cls_rbd.cc
@@ -684,6 +684,9 @@ int old_snapshot_add(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
   }
   snap_name = s.c_str();
 
+  if (header->snap_seq > snap_id)
+    return -ESTALE;
+
   const char *cur_snap_name;
   for (cur_snap_name = snap_names; cur_snap_name < end; cur_snap_name += strlen(cur_snap_name) + 1) {
     if (strncmp(cur_snap_name, snap_name, end - cur_snap_name) == 0)

diff --git a/src/librbd.cc b/src/librbd.cc
index 569a372acf74e36312baa4a108c52c3c86f1c991..54c9c27531bcefa514d7766255149a34c3b0945f 100644 (file)
--- a/src/librbd.cc
+++ b/src/librbd.cc
@@ -922,7 +922,9 @@ int snap_create(ImageCtx *ictx, const char *snap_name)
     return r;
 
   Mutex::Locker l(ictx->lock);
-  r = add_snap(ictx, snap_name);
+  do {
+    r = add_snap(ictx, snap_name);
+  } while (r == -ESTALE);
 
   if (r < 0)
     return r;

diff --git a/src/test/rbd/test_cls_rbd.cc b/src/test/rbd/test_cls_rbd.cc
index a99051cb739de86dd5ee691f2c3c1c7c61b2e488..bc085d7838246ffddcd3298d0a9427e66bc28572 100644 (file)
--- a/src/test/rbd/test_cls_rbd.cc
+++ b/src/test/rbd/test_cls_rbd.cc
@@ -25,6 +25,7 @@ using ::librbd::cls_client::snapshot_add;
 using ::librbd::cls_client::snapshot_remove;
 using ::librbd::cls_client::get_snapcontext;
 using ::librbd::cls_client::snapshot_list;
+using ::librbd::cls_client::old_snapshot_add;
 
 TEST(cls_rbd, create)
 {
@@ -335,3 +336,27 @@ TEST(cls_rbd, snapshots)
   ioctx.close();
   ASSERT_EQ(0, destroy_one_pool_pp(pool_name, rados));
 }
+
+TEST(cls_rbd, snapid_race)
+{
+   librados::Rados rados;
+  librados::IoCtx ioctx;
+  string pool_name = get_temp_pool_name();
+
+  ASSERT_EQ("", create_one_pool_pp(pool_name, rados));
+  ASSERT_EQ(0, rados.ioctx_create(pool_name.c_str(), ioctx));
+
+  buffer::list bl;
+  buffer::ptr bp(4096);
+  bp.zero();
+  bl.append(bp);
+
+  string oid = "foo";
+  ASSERT_EQ(4096, ioctx.write(oid, bl, 4096, 0));
+  ASSERT_EQ(0, old_snapshot_add(&ioctx, oid, 1, "test1"));
+  ASSERT_EQ(0, old_snapshot_add(&ioctx, oid, 3, "test3"));
+  ASSERT_EQ(-ESTALE, old_snapshot_add(&ioctx, oid, 2, "test2"));
+
+  ioctx.close();
+  ASSERT_EQ(0, destroy_one_pool_pp(pool_name, rados));
+}