From 9ea03123a16d087146ec7efcf4a61f3464869dfd Mon Sep 17 00:00:00 2001 From: Ramana Raja Date: Sat, 10 Aug 2024 22:18:07 -0400 Subject: [PATCH] rbd: fix CLI output of `rbd group snap info` command ... when a group snapshot has no member images. A group snapshot can be created with no member images. For such a group snapshot, omit the 'image snap' and 'images' fields from the unformatted CLI output of `rbd group snap info` command so as to not confuse the user. In the librbd C/C++ data structures representing a group snapshot with no member images, set the 'image_snap_name' data member to an empty string. Fixes: https://tracker.ceph.com/issues/67436 Signed-off-by: Ramana Raja --- qa/workunits/rbd/rbd_groups.sh | 27 ++++++++++++++++++++++----- src/librbd/api/Group.cc | 7 ++++--- src/test/librbd/test_Groups.cc | 31 +++++++++++++++++++++++++++---- src/test/pybind/test_rbd.py | 12 ++++++++++++ src/tools/rbd/action/Group.cc | 10 +++++++--- 5 files changed, 72 insertions(+), 15 deletions(-) diff --git a/qa/workunits/rbd/rbd_groups.sh b/qa/workunits/rbd/rbd_groups.sh index 450095dabfc..ee3cb506740 100755 --- a/qa/workunits/rbd/rbd_groups.sh +++ b/qa/workunits/rbd/rbd_groups.sh @@ -210,15 +210,32 @@ check_snapshot_info() local snap_name=$2 local image_count=$3 - local snap_info=$(rbd group snap info $group_name@$snap_name --format=json) - local actual_snap_name=$(jq -r ".name" <<< "$snap_info") + local snap_info_json=$( + rbd group snap info $group_name@$snap_name --format=json) + local actual_snap_name=$(jq -r ".name" <<< "$snap_info_json") test "$actual_snap_name" = "$snap_name" || return 1 - local snap_state=$(jq -r ".state" <<< "$snap_info") + local snap_state=$(jq -r ".state" <<< "$snap_info_json") test "$snap_state" = "complete" || return 1 - local actual_image_count=$(jq '.images | length' <<< "$snap_info") - test "$actual_image_count" = "$image_count" + local actual_image_count=$(jq '.images | length' <<< "$snap_info_json") + test "$actual_image_count" = "$image_count" || return 1 + + local image_snap_name=$(jq -r '.image_snap_name' <<< "$snap_info_json") + local snap_info=$(rbd group snap info $group_name@$snap_name) + local snap_state=$(grep -w 'state:' <<< "$snap_info" | tr -d '\t') + test "$snap_state" = "state: complete" || return 1 + local image_snap_field=$(grep -w 'image snap:' <<< "$snap_info") + local images_field=$(grep -w 'images:' <<< "$snap_info") + if ((image_count != 0)); then + test -n "$image_snap_name" || return 1 + test -n "$image_snap_field" || return 1 + test -n "$images_field" || return 1 + else + test -z "$image_snap_name" || return 1 + test -z "$image_snap_field" || return 1 + test -z "$images_field" || return 1 + fi } echo "TEST: create remove consistency group" diff --git a/src/librbd/api/Group.cc b/src/librbd/api/Group.cc index f3891dcf163..bf1c53284f2 100644 --- a/src/librbd/api/Group.cc +++ b/src/librbd/api/Group.cc @@ -500,9 +500,10 @@ int GroupSnapshot_to_group_snap_info2( group_snap->id = cls_group_snap.id; group_snap->name = cls_group_snap.name; group_snap->state = static_cast(cls_group_snap.state); - group_snap->image_snap_name = calc_ind_image_snap_name(group_ioctx.get_id(), - group_id, - cls_group_snap.id); + if (!image_snaps.empty()) { + group_snap->image_snap_name = calc_ind_image_snap_name( + group_ioctx.get_id(), group_id, cls_group_snap.id); + } group_snap->image_snaps = std::move(image_snaps); return 0; diff --git a/src/test/librbd/test_Groups.cc b/src/test/librbd/test_Groups.cc index a739d6de66c..eaef20611ac 100644 --- a/src/test/librbd/test_Groups.cc +++ b/src/test/librbd/test_Groups.cc @@ -505,8 +505,6 @@ TEST_F(TestGroup, snap_get_info) const char *gp_name = "gp_snapgetinfo"; ASSERT_EQ(0, rbd_group_create(ioctx2, gp_name)); - ASSERT_EQ(0, rbd_group_image_add(ioctx2, gp_name, ioctx, - m_image_name.c_str())); const char *gp_snap_name = "snap_snapshot"; ASSERT_EQ(0, rbd_group_snap_create(ioctx2, gp_name, gp_snap_name)); @@ -517,6 +515,20 @@ TEST_F(TestGroup, snap_get_info) ASSERT_EQ(-ENOENT, rbd_group_snap_get_info(ioctx2, gp_name, "absent", &gp_snap_info)); + ASSERT_EQ(0, rbd_group_snap_get_info(ioctx2, gp_name, gp_snap_name, + &gp_snap_info)); + ASSERT_STREQ(gp_snap_name, gp_snap_info.name); + ASSERT_EQ(RBD_GROUP_SNAP_STATE_COMPLETE, gp_snap_info.state); + ASSERT_STREQ("", gp_snap_info.image_snap_name); + ASSERT_EQ(0U, gp_snap_info.image_snaps_count); + + rbd_group_snap_get_info_cleanup(&gp_snap_info); + ASSERT_EQ(0, rbd_group_snap_remove(ioctx2, gp_name, gp_snap_name)); + + ASSERT_EQ(0, rbd_group_image_add(ioctx2, gp_name, ioctx, + m_image_name.c_str())); + ASSERT_EQ(0, rbd_group_snap_create(ioctx2, gp_name, gp_snap_name)); + ASSERT_EQ(0, rbd_group_snap_get_info(ioctx2, gp_name, gp_snap_name, &gp_snap_info)); ASSERT_STREQ(gp_snap_name, gp_snap_info.name); @@ -545,8 +557,6 @@ TEST_F(TestGroup, snap_get_infoPP) const char *gp_name = "gp_snapgetinfoPP"; ASSERT_EQ(0, m_rbd.group_create(ioctx2, gp_name)); - ASSERT_EQ(0, m_rbd.group_image_add(ioctx2, gp_name, m_ioctx, - m_image_name.c_str())); const char *gp_snap_name = "snap_snapshot"; ASSERT_EQ(0, m_rbd.group_snap_create(ioctx2, gp_name, gp_snap_name)); @@ -557,6 +567,19 @@ TEST_F(TestGroup, snap_get_infoPP) ASSERT_EQ(-ENOENT, m_rbd.group_snap_get_info(ioctx2, gp_name, "absent", &gp_snap_info)); + ASSERT_EQ(0, m_rbd.group_snap_get_info(ioctx2, gp_name, gp_snap_name, + &gp_snap_info)); + ASSERT_EQ(gp_snap_name, gp_snap_info.name); + ASSERT_EQ(RBD_GROUP_SNAP_STATE_COMPLETE, gp_snap_info.state); + ASSERT_EQ("", gp_snap_info.image_snap_name); + ASSERT_EQ(0U, gp_snap_info.image_snaps.size()); + + ASSERT_EQ(0, m_rbd.group_snap_remove(ioctx2, gp_name, gp_snap_name)); + + ASSERT_EQ(0, m_rbd.group_image_add(ioctx2, gp_name, m_ioctx, + m_image_name.c_str())); + ASSERT_EQ(0, m_rbd.group_snap_create(ioctx2, gp_name, gp_snap_name)); + ASSERT_EQ(0, m_rbd.group_snap_get_info(ioctx2, gp_name, gp_snap_name, &gp_snap_info)); ASSERT_EQ(gp_snap_name, gp_snap_info.name); diff --git a/src/test/pybind/test_rbd.py b/src/test/pybind/test_rbd.py index b44a471a23f..9e307cbb9bf 100644 --- a/src/test/pybind/test_rbd.py +++ b/src/test/pybind/test_rbd.py @@ -2918,6 +2918,18 @@ class TestGroups(object): self.group.remove_snap(snap_name) assert_raises(ObjectNotFound, self.group.get_snap_info, snap_name) + def test_group_snap_get_info_no_member_images(self): + self.group.create_snap(snap_name) + + snap_info_dict = self.group.get_snap_info(snap_name) + assert sorted(snap_info_dict.keys()) == self.gp_snap_keys + assert snap_info_dict['name'] == snap_name + assert snap_info_dict['state'] == RBD_GROUP_SNAP_STATE_COMPLETE + assert snap_info_dict['image_snap_name'] == "" + assert snap_info_dict['image_snaps'] == [] + + self.group.remove_snap(snap_name) + def test_group_snap(self): global snap_name eq([], list(self.group.list_snaps())) diff --git a/src/tools/rbd/action/Group.cc b/src/tools/rbd/action/Group.cc index 080d26e1757..afcc24455f6 100644 --- a/src/tools/rbd/action/Group.cc +++ b/src/tools/rbd/action/Group.cc @@ -805,9 +805,13 @@ int execute_group_snap_info(const po::variables_map &vm, } else { std::cout << "rbd group snapshot '" << group_snap.name << "':\n" << "\tid: " << group_snap.id << std::endl - << "\tstate: " << state_string << std::endl - << "\timage snap: " << group_snap.image_snap_name << std::endl - << "\timages:" << std::endl; + << "\tstate: " << state_string << std::endl; + if (!group_snap.image_snaps.empty()) { + std::cout << "\timage snap: " << group_snap.image_snap_name << std::endl + << "\timages:" << std::endl; + } else { + ceph_assert(group_snap.image_snap_name.empty()); + } } std::sort(group_snap.image_snaps.begin(), group_snap.image_snaps.end(), -- 2.39.5