gian/ceph-mirror
1
0
Fork 0
mirror of https://github.com/ceph/ceph synced 2026-08-01 22:45:39 +00:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

librbd: propagate ENOENT for non-existent groups and images

Browse Source 
Calling `list_images()` on a non-existent group falsely succeeded and
returned an empty iterator.This happened because `Group<I>::image_list`
ignored the negative error code returned by `group_image_list`, allowing
execution to proceed blindly. Furthermore, the C API `rbd_group_image_list`
intercepted `-ENOENT` and forced a success (0) return value.

Similarly other public C APIs `rbd_snap_list` and `rbd_group_snap_list`
also intercepts -ENOENT and forces a success (0) return value which
is incorrect.

Fix these behavior by:
- Propagating errors properly inside `Group<I>::image_list`.
- Removing the incorrect `-ENOENT` masking in `rbd_group_image_list`,
`rbd_snap_list`, rbd_group_snap_list`

Now `list_images()` consistently raise an `ObjectNotFound` error when
invoked on a non-existent group and public C APIs propagate ENOENT
without masking it into 0.
Also Extended the `TestGroups` test fixture with `self.dne_group` to
validate the expected error paths across all relevant APIs.

Fixes: https://tracker.ceph.com/issues/78108
Signed-off-by: VinayBhaskar-V <vvarada@redhat.com>
This commit is contained in:
VinayBhaskar-V 2026-07-09 22:10:06 +05:30
parent e677c76f12
commit c4c937d81c
5 changed files with 31 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
PendingReleaseNotes
View File

@ -269,6 +269,10 @@
rather rigid and also challenging to disseminate in some environments. The
key can be embedded in the migration spec or just referenced from there while
stored in the MON config-key store.
* RBD: `Group::list_images` python API will now correctly raise an ObjectNotFound
error when invoked on a non-existent group. The C APIs `rbd_group_image_list`,
`rbd_snap_list`, `rbd_group_snap_list` and C++ API `group_image_list` will now
return ENOENT instead of masking the error and returning 0.
* BlueStore: The experimental PMEM device backend has been removed, together
with the ``pmem`` value of the ``bdev_type`` option and the DML/DSA offload
path. The hardware it targeted, Intel Optane DC persistent memory, was

5
src/librbd/api/Group.cc
View File

@ -822,7 +822,10 @@ int Group<I>::image_list(librados::IoCtx& group_ioctx,
std::vector<cls::rbd::GroupImageStatus> image_ids;
group_image_list(group_ioctx, group_name, &image_ids);
int r = group_image_list(group_ioctx, group_name, &image_ids);
if (r < 0) {
return r;
}
for (auto image_id : image_ids) {
IoCtx ioctx;

16
src/librbd/librbd.cc
View File

@ -5700,10 +5700,6 @@ extern "C" int rbd_snap_list(rbd_image_t image, rbd_snap_info_t *snaps,
memset(snaps, 0, sizeof(*snaps) * *max_snaps);
int r = librbd::api::Snapshot<>::list(ictx, cpp_snaps);
if (r == -ENOENT) {
tracepoint(librbd, snap_list_exit, 0, *max_snaps);
return 0;
}
if (r < 0) {
tracepoint(librbd, snap_list_exit, r, *max_snaps);
return r;
@ -7266,12 +7262,6 @@ extern "C" int rbd_group_image_list(rados_ioctx_t group_p,
int r = librbd::api::Group<>::image_list(group_ioctx, group_name,
&cpp_images);
if (r == -ENOENT) {
tracepoint(librbd, group_image_list_exit, 0);
*image_size = 0;
return 0;
}
if (r < 0) {
tracepoint(librbd, group_image_list_exit, r);
return r;
@ -7447,12 +7437,6 @@ extern "C" int rbd_group_snap_list(rados_ioctx_t group_p,
int r = librbd::api::Group<>::snap_list(group_ioctx, group_name, true, false,
&cpp_snaps);
if (r == -ENOENT) {
*snaps_size = 0;
tracepoint(librbd, group_snap_list_exit, 0);
return 0;
}
if (r < 0) {
tracepoint(librbd, group_snap_list_exit, r);
return r;

12
src/test/librbd/test_Groups.cc
View File

@ -164,6 +164,10 @@ TEST_F(TestGroup, add_image)
ASSERT_EQ(1U, num_images);
rbd_group_image_info_t images[1];
ASSERT_EQ(-ENOENT, rbd_group_image_list(ioctx, "group_does_not_exist",
images,
sizeof(rbd_group_image_info_t),
&num_images));
ASSERT_EQ(1, rbd_group_image_list(ioctx, group_name, images,
sizeof(rbd_group_image_info_t),
&num_images));
@ -233,6 +237,9 @@ TEST_F(TestGroup, add_imagePP)
RBD_OPERATION_FEATURE_GROUP);
std::vector<librbd::group_image_info_t> images;
ASSERT_EQ(-ENOENT, rbd.group_image_list(ioctx, "group_does_not_exist",
&images,
sizeof(librbd::group_image_info_t)));
ASSERT_EQ(0, rbd.group_image_list(ioctx, group_name, &images,
sizeof(librbd::group_image_info_t)));
ASSERT_EQ(1U, images.size());
@ -342,6 +349,9 @@ TEST_F(TestGroup, add_snapshot)
ASSERT_EQ(1U, num_snaps);
rbd_group_snap_info_t snaps[1];
ASSERT_EQ(-ENOENT, rbd_group_snap_list(ioctx, "group_does_not_exist", snaps,
sizeof(rbd_group_snap_info_t),
&num_snaps));
ASSERT_EQ(1, rbd_group_snap_list(ioctx, group_name, snaps,
sizeof(rbd_group_snap_info_t),
&num_snaps));
@ -437,6 +447,8 @@ TEST_F(TestGroup, add_snapshotPP)
ASSERT_EQ(0, rbd.group_snap_create(ioctx, group_name, snap_name));
std::vector<librbd::group_snap_info_t> snaps;
ASSERT_EQ(-ENOENT, rbd.group_snap_list(ioctx, "group_does_not_exist", &snaps,
sizeof(librbd::group_snap_info_t)));
ASSERT_EQ(0, rbd.group_snap_list(ioctx, group_name, &snaps,
sizeof(librbd::group_snap_info_t)));
ASSERT_EQ(1U, snaps.size());

14
src/test/pybind/test_rbd.py
View File

@ -3087,12 +3087,11 @@ class TestGroups(object):
RBD().remove(ioctx, name)
def test_group_image_add(self):
assert_raises(ObjectNotFound, self.dne_group.add_image, ioctx, image_name)
self.group.add_image(ioctx, image_name)
def test_group_image_list_empty(self):
eq([], list(self.group.list_images()))
def test_group_image_list(self):
assert_raises(ObjectNotFound, self.dne_group.list_images)
eq([], list(self.group.list_images()))
self.group.add_image(ioctx, image_name)
eq([image_name], [img['name'] for img in self.group.list_images()])
@ -3114,6 +3113,7 @@ class TestGroups(object):
eq([image_name], [img['name'] for img in self.group.list_images()])
def test_group_get_id(self):
assert_raises(ObjectNotFound, self.dne_group.id)
id = self.group.id()
assert isinstance(id, str)
assert len(id) > 0
@ -3142,6 +3142,7 @@ class TestGroups(object):
eq(group_name, group['name'])
eq([image_name], [img['name'] for img in self.group.list_images()])
assert_raises(ObjectNotFound, self.dne_group.remove_image, ioctx, image_name)
self.group.remove_image(ioctx, image_name)
eq([], list(self.group.list_images()))
with Image(ioctx, image_name) as image:
@ -3157,6 +3158,7 @@ class TestGroups(object):
assert_raises(ObjectNotFound, self.group.get_snap_info, "")
self.group.create_snap(snap_name)
assert_raises(ObjectNotFound, self.dne_group.get_snap_info, snap_name)
snap_info_dict = self.group.get_snap_info(snap_name)
image_names = []
assert sorted(snap_info_dict.keys()) == self.gp_snap_keys
@ -3194,6 +3196,7 @@ class TestGroups(object):
global snap_name
assert_raises(ObjectNotFound, self.dne_group.list_snaps)
eq([], list(self.group.list_snaps()))
assert_raises(ObjectNotFound, self.dne_group.create_snap, snap_name)
self.group.create_snap(snap_name)
eq([snap_name], [snap['name'] for snap in self.group.list_snaps()])
@ -3203,6 +3206,7 @@ class TestGroups(object):
eq(group_name, info['group_name'])
eq(snap_name, info['group_snap_name'])
assert_raises(ObjectNotFound, self.dne_group.remove_snap, snap_name)
self.group.remove_snap(snap_name)
eq([], list(self.group.list_snaps()))
@ -3270,6 +3274,7 @@ class TestGroups(object):
eq([], list(self.group.list_snaps()))
self.group.create_snap(snap_name)
eq([snap_name], [snap['name'] for snap in self.group.list_snaps()])
assert_raises(ObjectNotFound, self.dne_group.rename_snap, snap_name, new_snap_name)
self.group.rename_snap(snap_name, new_snap_name)
eq([new_snap_name], [snap['name'] for snap in self.group.list_snaps()])
self.group.remove_snap(new_snap_name)
@ -3465,6 +3470,9 @@ class TestGroups(object):
image_snaps = list(image.list_snaps())
assert [s['namespace'] for s in image_snaps] == [RBD_SNAP_NAMESPACE_TYPE_GROUP]
# no group exists
assert_raises(ObjectNotFound, self.dne_group.rollback_to_snap, snap_name1)
# group = []
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name1)
assert_raises(InvalidArgument, self.group.rollback_to_snap, snap_name2)