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 22e0049e688ddea89def7da3b2ce1fd885a2e364..6328a557673f825d1e408fee923151d3165d0e79 100644 (file)
--- a/src/cls_rbd.cc
+++ b/src/cls_rbd.cc
@@ -1968,6 +1968,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/internal.cc b/src/librbd/internal.cc
index 28e1d51e110ae20e2d932bf03c3874fc03240548..dbf2883cd9c32c187af4a4f8d5921c9a5ec2516f 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -439,7 +439,9 @@ namespace librbd {
       return r;
 
     Mutex::Locker l(ictx->md_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 04ace57ed69900d8e818017492ebcda0eff431fb..76eda304a47e2694449639e6dfc9e03f8b33e64f 100644 (file)
--- a/src/test/rbd/test_cls_rbd.cc
+++ b/src/test/rbd/test_cls_rbd.cc
@@ -50,6 +50,7 @@ using ::librbd::parent_info;
 using ::librbd::parent_spec;
 using ::librbd::cls_client::get_protection_status;
 using ::librbd::cls_client::set_protection_status;
+using ::librbd::cls_client::old_snapshot_add;
 
 static char *random_buf(size_t len)
 {
@@ -948,3 +949,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));
+}