From 24c3cae15d69810f14e4269532ec86c23e6b2b3b Mon Sep 17 00:00:00 2001 From: Josh Durgin Date: Mon, 30 Jul 2012 15:19:29 -0700 Subject: [PATCH] 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 Reviewed-by: Sage Weil --- src/cls_rbd.cc | 3 +++ src/librbd/internal.cc | 4 +++- src/test/rbd/test_cls_rbd.cc | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/cls_rbd.cc b/src/cls_rbd.cc index 22e0049e688dd..6328a557673f8 100644 --- 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 28e1d51e110ae..dbf2883cd9c32 100644 --- 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 04ace57ed6990..76eda304a47e2 100644 --- 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)); +} -- 2.39.5