From: Ilya Dryomov Date: Fri, 14 Jun 2024 12:04:39 +0000 (+0200) Subject: librbd: disallow group snap rollback if memberships don't match X-Git-Tag: v20.0.0~1708^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f212a9ca5b9238b80ae5c728a7bf99366ed910a1;p=ceph.git 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 --- diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h index c8d2cb871e443..c1d64805ae429 100644 --- a/src/cls/rbd/cls_rbd_types.h +++ b/src/cls/rbd/cls_rbd_types.h @@ -374,6 +374,7 @@ struct GroupImageSpec { std::string image_key(); + bool operator==(const GroupImageSpec&) const = default; }; WRITE_CLASS_ENCODER(GroupImageSpec); diff --git a/src/librbd/api/Group.cc b/src/librbd/api/Group.cc index afa5b0aff032b..942426c95680a 100644 --- a/src/librbd/api/Group.cc +++ b/src/librbd/api/Group.cc @@ -1281,6 +1281,35 @@ int Group::snap_rollback(librados::IoCtx& group_ioctx, return -ENOENT; } + if (group_snap->state != cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE) { + lderr(cct) << "group snapshot is not complete" << dendl; + return -EINVAL; + } + + std::vector rollback_images; + for (const auto& snap : group_snap->snaps) { + rollback_images.emplace_back(snap.image_id, snap.pool); + } + + std::vector images; + r = group_image_list(group_ioctx, group_name, &images); + if (r < 0) { + return r; + } + + std::vector current_images; + for (const auto& image : images) { + if (image.state == cls::rbd::GROUP_IMAGE_LINK_STATE_ATTACHED) { + current_images.push_back(image.spec); + } + } + + if (rollback_images != current_images) { + lderr(cct) << "group snapshot membership does not match group membership" + << dendl; + return -EINVAL; + } + string group_header_oid = util::group_header_name(group_id); r = group_snap_rollback_by_record(group_ioctx, *group_snap, group_id, group_header_oid, pctx); diff --git a/src/test/pybind/test_rbd.py b/src/test/pybind/test_rbd.py index 5f52888f8eca0..45aab84d745b3 100644 --- a/src/test/pybind/test_rbd.py +++ b/src/test/pybind/test_rbd.py @@ -3057,33 +3057,187 @@ class TestGroups(object): self.rbd.remove(ioctx, clone_name) def test_group_snap_rollback(self): - eq([], list(self.group.list_images())) - self.group.add_image(ioctx, image_name) - with Image(ioctx, image_name) as image: - image.write(b'\0' * 256, 0) + for _ in range(1, 3): + create_image() + self.image_names.append(image_name) + + with Image(ioctx, self.image_names[0]) as image: + image.write(b'1' * 256, 0) + with Image(ioctx, self.image_names[1]) as image: + image.write(b'2' * 256, 0) + with Image(ioctx, self.image_names[2]) as image: + image.write(b'3' * 256, 0) + self.group.add_image(ioctx, self.image_names[0]) + snap_name1 = get_temp_snap_name() + self.group.create_snap(snap_name1) + + with Image(ioctx, self.image_names[0]) as image: + image.write(b'4' * 256, 0) + with Image(ioctx, self.image_names[1]) as image: + image.write(b'5' * 256, 0) + with Image(ioctx, self.image_names[2]) as image: + image.write(b'6' * 256, 0) + self.group.add_image(ioctx, self.image_names[1]) + snap_name2 = get_temp_snap_name() + self.group.create_snap(snap_name2) + + with Image(ioctx, self.image_names[0]) as image: + image.write(b'7' * 256, 0) + with Image(ioctx, self.image_names[1]) as image: + image.write(b'8' * 256, 0) + with Image(ioctx, self.image_names[2]) as image: + image.write(b'9' * 256, 0) + self.group.add_image(ioctx, self.image_names[2]) + snap_name3 = get_temp_snap_name() + self.group.create_snap(snap_name3) + + with Image(ioctx, self.image_names[0]) as image: + image.write(b'a' * 256, 0) + with Image(ioctx, self.image_names[1]) as image: + image.write(b'b' * 256, 0) + with Image(ioctx, self.image_names[2]) as image: + image.write(b'c' * 256, 0) + + for i in range(0, 3): + self.group.remove_image(ioctx, self.image_names[i]) + with Image(ioctx, self.image_names[0]) as image: + image_snaps = list(image.list_snaps()) + assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP, + RBD_SNAP_NAMESPACE_TYPE_GROUP, + RBD_SNAP_NAMESPACE_TYPE_GROUP] + with Image(ioctx, self.image_names[1]) as image: + image_snaps = list(image.list_snaps()) + assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP, + RBD_SNAP_NAMESPACE_TYPE_GROUP] + with Image(ioctx, self.image_names[2]) as image: + image_snaps = list(image.list_snaps()) + assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP] + + # group = [] + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + with Image(ioctx, self.image_names[0]) as image: + read = image.read(0, 256) + assert read == b'a' * 256 + with Image(ioctx, self.image_names[1]) as image: read = image.read(0, 256) - eq(read, b'\0' * 256) + assert read == b'b' * 256 + with Image(ioctx, self.image_names[2]) as image: + read = image.read(0, 256) + assert read == b'c' * 256 - global snap_name - eq([], list(self.group.list_snaps())) - self.group.create_snap(snap_name) - eq([snap_name], [snap['name'] for snap in self.group.list_snaps()]) + # group = [img0] + self.group.add_image(ioctx, self.image_names[0]) + self.group.rollback_to_snap(snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) - with Image(ioctx, image_name) as image: - data = rand_data(256) - image.write(data, 0) + with Image(ioctx, self.image_names[0]) as image: + read = image.read(0, 256) + assert read == b'1' * 256 + with Image(ioctx, self.image_names[1]) as image: + read = image.read(0, 256) + assert read == b'b' * 256 + with Image(ioctx, self.image_names[2]) as image: + read = image.read(0, 256) + assert read == b'c' * 256 + + # group = [img1] + self.group.remove_image(ioctx, self.image_names[0]) + self.group.add_image(ioctx, self.image_names[1]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + # group = [img2] + self.group.remove_image(ioctx, self.image_names[1]) + self.group.add_image(ioctx, self.image_names[2]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + # group = [img0 img1] + self.group.remove_image(ioctx, self.image_names[2]) + # re-add in reverse order to test that order doesn't matter + self.group.add_image(ioctx, self.image_names[1]) + self.group.add_image(ioctx, self.image_names[0]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + self.group.rollback_to_snap(snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + with Image(ioctx, self.image_names[0]) as image: read = image.read(0, 256) - eq(read, data) + assert read == b'4' * 256 + with Image(ioctx, self.image_names[1]) as image: + read = image.read(0, 256) + assert read == b'5' * 256 + with Image(ioctx, self.image_names[2]) as image: + read = image.read(0, 256) + assert read == b'c' * 256 + + # group = [img0 img2] + self.group.remove_image(ioctx, self.image_names[1]) + self.group.add_image(ioctx, self.image_names[2]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + # group = [img1 img2] + self.group.remove_image(ioctx, self.image_names[0]) + self.group.add_image(ioctx, self.image_names[1]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + # group = [img0 img1 img2] + self.group.add_image(ioctx, self.image_names[0]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + self.group.rollback_to_snap(snap_name3) + + with Image(ioctx, self.image_names[0]) as image: + read = image.read(0, 256) + assert read == b'7' * 256 + with Image(ioctx, self.image_names[1]) as image: + read = image.read(0, 256) + assert read == b'8' * 256 + with Image(ioctx, self.image_names[2]) as image: + read = image.read(0, 256) + assert read == b'9' * 256 - self.group.rollback_to_snap(snap_name) - with Image(ioctx, image_name) as image: + # group = [img0 img1] + self.group.remove_image(ioctx, self.image_names[2]) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1) + self.group.rollback_to_snap(snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + with Image(ioctx, self.image_names[0]) as image: + read = image.read(0, 256) + assert read == b'4' * 256 + with Image(ioctx, self.image_names[1]) as image: + read = image.read(0, 256) + assert read == b'5' * 256 + with Image(ioctx, self.image_names[2]) as image: read = image.read(0, 256) - eq(read, b'\0' * 256) + assert read == b'9' * 256 - self.group.remove_image(ioctx, image_name) - eq([], list(self.group.list_images())) - self.group.remove_snap(snap_name) - eq([], list(self.group.list_snaps())) + # group = [img0] + self.group.remove_image(ioctx, self.image_names[1]) + self.group.rollback_to_snap(snap_name1) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2) + assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name3) + + with Image(ioctx, self.image_names[0]) as image: + read = image.read(0, 256) + assert read == b'1' * 256 + with Image(ioctx, self.image_names[1]) as image: + read = image.read(0, 256) + assert read == b'5' * 256 + with Image(ioctx, self.image_names[2]) as image: + read = image.read(0, 256) + assert read == b'9' * 256 class TestMigration(object):