librbd: add rbd_snap_get_trash_namespace2() API to return full namespace

The existing rbd_snap_get_trash_namespace() API returns only the
original name of the deleted snapshot, omitting its namespace type.
While non-user snapshots have distinctive names, there is nothing
preventing the user from creating user snapshots with identical names
(i.e. starting with ".group" or ".mirror" prefix).  After cloning from
non-user snapshots is allowed, it's possible for such user snapshots to
get mixed up with non-user snapshots in the trash, so let's provide
means for disambiguation.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit ed09f3403f0f2cdb8a62f094d0654a8c6d8f49df)

diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h
index c0f6742412b4f470af25708beb40d1d2945fe202..5b9c17393b49d7784f51c385c4b588e4126eda60 100644 (file)
--- a/src/include/rbd/librbd.h
+++ b/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,
@@ -968,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);

diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp
index 2268dfe2b53fa8c5a6feb01ad5ac515b67af26a1..4b3c22296ebc945474722bb6c8f205f7773b1105 100644 (file)
--- a/src/include/rbd/librbd.hpp
+++ b/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 {
@@ -669,6 +674,9 @@ public:
                                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);

diff --git a/src/librbd/api/Snapshot.cc b/src/librbd/api/Snapshot.cc
index 306ddb593da8bcfda118fc2574f92f74fd597ba7..e32c79b97a30fe9d053205cbf8997ce798003b84 100644 (file)
--- a/src/librbd/api/Snapshot.cc
+++ b/src/librbd/api/Snapshot.cc
@@ -82,10 +82,10 @@ public:
 
 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 @@ public:
 
   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;

diff --git a/src/librbd/api/Snapshot.h b/src/librbd/api/Snapshot.h
index 7e06a5a8d0772b81c13e79ba60d88aa3521a679b..a2a7955e1c340abad6ea42758823684eb37b54ff 100644 (file)
--- a/src/librbd/api/Snapshot.h
+++ b/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,

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 818a9f74664df61aed91d18a72ef299abdd3bca1..1f75f0f4f0bfa21571c2091d423f395adff2f4b5 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -2480,8 +2480,29 @@ namespace librbd {
   int Image::snap_get_trash_namespace(uint64_t snap_id,
                                       std::string* original_name) {
     ImageCtx *ictx = (ImageCtx *)ctx;
+
+    snap_trash_namespace_t trash_snap;
+    int r = librbd::api::Snapshot<>::get_trash_namespace(ictx, snap_id,
+                                                         &trash_snap);
+    if (r < 0) {
+      return r;
+    }
+
+    *original_name = trash_snap.original_name;
+    return 0;
+  }
+
+  int Image::snap_get_trash_namespace2(
+      uint64_t snap_id, snap_trash_namespace_t *trash_snap,
+      size_t trash_snap_size) {
+    ImageCtx *ictx = (ImageCtx *)ctx;
+
+    if (trash_snap_size != sizeof(snap_trash_namespace_t)) {
+      return -ERANGE;
+    }
+
     return librbd::api::Snapshot<>::get_trash_namespace(ictx, snap_id,
-                                                        original_name);
+                                                        trash_snap);
   }
 
   int Image::snap_get_mirror_namespace(
@@ -7327,18 +7348,50 @@ extern "C" int rbd_snap_get_trash_namespace(rbd_image_t image, uint64_t snap_id,
                                             size_t max_length) {
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
 
-  std::string cpp_original_name;
+  librbd::snap_trash_namespace_t trash_namespace;
   int r = librbd::api::Snapshot<>::get_trash_namespace(ictx, snap_id,
-                                                       &cpp_original_name);
+                                                       &trash_namespace);
   if (r < 0) {
     return r;
   }
 
-  if (cpp_original_name.length() >= max_length) {
+  if (trash_namespace.original_name.length() >= max_length) {
+    return -ERANGE;
+  }
+
+  strcpy(original_name, trash_namespace.original_name.c_str());
+  return 0;
+}
+
+extern "C" 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) {
+  librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
+
+  if (trash_snap_size != sizeof(rbd_snap_trash_namespace_t)) {
+    return -ERANGE;
+  }
+
+  librbd::snap_trash_namespace_t trash_namespace;
+  int r = librbd::api::Snapshot<>::get_trash_namespace(ictx, snap_id,
+                                                       &trash_namespace);
+  if (r < 0) {
+    return r;
+  }
+
+  trash_snap->original_namespace_type = trash_namespace.original_namespace_type;
+  trash_snap->original_name = strdup(trash_namespace.original_name.c_str());
+  return 0;
+}
+
+extern "C" int rbd_snap_trash_namespace_cleanup(
+    rbd_snap_trash_namespace_t *trash_snap, size_t trash_snap_size) {
+  if (trash_snap_size != sizeof(rbd_snap_trash_namespace_t)) {
     return -ERANGE;
   }
 
-  strcpy(original_name, cpp_original_name.c_str());
+  free(trash_snap->original_name);
   return 0;
 }
 

