Skip to content

Commit f212a9c

Browse files
idryomovidryomov
committed
librbd: disallow group snap rollback if memberships don't match
Before proceeding with group rollback, ensure that the set of images that took part in the group snapshot matches the set of images that are currently part of the group. Otherwise, because we preserve affected snapshots when an image is removed from the group, data loss can ensue where an image gets rolled back while part of another group or not part of any group but long repurposed for something else. Similarly, ensure that the group snapshot is complete. Fixes: https://tracker.ceph.com/issues/66300 Signed-off-by: Ilya Dryomov <[email protected]>
1 parent 91a0632 commit f212a9c

File tree

3 files changed

+204
-20
lines changed

3 files changed

+204
-20
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: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,6 +1281,35 @@ int Group<I>::snap_rollback(librados::IoCtx& group_ioctx,
12811281
return -ENOENT;
12821282
}
12831283

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

‎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)