Merge pull request ceph#57954 from idryomov/wip-64662

librbd: allow cloning from non-user snapshots

Reviewed-by: Ramana Raja <[email protected]>
Reviewed-by: Afreen Misbah <[email protected]>

Author: idryomov

PendingReleaseNotes

@@ -226,6 +226,12 @@ CephFS: Disallow delegating preallocated inode ranges to clients. Config
   Copies, and list metrics.
 * RBD: `Image::access_timestamp` and `Image::modify_timestamp` Python APIs now
   return timestamps in UTC.
+* RBD: Support for cloning from non-user type snapshots is added.  This is
+  intended primarily as a building block for cloning new groups from group
+  snapshots created with `rbd group snap create` command, but has also been
+  exposed via the new `--snap-id` option for `rbd clone` command.
+* RBD: The output of `rbd snap ls --all` command now includes the original
+  type for trashed snapshots.
 
 >=18.0.0
 

qa/tasks/mgr/dashboard/test_rbd.py

@@ -236,9 +236,14 @@ def _validate_image(self, img, **kwargs):
             'features_name': JList(JLeaf(str)),
             'stripe_count': JLeaf(int, none=True),
             'stripe_unit': JLeaf(int, none=True),
-            'parent': JObj(sub_elems={'pool_name': JLeaf(str),
+            'parent': JObj(sub_elems={'pool_id': JLeaf(int),
+                                      'pool_name': JLeaf(str),
                                       'pool_namespace': JLeaf(str, none=True),
+                                      'image_id': JLeaf(str),
                                       'image_name': JLeaf(str),
+                                      'trash': JLeaf(bool),
+                                      'snap_id': JLeaf(int),
+                                      'snap_namespace_type': JLeaf(int),
                                       'snap_name': JLeaf(str)}, none=True),
             'data_pool': JLeaf(str, none=True),
             'snapshots': JList(JLeaf(dict)),
@@ -256,7 +261,12 @@ def _validate_image(self, img, **kwargs):
         self.assertSchema(img, schema)
 
         for k, v in kwargs.items():
-            if isinstance(v, list):
+            if k == 'parent' and v is not None:
+                # check that img['parent'] contains (is a superset of) v
+                actual = {pk: img['parent'][pk]
+                          for pk in v.keys() if pk in img['parent']}
+                self.assertEqual(actual, v)
+            elif isinstance(v, list):
                 self.assertSetEqual(set(img[k]), set(v))
             else:
                 self.assertEqual(img[k], v)

qa/workunits/rbd/cli_generic.sh

@@ -385,19 +385,35 @@ test_clone() {
     rbd clone test1@s1 rbd2/clone
     rbd -p rbd2 ls | grep clone
     rbd -p rbd2 ls -l | grep clone | grep test1@s1
-    rbd ls | grep -v clone
+    test "$(rbd ls)" = 'test1'
     rbd flatten rbd2/clone
     rbd snap create rbd2/clone@s1
     rbd snap protect rbd2/clone@s1
     rbd clone rbd2/clone@s1 clone2
     rbd ls | grep clone2
     rbd ls -l | grep clone2 | grep rbd2/clone@s1
-    rbd -p rbd2 ls | grep -v clone2
+    test "$(rbd -p rbd2 ls)" = 'clone'
+
+    rbd clone rbd2/clone clone3 |& grep 'snapshot name was not specified'
+    rbd clone rbd2/clone@invalid clone3 |& grep 'failed to open parent image'
+    rbd clone rbd2/clone --snap-id 0 clone3 |& grep 'failed to open parent image'
+    rbd clone rbd2/clone@invalid --snap-id 0 clone3 |&
+        grep 'trying to access snapshot using both name and id'
+    SNAP_ID=$(rbd snap ls rbd2/clone --format json |
+        jq '.[] | select(.name == "s1") | .id')
+    rbd clone --snap-id $SNAP_ID rbd2/clone clone3
+    rbd ls | grep clone3
+    rbd ls -l | grep clone3 | grep rbd2/clone@s1
+    test "$(rbd -p rbd2 ls)" = 'clone'
+    test "$(rbd ls -l | grep -c rbd2/clone@s1)" = '2'
+    rbd flatten clone3
+    test "$(rbd ls -l | grep -c rbd2/clone@s1)" = '1'
 
     rbd rm clone2
     rbd snap unprotect rbd2/clone@s1
     rbd snap rm rbd2/clone@s1
     rbd rm rbd2/clone
+    rbd rm clone3
     rbd snap unprotect test1@s1
     rbd snap rm test1@s1
     rbd rm test1
@@ -752,7 +768,9 @@ test_clone_v2() {
     rbd snap create test1@1
     rbd clone --rbd-default-clone-format=1 test1@1 test2 && exit 1 || true
     rbd clone --rbd-default-clone-format=2 test1@1 test2
-    rbd clone --rbd-default-clone-format=2 test1@1 test3
+    SNAP_ID=$(rbd snap ls test1 --format json |
+        jq '.[] | select(.name == "1") | .id')
+    rbd clone --rbd-default-clone-format=2 --snap-id $SNAP_ID test1 test3
 
     rbd snap protect test1@1
     rbd clone --rbd-default-clone-format=1 test1@1 test4
@@ -764,7 +782,7 @@ test_clone_v2() {
     rbd snap unprotect test1@1
 
     rbd snap remove test1@1
-    rbd snap list --all test1 | grep -E "trash \(1\) *$"
+    rbd snap list --all test1 | grep -E "trash \(user 1\) *$"
 
     rbd snap create test1@2
     rbd rm test1 2>&1 | grep 'image has snapshots'

qa/workunits/rbd/rbd_mirror_helpers.sh

@@ -1215,7 +1215,7 @@ test_snap_moved_to_trash()
     local snap_name=$4
 
     rbd --cluster ${cluster} snap ls ${pool}/${image} --all |
-        grep -F " trash (${snap_name})"
+        grep -F " trash (user ${snap_name})"
 }
 
 wait_for_snap_moved_to_trash()

src/include/rbd/librbd.h

@@ -260,6 +260,11 @@ typedef struct {
   char *group_snap_name;
 } rbd_snap_group_namespace_t;
 
+typedef struct {
+  rbd_snap_namespace_type_t original_namespace_type;
+  char *original_name;
+} rbd_snap_trash_namespace_t;
+
 typedef enum {
   RBD_SNAP_MIRROR_STATE_PRIMARY,
   RBD_SNAP_MIRROR_STATE_PRIMARY_DEMOTED,
@@ -479,6 +484,9 @@ CEPH_RBD_API int rbd_clone2(rados_ioctx_t p_ioctx, const char *p_name,
 CEPH_RBD_API int rbd_clone3(rados_ioctx_t p_ioctx, const char *p_name,
 	                    const char *p_snapname, rados_ioctx_t c_ioctx,
 	                    const char *c_name, rbd_image_options_t c_opts);
+CEPH_RBD_API int rbd_clone4(rados_ioctx_t p_ioctx, const char *p_name,
+                            uint64_t p_snap_id, rados_ioctx_t c_ioctx,
+                            const char *c_name, rbd_image_options_t c_opts);
 CEPH_RBD_API int rbd_remove(rados_ioctx_t io, const char *name);
 CEPH_RBD_API int rbd_remove_with_progress(rados_ioctx_t io, const char *name,
 			                  librbd_progress_fn_t cb,
@@ -965,6 +973,11 @@ CEPH_RBD_API int rbd_snap_get_trash_namespace(rbd_image_t image,
                                               uint64_t snap_id,
                                               char* original_name,
                                               size_t max_length);
+CEPH_RBD_API int rbd_snap_get_trash_namespace2(
+    rbd_image_t image, uint64_t snap_id,
+    rbd_snap_trash_namespace_t *trash_snap, size_t trash_snap_size);
+CEPH_RBD_API int rbd_snap_trash_namespace_cleanup(
+    rbd_snap_trash_namespace_t *trash_snap, size_t trash_snap_size);
 CEPH_RBD_API int rbd_snap_get_mirror_namespace(
     rbd_image_t image, uint64_t snap_id,
     rbd_snap_mirror_namespace_t *mirror_snap, size_t mirror_snap_size);

src/include/rbd/librbd.hpp

@@ -73,6 +73,11 @@ namespace librbd {
     std::string group_snap_name;
   } snap_group_namespace_t;
 
+  typedef struct {
+    snap_namespace_type_t original_namespace_type;
+    std::string original_name;
+  } snap_trash_namespace_t;
+
   typedef rbd_snap_mirror_state_t snap_mirror_state_t;
 
   typedef struct {
@@ -294,6 +299,8 @@ class CEPH_RBD_API RBD
 	     int *c_order, uint64_t stripe_unit, int stripe_count);
   int clone3(IoCtx& p_ioctx, const char *p_name, const char *p_snapname,
 	     IoCtx& c_ioctx, const char *c_name, ImageOptions& opts);
+  int clone4(IoCtx& p_ioctx, const char *p_name, uint64_t p_snap_id,
+	     IoCtx& c_ioctx, const char *c_name, ImageOptions& opts);
   int remove(IoCtx& io_ctx, const char *name);
   int remove_with_progress(IoCtx& io_ctx, const char *name, ProgressContext& pctx);
   int rename(IoCtx& src_io_ctx, const char *srcname, const char *destname);
@@ -677,6 +684,9 @@ class CEPH_RBD_API Image
                                snap_group_namespace_t *group_namespace,
                                size_t snap_group_namespace_size);
   int snap_get_trash_namespace(uint64_t snap_id, std::string* original_name);
+  int snap_get_trash_namespace2(uint64_t snap_id,
+                                snap_trash_namespace_t *trash_namespace,
+                                size_t snap_trash_namespace_size);
   int snap_get_mirror_namespace(
       uint64_t snap_id, snap_mirror_namespace_t *mirror_namespace,
       size_t snap_mirror_namespace_size);

src/librbd/api/Snapshot.cc

@@ -82,10 +82,10 @@ class GetGroupVisitor {
 
 class GetTrashVisitor {
 public:
-  std::string* original_name;
+  snap_trash_namespace_t *trash_snap;
 
-  explicit GetTrashVisitor(std::string* original_name)
-    : original_name(original_name) {
+  explicit GetTrashVisitor(snap_trash_namespace_t *trash_snap)
+    : trash_snap(trash_snap) {
   }
 
   template <typename T>
@@ -95,7 +95,9 @@ class GetTrashVisitor {
 
   inline int operator()(
       const cls::rbd::TrashSnapshotNamespace& snap_namespace) {
-    *original_name = snap_namespace.original_name;
+    trash_snap->original_namespace_type = static_cast<snap_namespace_type_t>(
+      snap_namespace.original_snapshot_namespace_type);
+    trash_snap->original_name = snap_namespace.original_name;
     return 0;
   }
 };
@@ -153,7 +155,7 @@ int Snapshot<I>::get_group_namespace(I *ictx, uint64_t snap_id,
 
 template <typename I>
 int Snapshot<I>::get_trash_namespace(I *ictx, uint64_t snap_id,
-                                     std::string* original_name) {
+                                     snap_trash_namespace_t *trash_snap) {
   int r = ictx->state->refresh_if_required();
   if (r < 0) {
     return r;
@@ -165,7 +167,7 @@ int Snapshot<I>::get_trash_namespace(I *ictx, uint64_t snap_id,
     return -ENOENT;
   }
 
-  auto visitor = GetTrashVisitor(original_name);
+  auto visitor = GetTrashVisitor(trash_snap);
   r = snap_info->snap_namespace.visit(visitor);
   if (r < 0) {
     return r;

src/librbd/api/Snapshot.h

@@ -21,7 +21,7 @@ struct Snapshot {
                                  snap_group_namespace_t *group_snap);
 
   static int get_trash_namespace(ImageCtxT *ictx, uint64_t snap_id,
-                                 std::string *original_name);
+                                 snap_trash_namespace_t *trash_snap);
 
   static int get_mirror_namespace(
       ImageCtxT *ictx, uint64_t snap_id,

src/librbd/image/CloneRequest.cc

@@ -397,7 +397,7 @@ void CloneRequest<I>::handle_attach_child(int r) {
   ldout(m_cct, 15) << "r=" << r << dendl;
 
   if (r < 0) {
-    lderr(m_cct) << "failed to attach parent: " << cpp_strerror(r) << dendl;
+    lderr(m_cct) << "failed to attach child: " << cpp_strerror(r) << dendl;
     m_r_saved = r;
     close_child();
     return;

src/librbd/internal.cc

@@ -716,24 +716,40 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     opts.set(RBD_IMAGE_OPTION_STRIPE_UNIT, stripe_unit);
     opts.set(RBD_IMAGE_OPTION_STRIPE_COUNT, stripe_count);
 
-    int r = clone(p_ioctx, nullptr, p_name, p_snap_name, c_ioctx, nullptr,
-                  c_name, opts, "", "");
+    int r = clone(p_ioctx, nullptr, p_name, CEPH_NOSNAP, p_snap_name,
+                  c_ioctx, nullptr, c_name, opts, "", "");
     opts.get(RBD_IMAGE_OPTION_ORDER, &order);
     *c_order = order;
     return r;
   }
 
   int clone(IoCtx& p_ioctx, const char *p_id, const char *p_name,
-            const char *p_snap_name, IoCtx& c_ioctx, const char *c_id,
-            const char *c_name, ImageOptions& c_opts,
+            uint64_t p_snap_id, const char *p_snap_name, IoCtx& c_ioctx,
+            const char *c_id, const char *c_name, ImageOptions& c_opts,
             const std::string &non_primary_global_image_id,
             const std::string &primary_mirror_uuid)
   {
-    ceph_assert((p_id == nullptr) ^ (p_name == nullptr));
-
     CephContext *cct = (CephContext *)p_ioctx.cct();
-    if (p_snap_name == nullptr) {
-      lderr(cct) << "image to be cloned must be a snapshot" << dendl;
+    ldout(cct, 10) << __func__
+                   << " p_id=" << (p_id ?: "")
+                   << ", p_name=" << (p_name ?: "")
+                   << ", p_snap_id=" << p_snap_id
+                   << ", p_snap_name=" << (p_snap_name ?: "")
+                   << ", c_id=" << (c_id ?: "")
+                   << ", c_name=" << c_name
+                   << ", c_opts=" << c_opts
+                   << ", non_primary_global_image_id=" << non_primary_global_image_id
+                   << ", primary_mirror_uuid=" << primary_mirror_uuid
+                   << dendl;
+
+    if (((p_id == nullptr) ^ (p_name == nullptr)) == 0) {
+      lderr(cct) << "must specify either parent image id or parent image name"
+                 << dendl;
+      return -EINVAL;
+    }
+    if (((p_snap_id == CEPH_NOSNAP) ^ (p_snap_name == nullptr)) == 0) {
+      lderr(cct) << "must specify either parent snap id or parent snap name"
+                 << dendl;
       return -EINVAL;
     }
 
@@ -766,10 +782,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
       clone_id = c_id;
     }
 
-    ldout(cct, 10) << __func__ << " "
-		   << "c_name=" << c_name << ", "
-		   << "c_id= " << clone_id << ", "
-		   << "c_opts=" << c_opts << dendl;
+    ldout(cct, 10) << __func__ << " parent_id=" << parent_id
+		   << ", clone_id=" << clone_id << dendl;
 
     ConfigProxy config{reinterpret_cast<CephContext *>(c_ioctx.cct())->_conf};
     api::Config<>::apply_pool_overrides(c_ioctx, &config);
@@ -778,8 +792,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
 
     C_SaferCond cond;
     auto *req = image::CloneRequest<>::create(
-      config, p_ioctx, parent_id, p_snap_name,
-      {cls::rbd::UserSnapshotNamespace{}}, CEPH_NOSNAP, c_ioctx, c_name,
+      config, p_ioctx, parent_id, (p_snap_name ?: ""),
+      {cls::rbd::UserSnapshotNamespace{}}, p_snap_id, c_ioctx, c_name,
       clone_id, c_opts, cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
       non_primary_global_image_id, primary_mirror_uuid,
       asio_engine.get_work_queue(), &cond);