Skip to content

Commit b5ed96c

Browse files
idryomovidryomov
authored
Merge pull request ceph#58074 from idryomov/wip-66300
librbd: disallow group snap rollback if memberships don't match Reviewed-by: Ramana Raja <[email protected]>
2 parents 73b599d + afbb744 commit b5ed96c

File tree

5 files changed

+214
-32
lines changed

5 files changed

+214
-32
lines changed

‎src/cls/rbd/cls_rbd_types.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ struct GroupImageSpec {
374374

375375
std::string image_key();
376376

377+
bool operator==(const GroupImageSpec&) const = default;
377378
};
378379
WRITE_CLASS_ENCODER(GroupImageSpec);
379380

‎src/librbd/api/Group.cc‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ int group_snap_remove_by_record(librados::IoCtx& group_ioctx,
309309
int group_snap_rollback_by_record(librados::IoCtx& group_ioctx,
310310
const cls::rbd::GroupSnapshot& group_snap,
311311
const std::string& group_id,
312-
const std::string& group_header_oid,
313312
ProgressContext& pctx) {
314313
CephContext *cct = (CephContext *)group_ioctx.cct();
315314
std::vector<C_SaferCond*> on_finishes;
@@ -1281,9 +1280,36 @@ int Group<I>::snap_rollback(librados::IoCtx& group_ioctx,
12811280
return -ENOENT;
12821281
}
12831282

1284-
string group_header_oid = util::group_header_name(group_id);
1285-
r = group_snap_rollback_by_record(group_ioctx, *group_snap, group_id,
1286-
group_header_oid, pctx);
1283+
if (group_snap->state != cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE) {
1284+
lderr(cct) << "group snapshot is not complete" << dendl;
1285+
return -EINVAL;
1286+
}
1287+
1288+
std::vector<cls::rbd::GroupImageSpec> rollback_images;
1289+
for (const auto& snap : group_snap->snaps) {
1290+
rollback_images.emplace_back(snap.image_id, snap.pool);
1291+
}
1292+
1293+
std::vector<cls::rbd::GroupImageStatus> images;
1294+
r = group_image_list(group_ioctx, group_name, &images);
1295+
if (r < 0) {
1296+
return r;
1297+
}
1298+
1299+
std::vector<cls::rbd::GroupImageSpec> current_images;
1300+
for (const auto& image : images) {
1301+
if (image.state == cls::rbd::GROUP_IMAGE_LINK_STATE_ATTACHED) {
1302+
current_images.push_back(image.spec);
1303+
}
1304+
}
1305+
1306+
if (rollback_images != current_images) {
1307+
lderr(cct) << "group snapshot membership does not match group membership"
1308+
<< dendl;
1309+
return -EINVAL;
1310+
}
1311+
1312+
r = group_snap_rollback_by_record(group_ioctx, *group_snap, group_id, pctx);
12871313
return r;
12881314
}
12891315

‎src/test/librados_test_stub/TestMemIoCtxImpl.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ int TestMemIoCtxImpl::selfmanaged_snap_rollback(const std::string& oid,
479479
for (TestMemCluster::FileSnapshots::reverse_iterator it = snaps.rbegin();
480480
it != snaps.rend(); ++it) {
481481
TestMemCluster::SharedFile file = *it;
482-
if (file->snap_id < get_snap_read()) {
482+
if (file->snap_id < snapid) {
483483
if (versions == 0) {
484484
// already at the snapshot version
485485
return 0;

‎src/test/librbd/test_Groups.cc‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,12 @@ TEST_F(TestGroup, add_snapshot)
345345

346346
ASSERT_STREQ(snap_name, snaps[0].name);
347347

348-
ASSERT_EQ(10, rbd_write(image, 11, 10, test_data));
349-
ASSERT_EQ(10, rbd_read(image, 11, 10, read_data));
348+
ASSERT_EQ(10, rbd_write(image, 9, 10, test_data));
349+
ASSERT_EQ(10, rbd_read(image, 9, 10, read_data));
350350
ASSERT_EQ(0, memcmp(test_data, read_data, 10));
351351

352+
ASSERT_EQ(10, rbd_read(image, 0, 10, read_data));
353+
ASSERT_NE(0, memcmp(orig_data, read_data, 10));
352354
ASSERT_EQ(0, rbd_group_snap_rollback(ioctx, group_name, snap_name));
353355
ASSERT_EQ(10, rbd_read(image, 0, 10, read_data));
354356
ASSERT_EQ(0, memcmp(orig_data, read_data, 10));
@@ -435,14 +437,13 @@ TEST_F(TestGroup, add_snapshotPP)
435437

436438
bufferlist write_bl;
437439
write_bl.append(std::string(1024, '2'));
438-
ASSERT_EQ(1024, image.write(513, write_bl.length(), write_bl));
439-
440-
read_bl.clear();
441-
ASSERT_EQ(1024, image.read(513, 1024, read_bl));
440+
ASSERT_EQ(1024, image.write(256, write_bl.length(), write_bl));
441+
ASSERT_EQ(1024, image.read(256, 1024, read_bl));
442442
ASSERT_TRUE(write_bl.contents_equal(read_bl));
443443

444+
ASSERT_EQ(512, image.read(0, 512, read_bl));
445+
ASSERT_FALSE(expect_bl.contents_equal(read_bl));
444446
ASSERT_EQ(0, rbd.group_snap_rollback(ioctx, group_name, snap_name));
445-
446447
ASSERT_EQ(512, image.read(0, 512, read_bl));
447448
ASSERT_TRUE(expect_bl.contents_equal(read_bl));
448449

‎src/test/pybind/test_rbd.py‎

Lines changed: 174 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,33 +3057,187 @@ def test_group_snap_clone_flatten(self):
30573057
self.rbd.remove(ioctx, clone_name)
30583058

30593059
def test_group_snap_rollback(self):
3060-
eq([], list(self.group.list_images()))
3061-
self.group.add_image(ioctx, image_name)
3062-
with Image(ioctx, image_name) as image:
3063-
image.write(b'\0' * 256, 0)
3060+
for _ in range(1, 3):
3061+
create_image()
3062+
self.image_names.append(image_name)
3063+
3064+
with Image(ioctx, self.image_names[0]) as image:
3065+
image.write(b'1' * 256, 0)
3066+
with Image(ioctx, self.image_names[1]) as image:
3067+
image.write(b'2' * 256, 0)
3068+
with Image(ioctx, self.image_names[2]) as image:
3069+
image.write(b'3' * 256, 0)
3070+
self.group.add_image(ioctx, self.image_names[0])
3071+
snap_name1 = get_temp_snap_name()
3072+
self.group.create_snap(snap_name1)
3073+
3074+
with Image(ioctx, self.image_names[0]) as image:
3075+
image.write(b'4' * 256, 0)
3076+
with Image(ioctx, self.image_names[1]) as image:
3077+
image.write(b'5' * 256, 0)
3078+
with Image(ioctx, self.image_names[2]) as image:
3079+
image.write(b'6' * 256, 0)
3080+
self.group.add_image(ioctx, self.image_names[1])
3081+
snap_name2 = get_temp_snap_name()
3082+
self.group.create_snap(snap_name2)
3083+
3084+
with Image(ioctx, self.image_names[0]) as image:
3085+
image.write(b'7' * 256, 0)
3086+
with Image(ioctx, self.image_names[1]) as image:
3087+
image.write(b'8' * 256, 0)
3088+
with Image(ioctx, self.image_names[2]) as image:
3089+
image.write(b'9' * 256, 0)
3090+
self.group.add_image(ioctx, self.image_names[2])
3091+
snap_name3 = get_temp_snap_name()
3092+
self.group.create_snap(snap_name3)
3093+
3094+
with Image(ioctx, self.image_names[0]) as image:
3095+
image.write(b'a' * 256, 0)
3096+
with Image(ioctx, self.image_names[1]) as image:
3097+
image.write(b'b' * 256, 0)
3098+
with Image(ioctx, self.image_names[2]) as image:
3099+
image.write(b'c' * 256, 0)
3100+
3101+
for i in range(0, 3):
3102+
self.group.remove_image(ioctx, self.image_names[i])
3103+
with Image(ioctx, self.image_names[0]) as image:
3104+
image_snaps = list(image.list_snaps())
3105+
assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP,
3106+
RBD_SNAP_NAMESPACE_TYPE_GROUP,
3107+
RBD_SNAP_NAMESPACE_TYPE_GROUP]
3108+
with Image(ioctx, self.image_names[1]) as image:
3109+
image_snaps = list(image.list_snaps())
3110+
assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP,
3111+
RBD_SNAP_NAMESPACE_TYPE_GROUP]
3112+
with Image(ioctx, self.image_names[2]) as image:
3113+
image_snaps = list(image.list_snaps())
3114+
assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP]
3115+
3116+
# group = []
3117+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3118+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3119+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3120+
3121+
with Image(ioctx, self.image_names[0]) as image:
3122+
read = image.read(0, 256)
3123+
assert read == b'a' * 256
3124+
with Image(ioctx, self.image_names[1]) as image:
30643125
read = image.read(0, 256)
3065-
eq(read, b'\0' * 256)
3126+
assert read == b'b' * 256
3127+
with Image(ioctx, self.image_names[2]) as image:
3128+
read = image.read(0, 256)
3129+
assert read == b'c' * 256
30663130

3067-
global snap_name
3068-
eq([], list(self.group.list_snaps()))
3069-
self.group.create_snap(snap_name)
3070-
eq([snap_name], [snap['name'] for snap in self.group.list_snaps()])
3131+
# group = [img0]
3132+
self.group.add_image(ioctx, self.image_names[0])
3133+
self.group.rollback_to_snap(snap_name1)
3134+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3135+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
30713136

3072-
with Image(ioctx, image_name) as image:
3073-
data = rand_data(256)
3074-
image.write(data, 0)
3137+
with Image(ioctx, self.image_names[0]) as image:
3138+
read = image.read(0, 256)
3139+
assert read == b'1' * 256
3140+
with Image(ioctx, self.image_names[1]) as image:
3141+
read = image.read(0, 256)
3142+
assert read == b'b' * 256
3143+
with Image(ioctx, self.image_names[2]) as image:
3144+
read = image.read(0, 256)
3145+
assert read == b'c' * 256
3146+
3147+
# group = [img1]
3148+
self.group.remove_image(ioctx, self.image_names[0])
3149+
self.group.add_image(ioctx, self.image_names[1])
3150+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3151+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3152+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3153+
3154+
# group = [img2]
3155+
self.group.remove_image(ioctx, self.image_names[1])
3156+
self.group.add_image(ioctx, self.image_names[2])
3157+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3158+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3159+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3160+
3161+
# group = [img0 img1]
3162+
self.group.remove_image(ioctx, self.image_names[2])
3163+
# re-add in reverse order to test that order doesn't matter
3164+
self.group.add_image(ioctx, self.image_names[1])
3165+
self.group.add_image(ioctx, self.image_names[0])
3166+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3167+
self.group.rollback_to_snap(snap_name2)
3168+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3169+
3170+
with Image(ioctx, self.image_names[0]) as image:
30753171
read = image.read(0, 256)
3076-
eq(read, data)
3172+
assert read == b'4' * 256
3173+
with Image(ioctx, self.image_names[1]) as image:
3174+
read = image.read(0, 256)
3175+
assert read == b'5' * 256
3176+
with Image(ioctx, self.image_names[2]) as image:
3177+
read = image.read(0, 256)
3178+
assert read == b'c' * 256
3179+
3180+
# group = [img0 img2]
3181+
self.group.remove_image(ioctx, self.image_names[1])
3182+
self.group.add_image(ioctx, self.image_names[2])
3183+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3184+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3185+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3186+
3187+
# group = [img1 img2]
3188+
self.group.remove_image(ioctx, self.image_names[0])
3189+
self.group.add_image(ioctx, self.image_names[1])
3190+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3191+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3192+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3193+
3194+
# group = [img0 img1 img2]
3195+
self.group.add_image(ioctx, self.image_names[0])
3196+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3197+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3198+
self.group.rollback_to_snap(snap_name3)
3199+
3200+
with Image(ioctx, self.image_names[0]) as image:
3201+
read = image.read(0, 256)
3202+
assert read == b'7' * 256
3203+
with Image(ioctx, self.image_names[1]) as image:
3204+
read = image.read(0, 256)
3205+
assert read == b'8' * 256
3206+
with Image(ioctx, self.image_names[2]) as image:
3207+
read = image.read(0, 256)
3208+
assert read == b'9' * 256
30773209

3078-
self.group.rollback_to_snap(snap_name)
3079-
with Image(ioctx, image_name) as image:
3210+
# group = [img0 img1]
3211+
self.group.remove_image(ioctx, self.image_names[2])
3212+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
3213+
self.group.rollback_to_snap(snap_name2)
3214+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3215+
3216+
with Image(ioctx, self.image_names[0]) as image:
3217+
read = image.read(0, 256)
3218+
assert read == b'4' * 256
3219+
with Image(ioctx, self.image_names[1]) as image:
3220+
read = image.read(0, 256)
3221+
assert read == b'5' * 256
3222+
with Image(ioctx, self.image_names[2]) as image:
30803223
read = image.read(0, 256)
3081-
eq(read, b'\0' * 256)
3224+
assert read == b'9' * 256
30823225

3083-
self.group.remove_image(ioctx, image_name)
3084-
eq([], list(self.group.list_images()))
3085-
self.group.remove_snap(snap_name)
3086-
eq([], list(self.group.list_snaps()))
3226+
# group = [img0]
3227+
self.group.remove_image(ioctx, self.image_names[1])
3228+
self.group.rollback_to_snap(snap_name1)
3229+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)
3230+
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3)
3231+
3232+
with Image(ioctx, self.image_names[0]) as image:
3233+
read = image.read(0, 256)
3234+
assert read == b'1' * 256
3235+
with Image(ioctx, self.image_names[1]) as image:
3236+
read = image.read(0, 256)
3237+
assert read == b'5' * 256
3238+
with Image(ioctx, self.image_names[2]) as image:
3239+
read = image.read(0, 256)
3240+
assert read == b'9' * 256
30873241

30883242
class TestMigration(object):
30893243

0 commit comments

Comments
 (0)