diff --git a/src/pybind/rbd/c_rbd.pxd b/src/pybind/rbd/c_rbd.pxd
index 8bfe7b5c6c7539d98dbbf9c8d760b0bbac73b399..946a72b5b500ff04b0812dafd6db1ed88ab512a6 100644 (file)
--- a/src/pybind/rbd/c_rbd.pxd
+++ b/src/pybind/rbd/c_rbd.pxd
@@ -86,6 +86,10 @@ cdef extern from "rbd/librbd.h" nogil:
         char *group_name
         char *group_snap_name
 
+    ctypedef struct rbd_snap_trash_namespace_t:
+        rbd_snap_namespace_type_t original_namespace_type;
+        char *original_name;
+
     ctypedef enum rbd_snap_mirror_state_t:
         _RBD_SNAP_MIRROR_STATE_PRIMARY "RBD_SNAP_MIRROR_STATE_PRIMARY"
         _RBD_SNAP_MIRROR_STATE_PRIMARY_DEMOTED "RBD_SNAP_MIRROR_STATE_PRIMARY_DEMOTED"
@@ -549,8 +553,11 @@ cdef extern from "rbd/librbd.h" nogil:
                                      size_t snap_group_namespace_size)
     void rbd_snap_group_namespace_cleanup(rbd_snap_group_namespace_t *group_spec,
                                           size_t snap_group_namespace_size)
