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 <rraja@redhat.com>

diff --git a/qa/workunits/rbd/rbd_groups.sh b/qa/workunits/rbd/rbd_groups.sh
index 450095dabfc1e805ab9cd333996ecf3b26a6344a..ee3cb5067406dac7cc9ca60935fe1e4f7a68ef55 100755 (executable)
--- 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 f3891dcf163d34587c94b279d9eb6b21d0fb0a4f..bf1c53284f2ba182363c0fe9094dcbc8ad9477a1 100644 (file)
--- 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<group_snap_state_t>(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 a739d6de66c9b68b44e2ea5c0ac2b56741d45007..eaef20611aca346e259ba0d634ead57b4265e915 100644 (file)
--- 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 b44a471a23f31c09bb41a93843318e70c545a21d..9e307cbb9bf1ed0c339a53e22ee145534a0b4eba 100644 (file)
--- 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 080d26e1757d09d50456d317e1983366786d5993..afcc24455f6f57ea1473c616208a5f93e1083a02 100644 (file)
--- 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(),