-    int rbd_snap_get_trash_namespace(rbd_image_t image, uint64_t snap_id,
-                                     char *original_name, size_t max_length)
+    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)
+    void rbd_snap_trash_namespace_cleanup(rbd_snap_trash_namespace_t *trash_snap,
+                                          size_t trash_snap_size)
     int rbd_snap_get_mirror_namespace(
         rbd_image_t image, uint64_t snap_id,
         rbd_snap_mirror_namespace_t *mirror_ns,

diff --git a/src/pybind/rbd/mock_rbd.pxi b/src/pybind/rbd/mock_rbd.pxi
index 75258eaa672d632e398d2cbdb5975bca3d4ec767..ec36d572f1f26cd690918ca18c7eac1e4f94bea3 100644 (file)
--- a/src/pybind/rbd/mock_rbd.pxi
+++ b/src/pybind/rbd/mock_rbd.pxi
@@ -90,6 +90,10 @@ cdef nogil:
         char *group_name
         char *group_snap_name
 
+    ctypedef struct rbd_snap_trash_namespace_t:
+        rbd_snap_namespace_type_t original_namespace_type;
+        char *original_name;
+
     ctypedef enum rbd_snap_mirror_state_t:
         _RBD_SNAP_MIRROR_STATE_PRIMARY "RBD_SNAP_MIRROR_STATE_PRIMARY"
         _RBD_SNAP_MIRROR_STATE_PRIMARY_DEMOTED "RBD_SNAP_MIRROR_STATE_PRIMARY_DEMOTED"
@@ -677,8 +681,12 @@ cdef nogil:
     void rbd_snap_group_namespace_cleanup(rbd_snap_group_namespace_t *group_spec,
                                           size_t snap_group_namespace_size):
         pass
-    int rbd_snap_get_trash_namespace(rbd_image_t image, uint64_t snap_id,
-                                     char *original_name, size_t max_length):
+    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):
+        pass
+    void rbd_snap_trash_namespace_cleanup(rbd_snap_trash_namespace_t *trash_snap,
+                                          size_t trash_snap_size):
         pass
     int rbd_snap_get_mirror_namespace(
         rbd_image_t image, uint64_t snap_id,

diff --git a/src/pybind/rbd/rbd.pyx b/src/pybind/rbd/rbd.pyx
index ba3484877f0447e4db13c68b824088c3a21e7dbe..c261daf9ecc6e687250da04cdf0aed20349455cd 100644 (file)
--- a/src/pybind/rbd/rbd.pyx
+++ b/src/pybind/rbd/rbd.pyx
@@ -5167,28 +5167,26 @@ written." % (self.name, ret, length))
         :type key: int
         :returns: dict - contains the following keys:
 
+            * ``original_namespace_type`` (int) - original snap namespace type
+
             * ``original_name`` (str) - original snap name
         """
         cdef:
+            rbd_snap_trash_namespace_t trash_snap
             uint64_t _snap_id = snap_id
-            size_t _size = 512
-            char *_name = NULL
-        try:
-            while True:
-                _name = <char*>realloc_chk(_name, _size);
-                with nogil:
-                    ret = rbd_snap_get_trash_namespace(self.image, _snap_id,
-                                                       _name, _size)
-                if ret >= 0:
-                    break
-                elif ret != -errno.ERANGE:
-                    raise make_ex(ret, 'error getting snapshot trash '
-                                       'namespace image: %s, snap_id: %d' % (self.name, snap_id))
-            return {
-                    'original_name' : decode_cstr(_name)
-                }
-        finally:
-            free(_name)
+        with nogil:
+            ret = rbd_snap_get_trash_namespace2(self.image, _snap_id,
+                                                &trash_snap, sizeof(trash_snap))
+        if ret != 0:
+            raise make_ex(ret, 'error getting snapshot trash '
+                               'namespace for image: %s, snap_id: %d' %
+                               (self.name, snap_id))
+        result = {
+            'original_namespace_type' : trash_snap.original_namespace_type,
+            'original_name' : decode_cstr(trash_snap.original_name)
+            }
+        rbd_snap_trash_namespace_cleanup(&trash_snap, sizeof(trash_snap))
+        return result
 
     @requires_not_closed
     def snap_get_mirror_namespace(self, snap_id):

diff --git a/src/test/pybind/test_rbd.py b/src/test/pybind/test_rbd.py
index 3095b1465ca724d7f2375258e248f07d83ba735c..01ac8b1e53808573ab34e9543304f7e0f954da9b 100644 (file)
--- a/src/test/pybind/test_rbd.py
+++ b/src/test/pybind/test_rbd.py
@@ -34,6 +34,7 @@ from rbd import (RBD, Group, Image, ImageNotFound, InvalidArgument, ImageExists,
                  RBD_MIRROR_IMAGE_MODE_JOURNAL, RBD_MIRROR_IMAGE_MODE_SNAPSHOT,
                  RBD_LOCK_MODE_EXCLUSIVE, RBD_OPERATION_FEATURE_GROUP,
                  RBD_OPERATION_FEATURE_CLONE_CHILD,
+                 RBD_SNAP_NAMESPACE_TYPE_USER,
                  RBD_SNAP_NAMESPACE_TYPE_GROUP,
                  RBD_SNAP_NAMESPACE_TYPE_TRASH,
                  RBD_SNAP_NAMESPACE_TYPE_MIRROR,
@@ -1842,6 +1843,7 @@ class TestClone(object):
         self.image.remove_snap('snap2')
         trash_snap = self.image.snap_get_trash_namespace(snap_id)
         assert trash_snap == {
+            'original_namespace_type' : RBD_SNAP_NAMESPACE_TYPE_USER,
             'original_name' : 'snap2'
             }
         clone_name3 = get_temp_image_name()
@@ -2138,7 +2140,11 @@ class TestClone(object):
 
         snaps = [s for s in self.image.list_snaps() if s['name'] != 'snap1']
         eq([RBD_SNAP_NAMESPACE_TYPE_TRASH], [s['namespace'] for s in snaps])
-        eq([{'original_name' : 'snap2'}], [s['trash'] for s in snaps])
+        trash_snap = {
+            'original_namespace_type' : RBD_SNAP_NAMESPACE_TYPE_USER,
+            'original_name' : 'snap2'
+            }
+        eq([trash_snap], [s['trash'] for s in snaps])
 
         self.rbd.remove(ioctx, clone_name)
         eq([], [s for s in self.image.list_snaps() if s['name'] != 'snap1'])
@@ -2978,6 +2984,7 @@ class TestGroups(object):
         trash_image_snap_name = image_snaps[0]['name']
         assert image_snaps[0]['id'] == image_snap_id
         assert image_snaps[0]['trash'] == {
+            'original_namespace_type' : RBD_SNAP_NAMESPACE_TYPE_GROUP,
             'original_name' : image_snap_name
             }
         assert trash_image_snap_name != image_snap_name